From 4703eac5fc8575b24f68aa062b0d52dd86cbe2f9 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:50:05 +0000 Subject: [PATCH 1/5] shell: tests for the redirect of a command that does not start --- test/js/bun/shell/bunshell.test.ts | 312 +++++++++++++++++- test/js/bun/shell/exec.test.ts | 12 + test/js/bun/shell/leak.test.ts | 19 ++ .../shell/shell-worker-terminate-leak.test.ts | 56 +++- 4 files changed, 397 insertions(+), 2 deletions(-) diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 16a4e10be530..7081a555cbad 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -6,10 +6,11 @@ */ import { $ } from "bun"; import { afterAll, beforeAll, describe, expect, it, test } from "bun:test"; -import { chmodSync, mkdirSync, symlinkSync } from "fs"; +import { chmodSync, closeSync, fstatSync, mkdirSync, openSync, readFileSync, symlinkSync } from "fs"; import { mkdir, rm, stat } from "fs/promises"; import { bunExe, + isLinux, isPosix, isRoot, isWindows, @@ -1967,6 +1968,315 @@ describe("deno_task", () => { TestBuilder.command`BUN_DEBUG_QUIET_LOGS=1 ${BUN} -e ${code} > /dev/null` .quiet() .runAsTest("bunception redirect /dev/null"); + + // The redirect of a command is performed before the command is searched + // for (POSIX 2.9.1). So the shell's own message about a command that does + // not start goes where the redirect sends stderr, and the target exists. + describe("a command that does not start", () => { + const notFound = "bun: command not found: nosuch_cmd_zz\n"; + + TestBuilder.command`nosuch_cmd_zz 2> err.txt` + .exitCode(1) + .fileEquals("err.txt", notFound) + .testMini() + .runAsTest("2> file"); + + TestBuilder.command`nosuch_cmd_zz 2>> err.txt` + .file("err.txt", "pre\n") + .exitCode(1) + .fileEquals("err.txt", "pre\n" + notFound) + .runAsTest("2>> file"); + + TestBuilder.command`nosuch_cmd_zz &> both.txt` + .exitCode(1) + .fileEquals("both.txt", notFound) + .testMini() + .runAsTest("&> file"); + + TestBuilder.command`nosuch_cmd_zz > out.txt` + .file("out.txt", "pre\n") + .exitCode(1) + .stderr(notFound) + .fileEquals("out.txt", "") + .runAsTest("> file makes the file empty"); + + TestBuilder.command`nosuch_cmd_zz 2>&1`.exitCode(1).stdout(notFound).testMini().runAsTest("2>&1"); + + // https://github.com/oven-sh/bun/issues/9099 + TestBuilder.command`nosuch_cmd_zz 2>&1`.exitCode(1).stdout(notFound).quiet().runAsTest("2>&1 quiet"); + + TestBuilder.command`nosuch_cmd_zz 1>&2`.exitCode(1).stderr(notFound).testMini().runAsTest("1>&2"); + + TestBuilder.command`nosuch_cmd_zz 2>&1 | cat`.stdout(notFound).testMini().runAsTest("2>&1 into a pipe"); + + TestBuilder.command`echo "[$(nosuch_cmd_zz 2>&1)]"` + .stdout(`[${notFound.trimEnd()}]\n`) + .testMini() + .runAsTest("2>&1 in a command substitution"); + + TestBuilder.command`nosuch_cmd_zz 2> /dev/null || echo fallback` + .stdout("fallback\n") + .testMini() + .runAsTest("2> /dev/null"); + + TestBuilder.command`nosuch_a_zz 2> err.txt; nosuch_b_zz` + .exitCode(1) + .stderr("bun: command not found: nosuch_b_zz\n") + .fileEquals("err.txt", "bun: command not found: nosuch_a_zz\n") + .runAsTest("the redirect is for one command"); + + TestBuilder.command`./missing_path_zz 2> err.txt` + .exitCode(1) + .fileEquals("err.txt", "bun: command not found: ./missing_path_zz\n") + .runAsTest("a path that does not exist"); + + TestBuilder.command`$EMPTY_ZZ > made.txt` + .fileEquals("made.txt", "") + .testMini() + .runAsTest("no command name: the file is made"); + + TestBuilder.command`false && nosuch_cmd_zz 2> never.txt` + .ensureTempDir() + .exitCode(1) + .doesNotExist("never.txt") + .runAsTest("a command that is skipped makes no file"); + + // The message of each of these is compared with the same command and no redirect. + async function messageFollowsRedirect(dir: string, run: (redirect: { raw: string }) => ReturnType) { + const plain = await run({ raw: "" }).cwd(dir).quiet(); + const redirected = await run({ raw: "2> err.txt" }).cwd(dir).quiet(); + expect(plain.stderr.toString()).not.toBe(""); + expect({ + stdout: redirected.stdout.toString(), + stderr: redirected.stderr.toString(), + file: await Bun.file(join(dir, "err.txt")).text(), + exitCode: redirected.exitCode, + }).toEqual({ stdout: "", stderr: "", file: plain.stderr.toString(), exitCode: plain.exitCode }); + } + + test.skipIf(isWindows)("a file with no x bit", async () => { + using dir = tempDir("shell-redirect-noexec", { "noexec.sh": "#!/bin/sh\necho hi\n" }); + chmodSync(join(String(dir), "noexec.sh"), 0o644); + await messageFollowsRedirect(String(dir), redirect => $`./noexec.sh ${redirect}`); + }); + + // The spawn fails after the redirect target was opened. + test.skipIf(isWindows)("a script with an interpreter that does not exist", async () => { + using dir = tempDir("shell-redirect-shebang", { "badinterp.sh": "#!/nonexistent_dir_zz/interp\necho hi\n" }); + chmodSync(join(String(dir), "badinterp.sh"), 0o755); + await messageFollowsRedirect(String(dir), redirect => $`./badinterp.sh ${redirect}`); + }); + + test.skipIf(!isWindows)("a batch file that gets an argument with a cmd.exe special character", async () => { + using dir = tempDir("shell-redirect-batch", { "say.bat": "@echo ran> ran.txt\r\n" }); + await messageFollowsRedirect(String(dir), redirect => $`./say.bat ${"a&b"} ${redirect}`); + expect(await Bun.file(join(String(dir), "ran.txt")).exists()).toBe(false); + }); + + test("2> ${buffer}", async () => { + const buffer = new Uint8Array(64); + const { stdout, stderr, exitCode } = await $`nosuch_cmd_zz 2> ${buffer}`.quiet(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + buffer: Buffer.from(buffer).toString().replace(/\0+$/, ""), + exitCode, + }).toEqual({ stdout: "", stderr: "", buffer: notFound, exitCode: 1 }); + }); + + test("2> ${Bun.file(fd)}", async () => { + using dir = tempDir("shell-redirect-fd", {}); + const path = join(String(dir), "err.txt"); + const fd = openSync(path, "w"); + try { + const { stdout, stderr, exitCode } = await $`nosuch_cmd_zz 2> ${Bun.file(fd)}`.quiet(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + file: readFileSync(path, "utf8"), + exitCode, + }).toEqual({ stdout: "", stderr: "", file: notFound, exitCode: 1 }); + // The shell does not close the fd of the caller. + expect(fstatSync(fd).isFile()).toBe(true); + } finally { + closeSync(fd); + } + }); + + // bash is silent when it cannot write the message. The script continues. + test.skipIf(!isLinux)("a message that cannot be written to the target", async () => { + const { stdout, stderr, exitCode } = await $`nosuch_cmd_zz 2> /dev/full || echo fallback`.quiet(); + expect({ stdout: stdout.toString(), stderr: stderr.toString(), exitCode }).toEqual({ + stdout: "fallback\n", + stderr: "", + exitCode: 0, + }); + }); + + describe("a redirect that fails is reported, and the command is not searched for", () => { + // A builtin reports the same failure of the same redirect. + async function sameAsBuiltin( + run: (name: string) => ReturnType, + check: (stderr: string) => void = () => {}, + ) { + using dir = tempDir("shell-redirect-fails", {}); + const builtin = await run("echo").cwd(String(dir)).quiet(); + const missing = await run("nosuch_cmd_zz").cwd(String(dir)).quiet(); + check(builtin.stderr.toString()); + expect({ + stdout: missing.stdout.toString(), + stderr: missing.stderr.toString().replace("nosuch_cmd_zz", "echo"), + exitCode: missing.exitCode, + }).toEqual({ stdout: "", stderr: builtin.stderr.toString(), exitCode: 1 }); + } + + test("< file that does not exist", async () => { + await sameAsBuiltin( + name => $`${name} < missing_input_zz`, + stderr => expect(stderr).toContain("missing_input_zz"), + ); + }); + + test("2> file in a directory that does not exist", async () => { + await sameAsBuiltin( + name => $`${name} 2> missing_dir_zz/err.txt`, + stderr => expect(stderr).toContain("missing_dir_zz/err.txt"), + ); + }); + + test("> with no target", async () => { + await sameAsBuiltin( + name => $`${name} > $EMPTY_ZZ`, + stderr => expect(stderr).toBe("bun: ambiguous redirect: at `echo`\n"), + ); + }); + + test("no command name", async () => { + using dir = tempDir("shell-redirect-fails", {}); + const builtin = await $`echo < missing_input_zz`.cwd(String(dir)).quiet(); + const { stdout, stderr, exitCode } = await $`$EMPTY_ZZ < missing_input_zz`.cwd(String(dir)).quiet(); + expect({ stdout: stdout.toString(), stderr: stderr.toString(), exitCode }).toEqual({ + stdout: "", + stderr: builtin.stderr.toString(), + exitCode: 1, + }); + }); + + // An external command used to abort the process on this target. + test("a JS object that a redirect cannot use", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { $ } from "bun"; + const settle = p => p.then(r => "resolved " + r.exitCode, e => "rejected " + e.message); + const out = []; + for (const name of ["echo", process.execPath, "nosuch_cmd_zz"]) { + out.push(await settle($\`\${name} --version > \${new ReadableStream()}\`.quiet())); + out.push(await settle($\`\${name} --version > \${new Blob(["x"])}\`.quiet())); + } + console.log(JSON.stringify(out, null, 2)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const stream = "rejected Unknown JS value used in shell: [object ReadableStream]"; + expect({ result: JSON.parse(stdout || "null"), stderr, exitCode }).toEqual({ + result: [ + stream, + "rejected Cannot redirect stdout/stderr to an immutable blob. Expected a file", + stream, + "rejected Blobs are immutable, and cannot be used for stdout/stderr", + stream, + "rejected Blobs are immutable, and cannot be used for stdout/stderr", + ], + stderr: "", + exitCode: 0, + }); + }); + }); + }); + + // `2>&1` and `1>&2` of a command that runs, in each place that gives the + // command a different stdout and stderr. + describe("a command that runs", () => { + const lsError = "ls: nosuch_dir_zz: No such file or directory\n"; + const both = "console.log('out'); console.error('err')"; + + TestBuilder.command`ls nosuch_dir_zz 2>&1`.exitCode(1).stdout(lsError).testMini().runAsTest("builtin 2>&1"); + TestBuilder.command`ls nosuch_dir_zz 2>&1`.exitCode(1).stdout(lsError).quiet().runAsTest("builtin 2>&1 quiet"); + TestBuilder.command`echo hi 1>&2`.stderr("hi\n").testMini().runAsTest("builtin 1>&2"); + TestBuilder.command`echo hi 1>&2`.stderr("hi\n").quiet().runAsTest("builtin 1>&2 quiet"); + TestBuilder.command`ls nosuch_dir_zz 2>&1 | cat`.stdout(lsError).testMini().runAsTest("builtin 2>&1 into a pipe"); + TestBuilder.command`echo hi 1>&2 | cat`.stderr("hi\n").testMini().runAsTest("builtin 1>&2 away from a pipe"); + TestBuilder.command`echo "[$(ls nosuch_dir_zz 2>&1)]"` + .stdout(`[${lsError.trimEnd()}]\n`) + .testMini() + .runAsTest("builtin 2>&1 in a command substitution"); + + TestBuilder.command`${BUN} -e ${both} 2>&1`.stdout("out\nerr\n").testMini().runAsTest("external 2>&1"); + TestBuilder.command`${BUN} -e ${both} 1>&2`.stderr("out\nerr\n").testMini().runAsTest("external 1>&2"); + TestBuilder.command`${BUN} -e ${both} 2>&1 | cat` + .stdout("out\nerr\n") + .testMini() + .runAsTest("external 2>&1 into a pipe"); + TestBuilder.command`${BUN} -e ${both} 1>&2 | cat` + .stderr("out\nerr\n") + .testMini() + .runAsTest("external 1>&2 away from a pipe"); + TestBuilder.command`echo "[$(${BUN} -e ${both} 2>&1)]"` + .stdout("[out\nerr]\n") + .testMini() + .runAsTest("external 2>&1 in a command substitution"); + + TestBuilder.command`ls nosuch_dir_zz 2>> err.txt` + .file("err.txt", "pre\n") + .exitCode(1) + .fileEquals("err.txt", "pre\n" + lsError) + .runAsTest("builtin 2>>"); + TestBuilder.command`${BUN} -e ${both} 2>> err.txt` + .file("err.txt", "pre\n") + .stdout("out\n") + .fileEquals("err.txt", "pre\nerr\n") + .runAsTest("external 2>>"); + TestBuilder.command`ls nosuch_dir_zz &> both.txt` + .exitCode(1) + .fileEquals("both.txt", lsError) + .testMini() + .runAsTest("builtin &> with output on stderr"); + TestBuilder.command`echo hi &> both.txt` + .fileEquals("both.txt", "hi\n") + .testMini() + .runAsTest("builtin &> with output on stdout"); + TestBuilder.command`${BUN} -e ${both} &> both.txt` + .fileEquals("both.txt", "out\nerr\n") + .testMini() + .runAsTest("external &>"); + + test.each([ + ["external", (file: Blob) => $`${BUN} -e ${"console.log('hi')"} > ${file}`], + ["builtin", (file: Blob) => $`echo hi > ${file}`], + ] as const)("%s > Bun.file(fd)", async (_name, run) => { + using dir = tempDir("shell-redirect-fd", {}); + const path = join(String(dir), "out.txt"); + const fd = openSync(path, "w"); + try { + const { stdout, stderr, exitCode } = await run(Bun.file(fd)).quiet(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + file: readFileSync(path, "utf8"), + exitCode, + }).toEqual({ stdout: "", stderr: "", file: "hi\n", exitCode: 0 }); + } finally { + closeSync(fd); + } + }); + }); }); describe("pwd", async () => { diff --git a/test/js/bun/shell/exec.test.ts b/test/js/bun/shell/exec.test.ts index b7c272197124..b2ce8eecb063 100644 --- a/test/js/bun/shell/exec.test.ts +++ b/test/js/bun/shell/exec.test.ts @@ -16,6 +16,18 @@ describe("bun exec", () => { .stderr("bun: command not found: sldkfjslkdjflksdjflj\n") .runAsTest("it works on command fail"); + TestBuilder.command`${BUN} exec ${"sldkfjslkdjflksdjflj 2> err.txt"}` + .env(bunEnv) + .exitCode(1) + .fileEquals("err.txt", "bun: command not found: sldkfjslkdjflksdjflj\n") + .runAsTest("a command that fails to start follows 2> file"); + + TestBuilder.command`${BUN} exec ${"sldkfjslkdjflksdjflj 2>&1"}` + .env(bunEnv) + .exitCode(1) + .stdout("bun: command not found: sldkfjslkdjflksdjflj\n") + .runAsTest("a command that fails to start follows 2>&1"); + TestBuilder.command`${BUN} exec` .env(bunEnv) .stdout( diff --git a/test/js/bun/shell/leak.test.ts b/test/js/bun/shell/leak.test.ts index 79b0e33facbe..15e98de9d9f4 100644 --- a/test/js/bun/shell/leak.test.ts +++ b/test/js/bun/shell/leak.test.ts @@ -171,6 +171,25 @@ describe.concurrent("fd leak", () => { memLeakTest(...args); }); + // The shell opens the redirect target of a command that does not start, and + // writes its own message to it. + fdLeakTest( + "redirect_file_command_not_found", + () => + TestBuilder.command`nosuch_cmd_zz 2> err.txt` + .exitCode(1) + .fileEquals("err.txt", "bun: command not found: nosuch_cmd_zz\n"), + 200, + ); + fdLeakTest( + "redirect_file_both_command_not_found", + () => + TestBuilder.command`nosuch_cmd_zz &> err.txt` + .exitCode(1) + .fileEquals("err.txt", "bun: command not found: nosuch_cmd_zz\n"), + 200, + ); + // Use text of this file so its big enough to cause a leak memLeakTest("ArrayBuffer", () => TestBuilder.command`cat ${import.meta.filename} > ${new ArrayBuffer(128)}`, 100); memLeakTest("Buffer", () => TestBuilder.command`cat ${import.meta.filename} > ${Buffer.alloc(128)}`, 100); diff --git a/test/js/bun/shell/shell-worker-terminate-leak.test.ts b/test/js/bun/shell/shell-worker-terminate-leak.test.ts index 834015b1cf95..ec6caa15c400 100644 --- a/test/js/bun/shell/shell-worker-terminate-leak.test.ts +++ b/test/js/bun/shell/shell-worker-terminate-leak.test.ts @@ -106,7 +106,7 @@ test.skipIf(!isASAN || isWindows)( test.skipIf(!isASAN || isWindows)( "worker.terminate() with a live Bun.$ file redirect does not leak the redirection fd", async () => { - // `> out.txt` opens a CowFd on the Cmd that only Cmd::deinit releases. + // `> out.txt` opens an fd that the Cmd owns. await runTerminateScenario(` const running = Bun.$\`sh -c "echo ok > flag.txt && exec sleep 100" > out.txt\`.nothrow(); (async () => { @@ -183,6 +183,60 @@ test.skipIf(!isASAN || isWindows)( LEAK_TEST_TIMEOUT, ); +test.skipIf(!isLinux)( + "worker.terminate() with a live Bun.$ file redirect closes the redirection fd", + async () => { + // A leak checker does not see an fd that stays open, so read /proc. + using dir = tempDir("shell-worker-terminate-fd", { + "main.ts": ` + import { readdirSync, readlinkSync } from "node:fs"; + const holdsOut = () => + readdirSync("/proc/self/fd").some(fd => { + try { + return readlinkSync("/proc/self/fd/" + fd).endsWith("/out.txt"); + } catch { + return false; + } + }); + const worker = new Worker(new URL("./worker.ts", import.meta.url).href); + const { promise, resolve, reject } = Promise.withResolvers(); + worker.onmessage = resolve; + worker.onerror = e => reject(e.error ?? e.message); + await promise; + const before = holdsOut(); + await worker.terminate(); + const deadline = Date.now() + 20_000; + while (holdsOut() && Date.now() < deadline) await Bun.sleep(10); + console.log(JSON.stringify({ before, after: holdsOut() })); + `, + "worker.ts": ` + const running = Bun.$\`sh -c "echo ok > flag.txt && exec sleep 100" > out.txt\`.nothrow(); + (async () => { + await running; + })(); + while (!(await Bun.file("flag.txt").exists())) { + await Bun.sleep(5); + } + postMessage("ok"); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ before: true, after: false }) + "\n", + stderr: "", + exitCode: 0, + }); + }, + 60_000, +); + test.skipIf(!isLinux)( "worker.terminate() kills a live Bun.$ subprocess", async () => { From 0f3ceaa512ebdbef877ed3076b3da95818921f58 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 16 Oct 2026 17:58:37 +0000 Subject: [PATCH 2/5] shell: apply a command's redirect before the command is searched for A command now performs its redirect on its own streams in one step, Cmd::apply_redirect, before builtin lookup and the PATH search. A builtin, the spawn of a child process and the shell's own messages about the command all use those streams. The message about a command that does not start (not found, a refused batch-file argument, a failed spawn) now goes where the redirect sends stderr, and the redirect target is made. A redirect that fails is reported and the command is not searched for. A command with no name still performs its redirect. Builtin::init_redirections, Cmd::init_subproc_redirections, CowFd, BuiltinIO, BuiltinInput and RedirectFlags::redirects_elsewhere are gone. --- src/runtime/shell/Builtin.rs | 628 ++------------------ src/runtime/shell/IO.rs | 481 ++++++++++++++-- src/runtime/shell/builtin/basename.rs | 6 +- src/runtime/shell/builtin/cat.rs | 33 +- src/runtime/shell/builtin/cd.rs | 4 +- src/runtime/shell/builtin/cp.rs | 12 +- src/runtime/shell/builtin/dirname.rs | 6 +- src/runtime/shell/builtin/echo.rs | 6 +- src/runtime/shell/builtin/export.rs | 6 +- src/runtime/shell/builtin/ls.rs | 12 +- src/runtime/shell/builtin/mkdir.rs | 12 +- src/runtime/shell/builtin/mv.rs | 6 +- src/runtime/shell/builtin/pwd.rs | 12 +- src/runtime/shell/builtin/rm.rs | 44 +- src/runtime/shell/builtin/seq.rs | 8 +- src/runtime/shell/builtin/touch.rs | 12 +- src/runtime/shell/builtin/which.rs | 16 +- src/runtime/shell/builtin/yes.rs | 29 +- src/runtime/shell/interpreter.rs | 49 +- src/runtime/shell/mod.rs | 2 +- src/runtime/shell/states/Cmd.rs | 795 +++++++++++++++----------- src/runtime/shell/subproc.rs | 4 +- src/runtime/webcore/Body.rs | 23 - src/shell_parser/parse.rs | 30 +- test/js/bun/shell/bunshell.test.ts | 81 ++- 25 files changed, 1137 insertions(+), 1180 deletions(-) diff --git a/src/runtime/shell/Builtin.rs b/src/runtime/shell/Builtin.rs index 24e6dfb90ca4..1f40be35faa8 100644 --- a/src/runtime/shell/Builtin.rs +++ b/src/runtime/shell/Builtin.rs @@ -1,30 +1,21 @@ //! A builtin command runs inside a `Cmd` state node. The builtin stores the //! `NodeId` of its owning Cmd and every method takes `&Interpreter`. -use bun_collections::VecExt; -use bun_jsc::PinnedArrayBuffer; use core::ffi::c_char; -use std::sync::Arc; use crate::shell::ExitCode; -use crate::shell::ast; -use crate::shell::interpreter::{ - Interpreter, NodeId, OutputNeedsIOSafeGuard, ParseError, is_pollable_from_mode, shell_openat, -}; -use crate::shell::io::{InKind, OutFd, OutKind}; -use crate::shell::io_reader::IOReader; -use crate::shell::io_writer::{self, IOWriter}; +use crate::shell::interpreter::{Interpreter, NodeId, ParseError}; +use crate::shell::io::{CmdIn, CmdOut}; +use crate::shell::io_writer; use crate::shell::states::cmd::{Cmd, CmdState}; use crate::shell::yield_::Yield; +/// The streams of a builtin are those of its `Cmd`: `Cmd::io`. pub(crate) struct Builtin { pub(crate) kind: Kind, /// argv[1..] as NUL-terminated strings (argv[0] is the builtin name). /// Points into the Cmd's `args` storage. pub args: Vec<*const c_char>, - pub(crate) stdin: BuiltinInput, - pub(crate) stdout: BuiltinIO, - pub(crate) stderr: BuiltinIO, /// Scratch for `fmt_error_arena`. One outstanding error string at a time. pub(crate) err_buf: Vec, pub(crate) impl_: Impl, @@ -224,224 +215,12 @@ pub(crate) enum IoKind { Stderr, } -// ────────────────────────────────────────────────────────────────────────── -// BuiltinIO -// -// Distinct from `IO::OutKind` because builtins can target ArrayBuffer/Blob -// JS objects (`> ${buf}`) and accumulate into a per-builtin `.buf` when the -// Cmd's IO is `.pipe`. The `.buf` arm is reshaped in the NodeId port: instead -// of a local Vec flushed in `done()`, `write_no_io` appends straight to the -// shell env's captured buffer (one less copy). The variant carries its flush -// target so `2>&1` (which makes `stderr` a shallow copy of `stdout`) routes -// stderr writes to `buffered_stdout`. -// ────────────────────────────────────────────────────────────────────────── - -/// One output stream of a builtin (stdout or stderr). -pub(crate) enum BuiltinIO { - /// Async writer (real fd). `needs_io()` returns Some. - Fd(OutFd), - /// Captured pipe — writes go to the shell env's `_buffered_{stdout,stderr}`. - /// Writes go through immediately (see module doc). - /// The payload names which shell-env bytelist to append to — set at - /// `from_out_kind` and copied verbatim by `dup_ref` so `2>&1` keeps - /// stderr aimed at stdout's buffer. - Buf(IoKind), - ArrayBuf { - buf: PinnedArrayBuffer, - i: u32, - }, - Blob(Arc), - Ignore, -} - -/// Input stream of a builtin. -pub(crate) enum BuiltinInput { - Fd(Arc), - ArrayBuf { buf: PinnedArrayBuffer }, - Blob(Arc), - Ignore, -} - -/// Refcounted wrapper around a `webcore.Blob`. `Arc` provides the refcount; -/// `Drop` runs `Blob::deinit`. -pub(crate) struct BuiltinBlob { - pub(crate) blob: crate::webcore::Blob, -} -// `BuiltinBlob` is auto-`Send + Sync`: its sole field is `webcore::Blob`, -// which already asserts `Send + Sync`. No `unsafe impl` needed. -const _: fn() = || { - fn assert() {} - assert::(); -}; - -impl BuiltinIO { - /// From the Cmd's IO::OutKind. `Arc::clone` (via `OutFd: Clone`) bumps - /// the `IOWriter` refcount; `Drop` decrements it symmetrically. `target` - /// is the shell-env bytelist this stream flushes to (Stdout or Stderr). - fn from_out_kind(ok: &OutKind, target: IoKind) -> BuiltinIO { - match ok { - OutKind::Fd(fd) => BuiltinIO::Fd(fd.clone()), - OutKind::Pipe => BuiltinIO::Buf(target), - OutKind::Ignore => BuiltinIO::Ignore, - } - } - - /// Bump refcounts and return a shallow copy. Only reachable from the - /// `duplicate_out` path, which fires before any `.jsbuf` redirect, so - /// `ArrayBuf`/`Blob` are unreachable here. The `Buf` target is copied - /// verbatim so stderr writes accumulate in (and flush from) stdout's - /// buffer; that aliasing is the carried `IoKind`. - fn dup_ref(&self) -> BuiltinIO { - match self { - BuiltinIO::Fd(fd) => BuiltinIO::Fd(fd.clone()), - BuiltinIO::Buf(target) => BuiltinIO::Buf(*target), - BuiltinIO::Ignore => BuiltinIO::Ignore, - BuiltinIO::Blob(b) => BuiltinIO::Blob(Arc::clone(b)), - BuiltinIO::ArrayBuf { .. } => { - unreachable!("duplicate_out precedes jsbuf redirects") - } - } - } - - #[inline] - pub(crate) fn needs_io(&self) -> Option { - match self { - BuiltinIO::Fd(_) => Some(OutputNeedsIOSafeGuard::OutputNeedsIo), - _ => None, - } - } - - /// Body of [`Builtin::write_no_io`] with the Cmd split-borrow already - /// performed by the caller. Exists so builtins whose payload lives in - /// `Builtin.impl_` (disjoint from `stdout`/`stderr`) can write a borrowed - /// slice without an intermediate heap clone. - /// - /// # Safety - /// `shell` must point to the live `ShellExecEnv` owning this builtin - /// (i.e. `cmd.base.shell`); only dereferenced for the [`BuiltinIO::Buf`] - /// arm. - pub(crate) unsafe fn write_no_io_to( - &mut self, - shell: *mut crate::shell::interpreter::ShellExecEnv, - buf: &[u8], - ) -> bun_sys::Result { - if buf.is_empty() { - return Ok(0); - } - match self { - BuiltinIO::Fd(_) => { - panic!("write_no_io called on fd output; caller must check needs_io()") - } - BuiltinIO::Buf(target) => { - // Writes go straight through to `buffered_{stdout,stderr}`; - // `target` is the destination identity, fixed at construction - // and preserved across `dup_ref` so `2>&1` lands in stdout's - // bytelist. - // SAFETY: caller contract — shell env outlives the Cmd node - // (single-threaded); `captured` points into a live - // `ShellExecEnv` Bufio. - unsafe { - let captured = match *target { - IoKind::Stdout => (*shell).buffered_stdout(), - IoKind::Stderr => (*shell).buffered_stderr(), - }; - (*captured).append_slice(buf) - }; - Ok(buf.len()) - } - BuiltinIO::ArrayBuf { buf: arraybuf, i } => { - // `len = buf.len` stays usize so `i + len > byte_len` is - // computed at usize width and cannot overflow; only the - // stored cursor is u32. - let idx = *i as usize; - let total = arraybuf.byte_len; - if idx >= total { - return Err(bun_sys::Error::from_code( - bun_sys::E::ENOSPC, - bun_sys::Tag::write, - )); - } - let write_len = (total - idx).min(buf.len()); - let dst = &mut arraybuf.slice_mut()[idx..idx + write_len]; - dst.copy_from_slice(&buf[..write_len]); - *i = i.saturating_add(write_len as u32); - Ok(write_len) - } - BuiltinIO::Blob(_) | BuiltinIO::Ignore => Ok(buf.len()), - } - } - - /// Queue `buf` on this stream's IOWriter and arrange for `child`'s - /// `on_io_writer_chunk` to fire when the chunk completes. Delegates to - /// `fd.writer.enqueue` passing `fd.captured` as the tee bytelist. - /// - /// `_safeguard` proves the caller checked `needs_io()`. - pub(crate) fn enqueue( - &mut self, - child: io_writer::ChildPtr, - buf: &[u8], - _safeguard: OutputNeedsIOSafeGuard, - ) -> Yield { - match self { - BuiltinIO::Fd(fd) => fd.writer.enqueue(child, fd.captured, buf), - _ => unreachable!("enqueue() on non-fd output; caller must check needs_io()"), - } - } - - /// Format with the optional `"{kind}: "` prefix and enqueue on the - /// underlying IOWriter. - pub(crate) fn enqueue_fmt( - &mut self, - child: io_writer::ChildPtr, - kind: Option, - args: core::fmt::Arguments<'_>, - _safeguard: OutputNeedsIOSafeGuard, - ) -> Yield { - match self { - BuiltinIO::Fd(fd) => fd.writer.enqueue_fmt_bltn(child, fd.captured, kind, args), - _ => unreachable!("enqueue_fmt() on non-fd output; caller must check needs_io()"), - } - } -} - -impl BuiltinInput { - fn from_in_kind(ik: &InKind) -> BuiltinInput { - match ik { - // `Arc::clone` bumps the IOReader refcount. - InKind::Fd(r) => BuiltinInput::Fd(Arc::clone(r)), - InKind::Ignore => BuiltinInput::Ignore, - } - } - - #[inline] - pub(crate) fn needs_io(&self) -> bool { - matches!(self, BuiltinInput::Fd(_)) - } -} - impl Builtin { #[inline] pub(crate) fn args_slice(&self) -> &[*const c_char] { &self.args } - /// `PinnedArrayBuffer::drop`'s unpin would write to a `JSC::ArrayBuffer` - /// impl the heap sweep already deleted; see - /// `ShellSubprocess::defuse_array_buffer_unpins`. VM-shutdown finalizer - /// only. - #[cfg(not(windows))] - pub(crate) fn defuse_array_buf_pins(&mut self) { - if let BuiltinInput::ArrayBuf { buf, .. } = &mut self.stdin { - buf.defuse(); - } - if let BuiltinIO::ArrayBuf { buf, .. } = &mut self.stdout { - buf.defuse(); - } - if let BuiltinIO::ArrayBuf { buf, .. } = &mut self.stderr { - buf.defuse(); - } - } - /// Borrow `argv[1..][idx]` as `&[u8]` (NUL excluded). /// /// Every entry in `self.args` borrows into the owning `Cmd`'s @@ -478,330 +257,34 @@ impl Builtin { bun_core::ZStr::from_cstr(unsafe { core::ffi::CStr::from_ptr(p) }) } - /// Construct a `Builtin` for `kind`, install it into the owning Cmd's - /// `exec` slot, then wire up file/jsbuf/`2>&1` redirections. Returns - /// `None` (meaning: caller should now call `Builtin::start`). A - /// `Some(yield)` return means setup wrote a failing error (or threw) and - /// the caller should propagate that yield instead. - pub(crate) fn init(interp: &Interpreter, cmd: NodeId, kind: Kind) -> Option { + /// Construct a `Builtin` for `kind` and install it into the owning Cmd's + /// `exec` slot. The caller then calls `Builtin::start`. + pub(crate) fn init(interp: &Interpreter, cmd: NodeId, kind: Kind) { use crate::shell::states::cmd::Exec; + let me = interp.as_cmd_mut(cmd); // Borrow argv[1..] as `*const c_char` into the Cmd's `args` storage. // The Cmd's `args: Vec>` are NUL-terminated by // `Cmd::transition_to_exec` before this is called. - let (args, stdin, stdout, stderr) = { - let me = interp.as_cmd(cmd); - let mut argv: Vec<*const c_char> = Vec::with_capacity(me.args.len().saturating_sub(1)); - for a in me.args.iter().skip(1) { - argv.push(a.as_ptr().cast::()); - } - // `Arc::clone` (inside `OutFd: Clone` / `InKind: Clone`) bumps - // the `IOWriter`/`IOReader` refcount; the builtin's `Drop` - // decrements it symmetrically. No double-deref. - ( - argv, - BuiltinInput::from_in_kind(&me.io.stdin), - BuiltinIO::from_out_kind(&me.io.stdout, IoKind::Stdout), - BuiltinIO::from_out_kind(&me.io.stderr, IoKind::Stderr), - ) - }; - - interp.as_cmd_mut(cmd).exec = Exec::Builtin(Box::new(Builtin { + let args = me + .args + .iter() + .skip(1) + .map(|a| a.as_ptr().cast::()) + .collect(); + me.exec = Exec::Builtin(Box::new(Builtin { kind, args, - stdin, - stdout, - stderr, err_buf: Vec::new(), impl_: Self::make_impl(kind), })); - - Self::init_redirections(interp, cmd, kind) - } - - /// Opens redirect files / wires ArrayBuffer & Blob targets / handles - /// `2>&1` (`duplicate_out`). - fn init_redirections(interp: &Interpreter, cmd: NodeId, kind: Kind) -> Option { - // `node` points into the AST arena which outlives every state node (see Cmd::next). - let node: &ast::Cmd = &*interp.as_cmd(cmd).node; - let redirect = node.redirect; - - match &node.redirect_file { - Some(ast::Redirect::Atom(_)) => { - // ── File redirect (`> path` / `< path` / `>> path` / `&> path`). - if interp.as_cmd(cmd).redirection_file.is_empty() { - return Some(Self::cmd_write_failing_error( - interp, - cmd, - format_args!("bun: ambiguous redirect: at `{}`\n", kind.as_str()), - )); - } - - // `redirection_file` was NUL-terminated by Expansion; build a - // `&ZStr` over it (path = bytes excluding the trailing NUL). - // Clone the path bytes so the - // `&mut interp` open call below doesn't overlap a borrow into - // the Cmd node. - let path_buf: Vec = { - let raw = &interp.as_cmd(cmd).redirection_file; - let len = raw.len().saturating_sub(1); - let mut v = raw[..len].to_vec(); - v.push(0); - v - }; - // SAFETY: `path_buf` ends in NUL by construction. - let path = bun_core::ZStr::from_slice_with_nul(&path_buf[..]); - let perm: bun_sys::Mode = 0o666; - let cwd_fd = Self::cwd(interp, cmd); - let evtloop = interp.event_loop; - - let mut pollable = false; - let mut is_socket = false; - let mut is_nonblocking = false; - - let redirfd: bun_sys::Fd = if redirect.stdin() { - match shell_openat(cwd_fd, path, redirect.to_flags(), perm) { - Err(e) => { - let sys = e.to_shell_system_error(); - return Some(Self::cmd_write_failing_error( - interp, - cmd, - format_args!( - "bun: {}: {}", - bstr::BStr::new(sys.message.byte_slice()), - bstr::BStr::new(path.as_bytes()), - ), - )); - } - Ok(f) => f, - } - } else { - let result = bun_io::open_for_writing_impl( - cwd_fd, - &path, - redirect.to_flags(), - perm, - &mut pollable, - &mut is_socket, - false, - &mut is_nonblocking, - (), - |_| {}, - is_pollable_from_mode, - shell_openat, - ); - match result { - Err(e) => { - let sys = e.to_shell_system_error(); - return Some(Self::cmd_write_failing_error( - interp, - cmd, - format_args!( - "bun: {}: {}", - bstr::BStr::new(sys.message.byte_slice()), - bstr::BStr::new(path.as_bytes()), - ), - )); - } - Ok(f) => { - #[cfg(windows)] - { - use bun_sys::FdExt as _; - match f.make_lib_uv_owned_for_syscall( - bun_sys::Tag::open, - bun_sys::ErrorCase::CloseOnFail, - ) { - Err(e) => { - let sys = e.to_shell_system_error(); - return Some(Self::cmd_write_failing_error( - interp, - cmd, - format_args!( - "bun: {}: {}", - bstr::BStr::new(sys.message.byte_slice()), - bstr::BStr::new(path.as_bytes()), - ), - )); - } - Ok(f2) => f2, - } - } - #[cfg(not(windows))] - { - f - } - } - } - }; - - let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); - if redirect.stdin() { - let r = IOReader::init(redirfd, evtloop); - r.set_interp(interp_ptr); - Self::of_mut(interp, cmd).stdin = BuiltinInput::Fd(r); - } - - if !redirect.stdout() && !redirect.stderr() { - return None; - } - - // Honor the `pollable` computed by `open_for_writing_impl` on - // POSIX so a FIFO/socket target (whose fd is now O_NONBLOCK) - // takes the pollable path; Windows keeps the async writer. - let redirect_writer = IOWriter::init( - redirfd, - io_writer::Flags { - pollable: if cfg!(windows) { true } else { pollable }, - nonblock: is_nonblocking, - is_socket, - ..Default::default() - }, - evtloop, - ); - redirect_writer.set_interp(interp_ptr); - - if redirect.stdout() { - let me = Self::of_mut(interp, cmd); - me.stdout = BuiltinIO::Fd(OutFd { - writer: Arc::clone(&redirect_writer), - captured: None, - }); - } - if redirect.stderr() { - let me = Self::of_mut(interp, cmd); - me.stderr = BuiltinIO::Fd(OutFd { - writer: redirect_writer, - captured: None, - }); - } - } - Some(ast::Redirect::JsBuf(jsbuf)) => { - // ── JS object redirect (`> ${arraybuf}` / `> ${blob}`). - let idx = jsbuf.idx as usize; - // Safe accessor — single `unsafe` deref lives in - // `Interpreter::global_this_ref`. - let Some(global) = interp - .global_this_ref() - .filter(|_| idx < interp.jsobjs.len()) - else { - interp.throw(crate::shell::ShellErr::Custom( - b"Invalid JS object reference in shell" - .to_vec() - .into_boxed_slice(), - )); - return Some(Yield::Failed(cmd)); - }; - let jsval = interp.jsobjs[idx]; - - if jsval.js_type().is_array_buffer_like() { - // Each slot gets its own pin + GC root; `None` has thrown OOM. - let root = || { - let buf = PinnedArrayBuffer::root(global, jsval); - if buf.is_none() { - let _ = global.throw_out_of_memory(); - } - buf - }; - let me = Self::of_mut(interp, cmd); - if redirect.stdin() { - let Some(buf) = root() else { - return Some(Yield::Failed(cmd)); - }; - me.stdin = BuiltinInput::ArrayBuf { buf }; - } - if redirect.stdout() { - let Some(buf) = root() else { - return Some(Yield::Failed(cmd)); - }; - me.stdout = BuiltinIO::ArrayBuf { buf, i: 0 }; - } - if redirect.stderr() { - let Some(buf) = root() else { - return Some(Yield::Failed(cmd)); - }; - me.stderr = BuiltinIO::ArrayBuf { buf, i: 0 }; - } - } else if let Some(body) = - crate::webcore::body::Value::from_request_or_response(jsval) - { - // SAFETY: returned a live JSC-owned `*mut Value` borrowed - // from a Response/Request wrapper. - let body = unsafe { &mut *body }; - let is_file_blob = matches!(body, crate::webcore::body::Value::Blob(b) - if !b.needs_to_read_file()); - if (redirect.stdout() || redirect.stderr()) && !is_file_blob { - let _ = global.throw(format_args!( - "Cannot redirect stdout/stderr to an immutable blob. Expected a file" - )); - return Some(Yield::Failed(cmd)); - } - let original_blob = body.use_(); - if !redirect.stdin() && !redirect.stdout() && !redirect.stderr() { - drop(original_blob); - return None; - } - let blob = Arc::new(BuiltinBlob { - blob: original_blob.dupe(), - }); - drop(original_blob); - let me = Self::of_mut(interp, cmd); - if redirect.stdin() { - me.stdin = BuiltinInput::Blob(Arc::clone(&blob)); - } - if redirect.stdout() { - me.stdout = BuiltinIO::Blob(Arc::clone(&blob)); - } - if redirect.stderr() { - me.stderr = BuiltinIO::Blob(blob); - } - } else if let Some(blob_ref) = jsval.as_class_ref::() { - if (redirect.stdout() || redirect.stderr()) && !blob_ref.needs_to_read_file() { - let _ = global.throw(format_args!( - "Cannot redirect stdout/stderr to an immutable blob. Expected a file" - )); - return Some(Yield::Failed(cmd)); - } - let theblob = Arc::new(BuiltinBlob { - blob: blob_ref.dupe(), - }); - let me = Self::of_mut(interp, cmd); - if redirect.stdin() { - me.stdin = BuiltinInput::Blob(theblob); - } else if redirect.stdout() { - me.stdout = BuiltinIO::Blob(theblob); - } else if redirect.stderr() { - me.stderr = BuiltinIO::Blob(theblob); - } - } else { - let _ = global.throw(format_args!( - "Unknown JS value used in shell: {}", - jsval.fmt_string(global) - )); - return Some(Yield::Failed(cmd)); - } - } - None if redirect.duplicate_out() => { - // `2>&1` (stderr=true,dup_out=true) → stderr := stdout - // `1>&2` (stdout=true,dup_out=true) → stdout := stderr - let me = Self::of_mut(interp, cmd); - if redirect.stdout() { - me.stderr = me.stdout.dup_ref(); - } - if redirect.stderr() { - me.stdout = me.stderr.dup_ref(); - } - } - None => {} - } - - None } - /// Sets the owning Cmd's state to - /// `WaitingWriteErr` and writes to the *Cmd's* `io.stderr` (not the - /// builtin's, which may already have been redirected). Hoisted here - /// because `init_redirections` and `Cmd::transition_to_exec` (the - /// "command not found" / spawn-error paths) are the only callers. + /// Writes a message of the shell about the command to the command's + /// stderr, and finishes the command with exit code 1. Before + /// `Cmd::apply_redirect` that is the stderr the command inherits: an + /// expansion error and a redirect that cannot be set up are reported + /// there. After it, it is the stderr that the redirect gave the command. pub(crate) fn cmd_write_failing_error( interp: &Interpreter, cmd: NodeId, @@ -810,31 +293,20 @@ impl Builtin { use std::io::Write as _; let mut buf = Vec::new(); let _ = buf.write_fmt(args); - if let Some(_safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { - // Only the `Fd` arm transitions state. + interp.as_cmd_mut(cmd).io.stderr.make_writable(interp); + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { interp.as_cmd_mut(cmd).state = CmdState::WaitingWriteErr; let child = io_writer::ChildPtr::new(cmd, io_writer::WriterTag::Cmd); - // SAFETY: `OutKind::Fd` guaranteed by `needs_io()`. - if let OutKind::Fd(fd) = &interp.as_cmd(cmd).io.stderr { - return fd.writer.enqueue(child, fd.captured, &buf); - } - unreachable!() - } - // No-IO path: append to the shell env's captured stderr and finish - // synchronously with exit 1 (Cmd::on_io_writer_chunk's behaviour). - if let OutKind::Pipe = &interp.as_cmd(cmd).io.stderr { - // SAFETY: single trampoline frame; no other borrow of the env's - // (or its parent's) stderr buffer is live. - let stderr = unsafe { - interp - .as_cmd_mut(cmd) - .base - .shell_mut() - .buffered_stderr_mut() - }; - stderr.append_slice(&buf); + return interp.as_cmd(cmd).io.stderr.enqueue(child, &buf, safeguard); } - let parent = interp.as_cmd(cmd).base.parent; + // No-IO path: the message goes to the captured buffer or the + // `> ${buffer}` target, and the command finishes synchronously + // (Cmd::on_io_writer_chunk's behaviour). + let me = interp.as_cmd_mut(cmd); + let shell = me.base.shell; + // SAFETY: `shell` is `me.base.shell`, live for the Cmd's lifetime. + let _ = unsafe { me.io.stderr.write_no_io_to(shell, &buf) }; + let parent = me.base.parent; interp.child_done(parent, cmd, 1) } @@ -872,15 +344,15 @@ impl Builtin { /// Returns the bytes available on stdin when it is *not* an async fd /// (arraybuf / piped buf / blob). pub(crate) fn read_stdin_no_io<'a>(interp: &'a Interpreter, cmd: NodeId) -> &'a [u8] { - match &Self::of(interp, cmd).stdin { - BuiltinInput::ArrayBuf { buf, .. } => buf.slice(), - BuiltinInput::Blob(b) => b.blob.shared_view(), - BuiltinInput::Fd(_) | BuiltinInput::Ignore => b"", + match &interp.as_cmd(cmd).io.stdin { + CmdIn::ArrayBuf(buf) => buf.slice(), + CmdIn::Blob(blob) => blob.shared_view(), + CmdIn::Fd(_) | CmdIn::File(_) | CmdIn::Js(_) | CmdIn::Ignore => b"", } } /// Write `buf` to stdout/stderr without going through IOWriter (the - /// stream is a captured buffer / arraybuffer / blob / /dev/null). + /// stream is a captured buffer / arraybuffer / nothing). /// /// Returns `Err(ENOSPC)` when an ArrayBuffer target is already full. /// **WARNING**: caller must have checked `needs_io() == None` first. @@ -893,16 +365,11 @@ impl Builtin { if buf.is_empty() { return Ok(0); } - // Split-borrow the Cmd so `shell` - // and the builtin's stdout/stderr are accessible simultaneously. let cmd_node = interp.as_cmd_mut(cmd); let shell = cmd_node.base.shell; - let crate::shell::states::cmd::Exec::Builtin(me) = &mut cmd_node.exec else { - panic!("Cmd {} is not running a builtin", cmd); - }; - let out: &mut BuiltinIO = match io_kind { - IoKind::Stdout => &mut me.stdout, - IoKind::Stderr => &mut me.stderr, + let out: &mut CmdOut = match io_kind { + IoKind::Stdout => &mut cmd_node.io.stdout, + IoKind::Stderr => &mut cmd_node.io.stderr, }; // SAFETY: `shell` is `cmd_node.base.shell`, live for the Cmd's lifetime. unsafe { out.write_no_io_to(shell, buf) } @@ -1075,22 +542,15 @@ impl Builtin { buf: &[u8], exit_code: crate::shell::ExitCode, ) -> Yield { - if let Some(safeguard) = Self::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { let child = io_writer::ChildPtr::new(cmd, io_writer::WriterTag::Builtin); - // Clone buf so the &mut on - // `stderr` doesn't overlap a borrow into `err_buf`. - let owned = buf.to_vec(); - return Self::of_mut(interp, cmd) - .stderr - .enqueue(child, &owned, safeguard); + return interp.as_cmd(cmd).io.stderr.enqueue(child, buf, safeguard); } let _ = Self::write_no_io(interp, cmd, IoKind::Stderr, buf); Self::done(interp, cmd, exit_code) } } -// Cleanup: every `Impl` variant owns its state via `Box`/`Vec`/`Arc`, and -// `BuiltinIO`/`BuiltinInput` hold `Arc` / `Arc` / -// `PinnedArrayBuffer` / `Arc` whose `Drop` already decrements -// the refcount. So cleanup is fully covered by `Drop` on `Box` -// (called from `Cmd::deinit`). No explicit deinit needed. +// Cleanup: every `Impl` variant owns its state via `Box`/`Vec`/`Arc`. So +// cleanup is fully covered by `Drop` on `Box` (called from +// `Cmd::deinit`). No explicit deinit needed. diff --git a/src/runtime/shell/IO.rs b/src/runtime/shell/IO.rs index 6ad8ac242ee3..af0a8f57558b 100644 --- a/src/runtime/shell/IO.rs +++ b/src/runtime/shell/IO.rs @@ -1,14 +1,23 @@ //! Carries stdin/stdout/stderr for a state node. //! //! `IO` is a plain `Clone` value; `IOReader`/`IOWriter` are `Arc`-refcounted. +//! A command turns the `IO` it inherits into a [`CmdIO`] and applies its +//! redirect to that. + +use std::sync::Arc; use bun_collections::VecExt; +use bun_jsc::{JSValue, JsResult, PinnedArrayBuffer}; +use bun_sys::Fd; -use crate::api::bun_spawn::stdio::{Capture, Stdio}; -use crate::shell::interpreter::OutputNeedsIOSafeGuard; +use crate::api::bun_spawn::stdio::{Capture, Dup2, Stdio}; +use crate::shell::builtin::{IoKind, Kind}; +use crate::shell::interpreter::{Interpreter, OutputNeedsIOSafeGuard, ShellExecEnv, shell_dup}; use crate::shell::io_reader::IOReader; -use crate::shell::io_writer::IOWriter; -use crate::shell::shell_body::subproc::ShellIO; +use crate::shell::io_writer::{self, IOWriter}; +use crate::shell::shell_body::subproc::{ShellIO, StdioKind}; +use crate::shell::yield_::Yield; +use crate::webcore; #[derive(Clone, Default)] pub(crate) struct IO { @@ -26,16 +35,6 @@ impl IO { size += self.stderr.memory_cost(); size } - - /// Maps the state-node IO triple onto - /// `subproc::Stdio` for [`ShellSubprocess::spawn_async`], and stashes the - /// owning `IOWriter` Arcs on `shellio` so [`PipeReader`]'s captured-writer - /// path can tee subprocess output back into the JS-side buffers. - pub(crate) fn to_subproc_stdio(&self, stdio: &mut [Stdio; 3], shellio: &mut ShellIO) { - stdio[0] = self.stdin.to_subproc_stdio(); - stdio[1] = self.stdout.to_subproc_stdio(&mut shellio.stdout); - stdio[2] = self.stderr.to_subproc_stdio(&mut shellio.stderr); - } } #[derive(Clone, Default)] @@ -66,27 +65,6 @@ pub(crate) struct OutFd { pub(crate) captured: Option<*mut Vec>, } -impl OutFd { - /// Mutably borrow the JS-side captured stdout/stderr buffer if configured. - /// - /// `captured` is a non-owning backref into `ShellExecEnv::_buffered_*` - /// (see field doc); the owning `ShellExecEnv` outlives every `Cmd`/builtin - /// that holds an `OutFd`. Localises the per-callsite raw deref so callers - /// can `if let Some(buf) = fd.captured_mut() { buf.extend_from_slice(...) }`. - /// - /// # Safety - /// Caller must ensure no other `&`/`&mut` to the target `Vec` is live - /// (including via the parent `ShellExecEnv`) for the returned borrow's - /// lifetime. The `(&self) -> &mut T` shape cannot encode this, hence - /// `unsafe fn`. - #[inline] - #[allow(clippy::mut_from_ref)] - pub(crate) unsafe fn captured_mut(&self) -> Option<&mut Vec> { - // SAFETY: caller contract — single-threaded shell, env outlives `self`. - self.captured.map(|p| unsafe { &mut *p }) - } -} - impl InKind { fn memory_cost(&self) -> usize { match self { @@ -94,13 +72,6 @@ impl InKind { InKind::Ignore => 0, } } - - fn to_subproc_stdio(&self) -> Stdio { - match self { - InKind::Fd(r) => Stdio::Fd(r.fd()), - InKind::Ignore => Stdio::Ignore, - } - } } impl OutFd { @@ -132,14 +103,394 @@ impl OutKind { _ => None, } } +} + +// ────────────────────────────────────────────────────────────────────────── +// CmdIO +// ────────────────────────────────────────────────────────────────────────── + +/// stdin, stdout and stderr of one command. `Cmd::init` builds it from the +/// `IO` that the command inherits. `Cmd::apply_redirect` then points the +/// redirected streams at their target. A builtin, a child process and the +/// shell's own messages about the command all use these streams. +pub(crate) struct CmdIO { + pub(crate) stdin: CmdIn, + pub(crate) stdout: CmdOut, + pub(crate) stderr: CmdOut, +} + +/// Where a command reads stdin from. +pub(crate) enum CmdIn { + /// The inherited stream, or a file that a builtin reads through an `IOReader`. + Fd(Arc), + /// `< file` for a command that is not a builtin. The child reads the fd. + File(bun_sys::File), + /// `< ${buffer}` for a builtin. + ArrayBuf(Box), + /// `< ${blob}` or `< ${response}` for a builtin. + Blob(Box), + /// `< ${object}` for a command that is not a builtin: the index of the + /// object in `Interpreter::jsobjs`. The spawn reads it. + Js(u32), + Ignore, +} + +/// Where a command writes stdout or stderr to. +pub(crate) enum CmdOut { + /// The inherited stream, or a file that a builtin writes through an `IOWriter`. + Fd(OutFd), + /// The captured buffer of the shell env: `_buffered_stdout` or `_buffered_stderr`. + Buf(IoKind), + /// `> file` for a command that is not a builtin. The fd stays blocking, as a child needs it. + File(bun_sys::File), + /// An fd that this stream does not own: the `File` of the other stream + /// (`&> file`), or the fd of a `Bun.file(fd)`. + BorrowedFd(Fd), + /// `> ${buffer}` for a builtin. + ArrayBuf(Box), + /// `> ${buffer}` for a command that is not a builtin: the index of the + /// buffer in `Interpreter::jsobjs`. The spawn pins it. + JsBuf(u32), + Ignore, +} + +pub(crate) struct ArrayBufSink { + pub(crate) buf: PinnedArrayBuffer, + /// Offset of the next write. + pub(crate) i: u32, +} + +/// A JS object that a redirect can name: `> ${object}` or `< ${object}`. +pub(crate) enum JsObject { + ArrayBuffer(JSValue), + Blob(&'static webcore::Blob), + Response(&'static webcore::Response), +} + +impl JsObject { + pub(crate) fn from_value(value: JSValue) -> Option { + if value.js_type().is_array_buffer_like() { + return Some(JsObject::ArrayBuffer(value)); + } + if let Some(blob) = value.as_class_ref::() { + return Some(JsObject::Blob(blob)); + } + value + .as_class_ref::() + .map(JsObject::Response) + } +} + +// `Cmd` is the largest state node. Its streams must not make it larger. +const _: () = assert!(size_of::() == size_of::()); +const _: () = assert!(size_of::() == 2 * size_of::()); + +impl From for CmdIO { + fn from(io: IO) -> CmdIO { + CmdIO { + stdin: match io.stdin { + InKind::Fd(reader) => CmdIn::Fd(reader), + InKind::Ignore => CmdIn::Ignore, + }, + stdout: CmdOut::inherited(io.stdout, IoKind::Stdout), + stderr: CmdOut::inherited(io.stderr, IoKind::Stderr), + } + } +} + +impl CmdIO { + /// Maps the streams onto the stdio of a child process. `shellio` keeps the + /// `IOWriter` of a captured stream, so that `PipeReader` can also write + /// what it reads to it. `redirect_stdout`/`redirect_stderr` get the pin of + /// a `> ${buffer}` target. + #[inline] + pub(crate) fn to_subproc_stdio( + &self, + interp: &Interpreter, + stdio: &mut [Stdio; 3], + shellio: &mut ShellIO, + redirect_stdout: &mut Option, + redirect_stderr: &mut Option, + ) -> JsResult<()> { + stdio[0] = self.stdin.to_subproc_stdio(interp)?; + stdio[1] = self.stdout.to_subproc_stdio( + interp, + StdioKind::Stdout, + &mut shellio.stdout, + redirect_stdout, + )?; + stdio[2] = if self.stderr.is_same_stream(&self.stdout) { + // `2>&1` or `1>&2`: the child gets the one stream on stdout. + Stdio::Dup2(Dup2 { + out: StdioKind::Stderr, + to: StdioKind::Stdout, + }) + } else { + self.stderr.to_subproc_stdio( + interp, + StdioKind::Stderr, + &mut shellio.stderr, + redirect_stderr, + )? + }; + Ok(()) + } + + /// `PinnedArrayBuffer::drop`'s unpin would write to a `JSC::ArrayBuffer` + /// impl the heap sweep already deleted; see + /// `ShellSubprocess::defuse_array_buffer_unpins`. VM-shutdown finalizer + /// only. + #[cfg(not(windows))] + pub(crate) fn defuse_array_buf_pins(&mut self) { + if let CmdIn::ArrayBuf(buf) = &mut self.stdin { + buf.defuse(); + } + for out in [&mut self.stdout, &mut self.stderr] { + if let CmdOut::ArrayBuf(sink) = out { + sink.buf.defuse(); + } + } + } +} + +impl CmdIn { + #[inline] + pub(crate) fn needs_io(&self) -> bool { + matches!(self, CmdIn::Fd(_)) + } + + fn to_subproc_stdio(&self, interp: &Interpreter) -> JsResult { + Ok(match self { + CmdIn::Fd(reader) => Stdio::Fd(reader.fd()), + CmdIn::File(file) => Stdio::Fd(file.handle()), + CmdIn::Js(idx) => { + let mut stdio = Stdio::Ignore; + // `Cmd::apply_redirect` stored the index after this lookup worked. + let Some((global, value)) = interp.js_object(*idx) else { + return Ok(stdio); + }; + match JsObject::from_value(value) { + Some(JsObject::ArrayBuffer(value)) => { + // The child gets the bytes that the buffer holds now. + // An empty buffer is EOF, as in `Stdio::extract_blob`. + if let Some(buf) = value.as_array_buffer(global) + && !buf.byte_slice().is_empty() + { + stdio = Stdio::Blob(webcore::blob::Any::from_owned_slice( + buf.byte_slice().to_vec(), + )); + } + } + Some(JsObject::Blob(blob)) => { + stdio.extract_blob(global, webcore::blob::Any::Blob(blob.dupe()), 0)?; + } + Some(JsObject::Response(response)) => { + let body = response.get_body_value(); + body.to_blob_if_possible(); + stdio.extract_blob(global, body.use_as_any_blob(), 0)?; + } + None => {} + } + stdio + } + CmdIn::ArrayBuf(_) | CmdIn::Blob(_) => { + unreachable!("the stdin of a builtin is not given to a child process") + } + CmdIn::Ignore => Stdio::Ignore, + }) + } +} + +impl CmdOut { + /// `target` is the shell-env buffer that a captured stream appends to. + fn inherited(out: OutKind, target: IoKind) -> CmdOut { + match out { + OutKind::Fd(fd) => CmdOut::Fd(fd), + OutKind::Pipe => CmdOut::Buf(target), + OutKind::Ignore => CmdOut::Ignore, + } + } + + /// The same stream again: for `2>&1` and `1>&2`, and for the second stream + /// of `&> file`. A `Buf` keeps its target, so stderr after `2>&1` appends + /// to the stdout buffer. + pub(crate) fn dup(&self) -> CmdOut { + match self { + CmdOut::Fd(fd) => CmdOut::Fd(fd.clone()), + CmdOut::Buf(target) => CmdOut::Buf(*target), + CmdOut::File(file) => CmdOut::BorrowedFd(file.handle()), + CmdOut::BorrowedFd(fd) => CmdOut::BorrowedFd(*fd), + CmdOut::JsBuf(idx) => CmdOut::JsBuf(*idx), + CmdOut::Ignore => CmdOut::Ignore, + CmdOut::ArrayBuf(_) => unreachable!("each stream pins the buffer for itself"), + } + } + + /// True when both are one stream: `>&` made one a copy of the other. + fn is_same_stream(&self, other: &CmdOut) -> bool { + match (self, other) { + (CmdOut::Fd(a), CmdOut::Fd(b)) => Arc::ptr_eq(&a.writer, &b.writer), + (CmdOut::Buf(a), CmdOut::Buf(b)) => a == b, + _ => false, + } + } + + #[inline] + pub(crate) fn needs_io(&self) -> Option { + match self { + CmdOut::Fd(_) => Some(OutputNeedsIOSafeGuard::OutputNeedsIo), + _ => None, + } + } + + /// The buffer that gets the output of a child when the pipe it wrote to + /// closes: the JS-side capture of an `Fd`, or the shell env's buffer. + pub(crate) fn capture_buf(&self, shell: &mut ShellExecEnv) -> Option<*mut Vec> { + match self { + CmdOut::Fd(fd) => fd.captured, + CmdOut::Buf(target) => Some(env_buf(shell, *target)), + _ => None, + } + } + + /// Writes `buf` without an `IOWriter`: to the captured buffer, to the + /// `> ${buffer}` target, or nowhere. + /// + /// Returns `Err(ENOSPC)` when an ArrayBuffer target is already full. + /// **WARNING**: caller must have checked `needs_io() == None` first. + /// + /// # Safety + /// `shell` must point to the live `ShellExecEnv` of the command that owns + /// this stream (`cmd.base.shell`); only dereferenced for [`CmdOut::Buf`]. + pub(crate) unsafe fn write_no_io_to( + &mut self, + shell: *mut ShellExecEnv, + buf: &[u8], + ) -> bun_sys::Result { + if buf.is_empty() { + return Ok(0); + } + match self { + CmdOut::Fd(_) => { + panic!("write_no_io called on fd output; caller must check needs_io()") + } + CmdOut::Buf(target) => { + // Writes go straight through to `buffered_{stdout,stderr}`. + // SAFETY: caller contract — shell env outlives the Cmd node + // (single-threaded); the buffer is a live `ShellExecEnv` Bufio. + unsafe { (*env_buf(&mut *shell, *target)).append_slice(buf) }; + Ok(buf.len()) + } + CmdOut::ArrayBuf(sink) => { + // `len = buf.len` stays usize so `i + len > byte_len` is + // computed at usize width and cannot overflow; only the + // stored cursor is u32. + let idx = sink.i as usize; + let total = sink.buf.byte_len; + if idx >= total { + return Err(bun_sys::Error::from_code( + bun_sys::E::ENOSPC, + bun_sys::Tag::write, + )); + } + let write_len = (total - idx).min(buf.len()); + let dst = &mut sink.buf.slice_mut()[idx..idx + write_len]; + dst.copy_from_slice(&buf[..write_len]); + sink.i = sink.i.saturating_add(write_len as u32); + Ok(write_len) + } + CmdOut::Ignore => Ok(buf.len()), + CmdOut::File(_) | CmdOut::BorrowedFd(_) | CmdOut::JsBuf(_) => { + unreachable!("a stream for a child process; see `make_writable`") + } + } + } - /// Retains the `IOWriter` Arc on - /// `shellio` so the subprocess's `PipeReader::captured_writer` can drain - /// captured bytes into it after the spawn returns. - fn to_subproc_stdio(&self, shellio: &mut Option>) -> Stdio { + /// Queue `buf` on this stream's IOWriter and arrange for `child`'s + /// `on_io_writer_chunk` to fire when the chunk completes. Delegates to + /// `fd.writer.enqueue` passing `fd.captured` as the tee bytelist. + /// + /// `_safeguard` proves the caller checked `needs_io()`. + pub(crate) fn enqueue( + &self, + child: io_writer::ChildPtr, + buf: &[u8], + _safeguard: OutputNeedsIOSafeGuard, + ) -> Yield { match self { - OutKind::Fd(val) => { - *shellio = Some(std::sync::Arc::clone(&val.writer)); + CmdOut::Fd(fd) => fd.writer.enqueue(child, fd.captured, buf), + _ => unreachable!("enqueue() on non-fd output; caller must check needs_io()"), + } + } + + /// Format with the optional `"{kind}: "` prefix and enqueue on the + /// underlying IOWriter. + pub(crate) fn enqueue_fmt( + &self, + child: io_writer::ChildPtr, + kind: Option, + args: core::fmt::Arguments<'_>, + _safeguard: OutputNeedsIOSafeGuard, + ) -> Yield { + match self { + CmdOut::Fd(fd) => fd.writer.enqueue_fmt_bltn(child, fd.captured, kind, args), + _ => unreachable!("enqueue_fmt() on non-fd output; caller must check needs_io()"), + } + } + + /// Makes a stream that `Cmd::apply_redirect` set up for a child process + /// into one that the shell itself can write to: the command did not start, + /// and the shell reports that on it. A target that cannot be written to + /// becomes `Ignore`. + pub(crate) fn make_writable(&mut self, interp: &Interpreter) { + let fd = match core::mem::replace(self, CmdOut::Ignore) { + // The `IOWriter` takes over the fd. + CmdOut::File(file) => Ok(file.into_raw()), + // The owner keeps its fd. The `IOWriter` closes a copy. + CmdOut::BorrowedFd(fd) => shell_dup(fd), + CmdOut::JsBuf(idx) => { + if let Some(buf) = interp + .js_object(idx) + .and_then(|(global, value)| PinnedArrayBuffer::root(global, value)) + { + *self = CmdOut::ArrayBuf(Box::new(ArrayBufSink { buf, i: 0 })); + } + return; + } + writable @ (CmdOut::Fd(_) | CmdOut::Buf(_) | CmdOut::ArrayBuf(_) | CmdOut::Ignore) => { + *self = writable; + return; + } + }; + let Ok(fd) = fd else { return }; + let writer = IOWriter::init( + fd, + io_writer::Flags { + pollable: cfg!(windows), + ..Default::default() + }, + interp.event_loop, + ); + writer.set_interp(interp.as_ctx_ptr()); + *self = CmdOut::Fd(OutFd { + writer, + captured: None, + }); + } + + /// The stdio that a child process gets for this stream. + #[inline] + fn to_subproc_stdio( + &self, + interp: &Interpreter, + kind: StdioKind, + shellio: &mut Option>, + redirect_buf: &mut Option, + ) -> JsResult { + Ok(match self { + CmdOut::Fd(val) => { + // `PipeReader::captured_writer` drains what it captures into it. + *shellio = Some(Arc::clone(&val.writer)); if let Some(cap) = val.captured { #[cfg(not(any(target_os = "linux", target_os = "android")))] let _ = cap; @@ -152,7 +503,7 @@ impl OutKind { // once the fd has been handed off to libuv, so the // sentinel compare here checks for that hand-off state. let fd = val.writer.fd(); - if fd != bun_sys::Fd::INVALID { + if fd != Fd::INVALID { Stdio::Fd(fd) } else { // Windows: fd was moved to libuv → inherit (libuv @@ -162,8 +513,34 @@ impl OutKind { } } } - OutKind::Pipe => Stdio::Pipe, - OutKind::Ignore => Stdio::Ignore, - } + CmdOut::Buf(_) => Stdio::Pipe, + CmdOut::File(file) => Stdio::Fd(file.handle()), + // `> ${Bun.stdout}` is the stdout that the child has already, as in `Stdio::extract_blob`. + CmdOut::BorrowedFd(fd) if *fd == kind.to_fd() => Stdio::Inherit, + CmdOut::BorrowedFd(fd) => Stdio::Fd(*fd), + CmdOut::JsBuf(idx) => { + // `Cmd::apply_redirect` stored the index after this lookup worked. + let Some((global, value)) = interp.js_object(*idx) else { + return Ok(Stdio::Ignore); + }; + let Some(buf) = PinnedArrayBuffer::root(global, value) else { + return Err(global.throw_out_of_memory()); + }; + *redirect_buf = Some(buf); + Stdio::Pipe + } + CmdOut::ArrayBuf(_) => { + unreachable!("the buffer of a builtin is not given to a child process") + } + CmdOut::Ignore => Stdio::Ignore, + }) + } +} + +/// The captured stdout or stderr of `shell`. +fn env_buf(shell: &mut ShellExecEnv, target: IoKind) -> *mut Vec { + match target { + IoKind::Stdout => shell.buffered_stdout(), + IoKind::Stderr => shell.buffered_stderr(), } } diff --git a/src/runtime/shell/builtin/basename.rs b/src/runtime/shell/builtin/basename.rs index dc408de88efc..3cacaad65436 100644 --- a/src/runtime/shell/builtin/basename.rs +++ b/src/runtime/shell/builtin/basename.rs @@ -34,11 +34,13 @@ impl Basename { }; Self::state_mut(interp, cmd).state = State::Done; - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).buf = buf; let owned = Self::state_mut(interp, cmd).buf.clone(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) + return interp + .as_cmd(cmd) + .io .stdout .enqueue(child, &owned, safeguard); } diff --git a/src/runtime/shell/builtin/cat.rs b/src/runtime/shell/builtin/cat.rs index 6a444713556d..077460ef4532 100644 --- a/src/runtime/shell/builtin/cat.rs +++ b/src/runtime/shell/builtin/cat.rs @@ -1,10 +1,11 @@ use std::sync::Arc; use crate::shell::ExitCode; -use crate::shell::builtin::{Builtin, BuiltinIO, BuiltinInput, BuiltinState, IoKind, Kind}; +use crate::shell::builtin::{Builtin, BuiltinState, IoKind, Kind}; use crate::shell::interpreter::{ FlagParser, Interpreter, NodeId, ParseFlagResult, parse_flags, shell_openat, unsupported_flag, }; +use crate::shell::io::{CmdIn, CmdOut}; use crate::shell::io_reader::{ChildPtr as ReaderChildPtr, IOReader, ReaderTag}; use crate::shell::io_writer::{ChildPtr, WriterTag}; use crate::shell::yield_::Yield; @@ -103,12 +104,10 @@ impl Cat { buf: &[u8], exit_code: ExitCode, ) -> Yield { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { Self::state_mut(interp, cmd).state = CatState::WaitingWriteErr; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, buf, safeguard); + return interp.as_cmd(cmd).io.stderr.enqueue(child, buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf); Builtin::done(interp, cmd, exit_code) @@ -136,7 +135,7 @@ impl Cat { Branch::Stdin => { // Stdin doesn't need IO (captured/ignored): read it all // synchronously and write straight to stdout. - let stdin_needs_io = Builtin::of(interp, cmd).stdin.needs_io(); + let stdin_needs_io = interp.as_cmd(cmd).io.stdin.needs_io(); if !stdin_needs_io { if let CatState::ExecStdin { in_done, .. } = &mut Self::state_mut(interp, cmd).state @@ -146,11 +145,9 @@ impl Cat { // Copy stdin bytes so the &mut on `stdout`/`write_no_io` // doesn't overlap a borrow of `stdin`. let buf = Builtin::read_stdin_no_io(interp, cmd).to_vec(); - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return interp.as_cmd(cmd).io.stdout.enqueue(child, &buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); return Builtin::done(interp, cmd, 0); @@ -159,8 +156,8 @@ impl Cat { // out of `stdin` so we hold no borrow of `interp` across // `start()` (which may re-enter via the raw interp backref). let interp_ptr: *mut Interpreter = interp.as_ctx_ptr(); - let reader = match &Builtin::of(interp, cmd).stdin { - BuiltinInput::Fd(r) => Arc::clone(r), + let reader = match &interp.as_cmd(cmd).io.stdin { + CmdIn::Fd(r) => Arc::clone(r), _ => unreachable!("needs_io() returned true"), }; reader.set_interp(interp_ptr); @@ -260,7 +257,7 @@ impl Cat { *st_errno = errno; let was_done = core::mem::replace(in_done, true); if !was_done { - if let BuiltinInput::Fd(r) = &Builtin::of(interp, cmd).stdin { + if let CmdIn::Fd(r) = &interp.as_cmd(cmd).io.stdin { r.remove_reader(rchild); } } @@ -320,14 +317,16 @@ impl Cat { remove: &mut bool, ) -> Yield { *remove = false; - let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io(); + let stdout_needs_io = interp.as_cmd(cmd).io.stdout.needs_io(); match &mut Self::state_mut(interp, cmd).state { CatState::ExecStdin { chunks_queued, .. } | CatState::ExecFilepathArgs { chunks_queued, .. } => { if let Some(safeguard) = stdout_needs_io { *chunks_queued += 1; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) + return interp + .as_cmd(cmd) + .io .stdout .enqueue(child, chunk, safeguard); } @@ -344,7 +343,7 @@ impl Cat { err: Option, ) -> Yield { let errno: ExitCode = err.map(|e| e.get_errno() as ExitCode).unwrap_or(0); - let stdout_needs_io = Builtin::of(interp, cmd).stdout.needs_io().is_some(); + let stdout_needs_io = interp.as_cmd(cmd).io.stdout.needs_io().is_some(); let mut cancel = false; let step = match &mut Self::state_mut(interp, cmd).state { CatState::ExecStdin { @@ -396,7 +395,7 @@ impl Cat { }; if cancel { let wchild = ChildPtr::new(cmd, WriterTag::Builtin); - if let BuiltinIO::Fd(fd) = &Builtin::of(interp, cmd).stdout { + if let CmdOut::Fd(fd) = &interp.as_cmd(cmd).io.stdout { fd.writer.cancel_chunks(wchild); } } diff --git a/src/runtime/shell/builtin/cd.rs b/src/runtime/shell/builtin/cd.rs index b7190ff9a99d..017259fae83f 100644 --- a/src/runtime/shell/builtin/cd.rs +++ b/src/runtime/shell/builtin/cd.rs @@ -102,9 +102,9 @@ impl Cd { args: core::fmt::Arguments<'_>, ) -> Yield { Self::state_mut(interp, cmd).state = State::WaitingIo; - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd).stderr.enqueue_fmt( + return interp.as_cmd(cmd).io.stderr.enqueue_fmt( child, Some(Kind::Cd), args, diff --git a/src/runtime/shell/builtin/cp.rs b/src/runtime/shell/builtin/cp.rs index 2c2c8b25644e..57e84c1952cd 100644 --- a/src/runtime/shell/builtin/cp.rs +++ b/src/runtime/shell/builtin/cp.rs @@ -337,13 +337,15 @@ impl OutputTaskVTable for Cp { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { // Stash so on_io_writer_chunk can route to the OutputTask state // machine and reclaim the box (stopgap for missing WriterTag). Self::state_mut(interp, cmd).output_queue.push_back(child); let childptr = ChildPtr::new(cmd, WriterTag::Builtin); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stderr .enqueue(childptr, errbuf, safeguard), ); @@ -365,12 +367,14 @@ impl OutputTaskVTable for Cp { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).output_queue.push_back(child); let childptr = ChildPtr::new(cmd, WriterTag::Builtin); let buf = output.slice().to_vec(); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stdout .enqueue(childptr, &buf, safeguard), ); diff --git a/src/runtime/shell/builtin/dirname.rs b/src/runtime/shell/builtin/dirname.rs index 32df36e5cb5c..aa4780bd398d 100644 --- a/src/runtime/shell/builtin/dirname.rs +++ b/src/runtime/shell/builtin/dirname.rs @@ -25,7 +25,7 @@ impl Dirname { return Self::fail(interp, cmd, b"usage: dirname string\n"); } - let stdout_needs_io = bltn.stdout.needs_io(); + let stdout_needs_io = interp.as_cmd(cmd).io.stdout.needs_io(); let mut buf = Vec::new(); for i in 0..argc { let path = bltn.arg_bytes(i); @@ -40,7 +40,9 @@ impl Dirname { Self::state_mut(interp, cmd).buf = buf; let owned = Self::state_mut(interp, cmd).buf.clone(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) + return interp + .as_cmd(cmd) + .io .stdout .enqueue(child, &owned, safeguard); } diff --git a/src/runtime/shell/builtin/echo.rs b/src/runtime/shell/builtin/echo.rs index 609caab921c2..80a2eda203c2 100644 --- a/src/runtime/shell/builtin/echo.rs +++ b/src/runtime/shell/builtin/echo.rs @@ -89,13 +89,11 @@ impl Echo { }; Self::state_mut(interp, cmd).output = output; - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).state = State::WaitingIo; let buf = Self::state_mut(interp, cmd).output.clone(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return interp.as_cmd(cmd).io.stdout.enqueue(child, &buf, safeguard); } let buf = Self::state_mut(interp, cmd).output.clone(); let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); diff --git a/src/runtime/shell/builtin/export.rs b/src/runtime/shell/builtin/export.rs index 8e03dac1ed17..0a451556353a 100644 --- a/src/runtime/shell/builtin/export.rs +++ b/src/runtime/shell/builtin/export.rs @@ -64,12 +64,10 @@ impl Export { buf.push(b'\n'); } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).state = State::WaitingIo; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return interp.as_cmd(cmd).io.stdout.enqueue(child, &buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); Builtin::done(interp, cmd, 0) diff --git a/src/runtime/shell/builtin/ls.rs b/src/runtime/shell/builtin/ls.rs index 635157a978cc..49ddf90fdd98 100644 --- a/src/runtime/shell/builtin/ls.rs +++ b/src/runtime/shell/builtin/ls.rs @@ -284,7 +284,7 @@ impl OutputTaskVTable for Ls { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { // Stash so on_io_writer_chunk can route to the OutputTask state // machine and reclaim the box (stopgap for missing WriterTag). if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -292,7 +292,9 @@ impl OutputTaskVTable for Ls { } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stderr .enqueue(childptr, errbuf, safeguard), ); @@ -314,14 +316,16 @@ impl OutputTaskVTable for Ls { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_queue.push_back(child); } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); let buf = output.slice().to_vec(); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stdout .enqueue(childptr, &buf, safeguard), ); diff --git a/src/runtime/shell/builtin/mkdir.rs b/src/runtime/shell/builtin/mkdir.rs index a50d8b9aca49..816ec690a610 100644 --- a/src/runtime/shell/builtin/mkdir.rs +++ b/src/runtime/shell/builtin/mkdir.rs @@ -192,7 +192,7 @@ impl OutputTaskVTable for Mkdir { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { // OutputTask has no `WriterTag` of its own (it is not directly // dispatchable as an IOWriter child), so the enqueue is tagged // `WriterTag::Builtin` and `child` is stashed on `output_queue`; @@ -203,7 +203,9 @@ impl OutputTaskVTable for Mkdir { } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stderr .enqueue(childptr, errbuf, safeguard), ); @@ -227,7 +229,7 @@ impl OutputTaskVTable for Mkdir { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { // See write_err — stash `child` so the chunk callback routes to // OutputTask::on_io_writer_chunk. if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -236,7 +238,9 @@ impl OutputTaskVTable for Mkdir { let childptr = ChildPtr::new(cmd, WriterTag::Builtin); let buf = output.slice().to_vec(); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stdout .enqueue(childptr, &buf, safeguard), ); diff --git a/src/runtime/shell/builtin/mv.rs b/src/runtime/shell/builtin/mv.rs index 2d143b92a882..2e308a31cb35 100644 --- a/src/runtime/shell/builtin/mv.rs +++ b/src/runtime/shell/builtin/mv.rs @@ -61,12 +61,10 @@ impl Mv { buf: &[u8], exit_code: ExitCode, ) -> Yield { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { Self::state_mut(interp, cmd).state = MvState::WaitingWriteErr { exit_code }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, buf, safeguard); + return interp.as_cmd(cmd).io.stderr.enqueue(child, buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf); Builtin::done(interp, cmd, exit_code) diff --git a/src/runtime/shell/builtin/pwd.rs b/src/runtime/shell/builtin/pwd.rs index 0fa42dbf8061..ccab5006ff15 100644 --- a/src/runtime/shell/builtin/pwd.rs +++ b/src/runtime/shell/builtin/pwd.rs @@ -29,14 +29,12 @@ impl Pwd { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { if !Builtin::of(interp, cmd).args_slice().is_empty() { let msg: &[u8] = b"pwd: too many arguments\n"; - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { Self::state_mut(interp, cmd).state = State::WaitingIo { kind: WaitKind::Stderr, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, msg, safeguard); + return interp.as_cmd(cmd).io.stderr.enqueue(child, msg, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, msg); return Builtin::done(interp, cmd, 1); @@ -47,14 +45,12 @@ impl Pwd { v.push(b'\n'); v }; - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).state = State::WaitingIo { kind: WaitKind::Stdout, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &cwd, safeguard); + return interp.as_cmd(cmd).io.stdout.enqueue(child, &cwd, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &cwd); Self::state_mut(interp, cmd).state = State::Done; diff --git a/src/runtime/shell/builtin/rm.rs b/src/runtime/shell/builtin/rm.rs index b8d5421f3097..8493e32d3f42 100644 --- a/src/runtime/shell/builtin/rm.rs +++ b/src/runtime/shell/builtin/rm.rs @@ -206,7 +206,7 @@ impl Rm { // re-borrowing `interp` mutably. let resolved_owned = resolved.to_vec(); if let Some(safeguard) = - Builtin::of(interp, cmd).stderr.needs_io() + interp.as_cmd(cmd).io.stderr.needs_io() { Self::state_mut(interp, cmd).state = RmState::ParseOpts { @@ -214,17 +214,15 @@ impl Rm { wait_write_err: true, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue_fmt( - child, - Some(Kind::Rm), - format_args!( - "\"{}\" may not be removed\n", - bstr::BStr::new(&resolved_owned) - ), - safeguard, - ); + return interp.as_cmd(cmd).io.stderr.enqueue_fmt( + child, + Some(Kind::Rm), + format_args!( + "\"{}\" may not be removed\n", + bstr::BStr::new(&resolved_owned) + ), + safeguard, + ); } let buf = Builtin::fmt_error_arena( interp, @@ -265,13 +263,13 @@ impl Rm { ); } RmParseFlag::IllegalOptionWithFlag => { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { Self::state_mut(interp, cmd).state = RmState::ParseOpts { idx, wait_write_err: true, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd).stderr.enqueue_fmt( + return interp.as_cmd(cmd).io.stderr.enqueue_fmt( child, Some(Kind::Rm), format_args!( @@ -336,15 +334,13 @@ impl Rm { } fn write_err_literal(interp: &Interpreter, cmd: NodeId, idx: u32, buf: &[u8]) -> Yield { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { Self::state_mut(interp, cmd).state = RmState::ParseOpts { idx, wait_write_err: true, }; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stderr - .enqueue(child, buf, safeguard); + return interp.as_cmd(cmd).io.stderr.enqueue(child, buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stderr, buf); Builtin::done(interp, cmd, 1) @@ -414,12 +410,14 @@ impl Rm { }; if let Some(s) = errstr { - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { if let RmState::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_count.fetch_add(1, Ordering::SeqCst); } let child = ChildPtr::new(cmd, WriterTag::Builtin); - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stderr .enqueue(child, &s, safeguard) .run(interp); @@ -473,11 +471,9 @@ impl Rm { } }); - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &buf, safeguard); + return interp.as_cmd(cmd).io.stdout.enqueue(child, &buf, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &buf); let done = match &mut Self::state_mut(interp, cmd).state { diff --git a/src/runtime/shell/builtin/seq.rs b/src/runtime/shell/builtin/seq.rs index 66d3270438bf..4816395ef16b 100644 --- a/src/runtime/shell/builtin/seq.rs +++ b/src/runtime/shell/builtin/seq.rs @@ -156,7 +156,7 @@ impl Seq { } fn do_(interp: &Interpreter, cmd: NodeId) -> Yield { - let needs_io = Builtin::of(interp, cmd).stdout.needs_io().is_some(); + let needs_io = interp.as_cmd(cmd).io.stdout.needs_io().is_some(); // Render entirely into a local Vec, then either enqueue it or // write_no_io it; we buffer once for simplicity. let (start, end, incr, sep, term) = { @@ -188,11 +188,9 @@ impl Seq { Self::state_mut(interp, cmd).state = State::Done; if needs_io { - let safeguard = Builtin::of(interp, cmd).stdout.needs_io().unwrap(); + let safeguard = interp.as_cmd(cmd).io.stdout.needs_io().unwrap(); let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) - .stdout - .enqueue(child, &out, safeguard); + return interp.as_cmd(cmd).io.stdout.enqueue(child, &out, safeguard); } let _ = Builtin::write_no_io(interp, cmd, IoKind::Stdout, &out); Builtin::done(interp, cmd, 0) diff --git a/src/runtime/shell/builtin/touch.rs b/src/runtime/shell/builtin/touch.rs index ca306ceea23d..22cc17eb157b 100644 --- a/src/runtime/shell/builtin/touch.rs +++ b/src/runtime/shell/builtin/touch.rs @@ -176,7 +176,7 @@ impl OutputTaskVTable for Touch { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stderr.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { // Stash so on_io_writer_chunk can route to the OutputTask state // machine and reclaim the box (stopgap for missing WriterTag). if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { @@ -184,7 +184,9 @@ impl OutputTaskVTable for Touch { } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stderr .enqueue(childptr, errbuf, safeguard), ); @@ -206,14 +208,16 @@ impl OutputTaskVTable for Touch { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_waiting += 1; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { if let State::Exec(exec) = &mut Self::state_mut(interp, cmd).state { exec.output_queue.push_back(child); } let childptr = ChildPtr::new(cmd, WriterTag::Builtin); let buf = output.slice().to_vec(); return Some( - Builtin::of_mut(interp, cmd) + interp + .as_cmd(cmd) + .io .stdout .enqueue(childptr, &buf, safeguard), ); diff --git a/src/runtime/shell/builtin/which.rs b/src/runtime/shell/builtin/which.rs index 27a3e606443c..ca16bbe2f27f 100644 --- a/src/runtime/shell/builtin/which.rs +++ b/src/runtime/shell/builtin/which.rs @@ -32,10 +32,12 @@ impl Which { pub(crate) fn start(interp: &Interpreter, cmd: NodeId) -> Yield { let argc = Builtin::of(interp, cmd).args_slice().len(); if argc == 0 { - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).state = State::OneArg; let child = ChildPtr::new(cmd, WriterTag::Builtin); - return Builtin::of_mut(interp, cmd) + return interp + .as_cmd(cmd) + .io .stdout .enqueue(child, b"\n", safeguard); } @@ -43,7 +45,7 @@ impl Which { return Builtin::done(interp, cmd, 1); } - if Builtin::of(interp, cmd).stdout.needs_io().is_none() { + if interp.as_cmd(cmd).io.stdout.needs_io().is_none() { // Synchronous path: resolve every arg, write straight to the // captured buffer, then finish. let search = SearchEnv::load(interp, cmd); @@ -114,8 +116,8 @@ impl Which { *had_not_found = true; *waiting_write = true; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - return Builtin::of_mut(interp, cmd).stdout.enqueue_fmt( + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { + return interp.as_cmd(cmd).io.stdout.enqueue_fmt( child, None, format_args!("{} not found\n", bstr::BStr::new(&arg)), @@ -138,8 +140,8 @@ impl Which { { *waiting_write = true; } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { - return Builtin::of_mut(interp, cmd).stdout.enqueue_fmt( + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { + return interp.as_cmd(cmd).io.stdout.enqueue_fmt( child, None, format_args!("{}\n", bstr::BStr::new(&resolved)), diff --git a/src/runtime/shell/builtin/yes.rs b/src/runtime/shell/builtin/yes.rs index 32a019ccc1c6..d901fa0e07b4 100644 --- a/src/runtime/shell/builtin/yes.rs +++ b/src/runtime/shell/builtin/yes.rs @@ -1,8 +1,9 @@ use crate::shell::ExitCode; -use crate::shell::builtin::{Builtin, BuiltinIO, BuiltinState, Impl, Kind}; +use crate::shell::builtin::{Builtin, BuiltinState, Impl, Kind}; use crate::shell::interpreter::{EventLoopHandle, Interpreter, NodeId, OutputNeedsIOSafeGuard}; +use crate::shell::io::CmdOut; use crate::shell::io_writer::{ChildPtr, WriterTag}; -use crate::shell::states::cmd::Exec; +use crate::shell::states::cmd::{Cmd, Exec}; use crate::shell::yield_::Yield; use bun_event_loop::{EventLoopTask, TaskTag, Taskable, task_tag}; @@ -77,7 +78,7 @@ impl Yes { }); } - if let Some(safeguard) = Builtin::of(interp, cmd).stdout.needs_io() { + if let Some(safeguard) = interp.as_cmd(cmd).io.stdout.needs_io() { Self::state_mut(interp, cmd).state = State::WaitingIo; return Self::enqueue_chunk(interp, cmd, safeguard); } @@ -94,10 +95,7 @@ impl Yes { let err = { let cmd_node = interp.as_cmd_mut(cmd); let shell = cmd_node.base.shell; - let Exec::Builtin(me) = &mut cmd_node.exec else { - unreachable!() - }; - let (stdout, yes) = Self::split_stdout_state(me); + let (stdout, yes) = Self::split_stdout_state(cmd_node); let chunk = &yes.buffer[..yes.buffer_used]; let mut err = None; for _ in 0..4 { @@ -139,9 +137,9 @@ impl Yes { safeguard: OutputNeedsIOSafeGuard, ) -> Yield { let child = ChildPtr::new(cmd, WriterTag::Builtin); - // `stdout` and `impl_` are disjoint fields of `Builtin` — split-borrow - // so the tiled buffer is enqueued zero-copy. - let (stdout, yes) = Self::split_stdout_state(Builtin::of_mut(interp, cmd)); + // `stdout` and the builtin are disjoint fields of the `Cmd` — + // split-borrow so the tiled buffer is enqueued zero-copy. + let (stdout, yes) = Self::split_stdout_state(interp.as_cmd_mut(cmd)); stdout.enqueue(child, &yes.buffer[..yes.buffer_used], safeguard) } @@ -168,18 +166,21 @@ impl Yes { if Self::state_mut(interp, cmd).state == State::WaitingWriteErr { return Builtin::done(interp, cmd, 1); } - debug_assert!(Builtin::of(interp, cmd).stdout.needs_io().is_some()); + debug_assert!(interp.as_cmd(cmd).io.stdout.needs_io().is_some()); Self::enqueue_chunk(interp, cmd, OutputNeedsIOSafeGuard::OutputNeedsIo) } - /// Split-borrow `&mut Builtin` into `(&mut stdout, &mut Yes)`; the fields + /// Split-borrow `&mut Cmd` into `(&mut stdout, &mut Yes)`; the fields /// are disjoint so this is a sound reborrow without `unsafe`. #[inline] - fn split_stdout_state(me: &mut Builtin) -> (&mut BuiltinIO, &mut Yes) { + fn split_stdout_state(cmd: &mut Cmd) -> (&mut CmdOut, &mut Yes) { + let Exec::Builtin(me) = &mut cmd.exec else { + unreachable!() + }; let Impl::Yes(yes) = &mut me.impl_ else { unreachable!() }; - (&mut me.stdout, &mut **yes) + (&mut cmd.io.stdout, &mut **yes) } } diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index d1735dad6f74..d3fec6e35ea4 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -51,7 +51,6 @@ use crate::shell::yield_::Yield; use crate::shell::{ShellErr, ast}; bun_core::declare_scope!(SHELL, visible); -bun_core::declare_scope!(CowFd, hidden); /// `log!("...")` — scoped debug logger for the shell interpreter. Expands to /// nothing in release; the `SHELL` static is referenced by absolute path so @@ -1159,6 +1158,17 @@ impl Interpreter { Some(crate::jsc::JSGlobalObject::opaque_ref(g)) } + /// The `${...}` object number `idx` of the script, and the global it + /// lives in. `None` when the script has no such object: its text can + /// spell any number. + pub(crate) fn js_object( + &self, + idx: u32, + ) -> Option<(&crate::jsc::JSGlobalObject, crate::jsc::JSValue)> { + let value = *self.jsobjs.get(idx as usize)?; + Some((self.global_this_ref()?, value)) + } + /// `mini` prints and exits(1); `js` raises a JS exception. Dispatch on /// `global_this` (set only on the JS event-loop path by /// `create_shell_interpreter`). @@ -2198,43 +2208,6 @@ pub(crate) struct ShellArgs { /// here so `IOReader`/`IOWriter`/builtin tasks keep their existing import path. pub(crate) use bun_event_loop::EventLoopHandle; -// ──────────────────────────────────────────────────────────────────────────── -// CowFd -// ──────────────────────────────────────────────────────────────────────────── - -/// Copy-on-write file descriptor: avoids multiple non-blocking writers on the -/// same fd (which breaks epoll/kqueue). -pub(crate) struct CowFd { - __fd: Fd, - refcount: u32, -} - -impl CowFd { - pub(crate) fn init(fd: Fd) -> *mut CowFd { - let new = bun_core::heap::into_raw(Box::new(CowFd { - __fd: fd, - refcount: 1, - })); - bun_core::scoped_log!(CowFd, "init {:x} fd={}", new as usize, fd); - new - } - - /// # Safety - /// `this` must point to a live `CowFd` (refcount ≥ 1). If this drops the - /// refcount to 0, `this` is freed and must not be used again. - pub(crate) fn deref(this: *mut CowFd) { - // SAFETY: caller upholds the precondition above (`this` is a live `CowFd`). - unsafe { - (*this).refcount -= 1; - if (*this).refcount == 0 { - // Close the fd before freeing; `closefd` tolerates EBADF. - closefd((*this).__fd); - drop(bun_core::heap::take(this)); - } - } - } -} - // ──────────────────────────────────────────────────────────────────────────── // Convenience re-exports for state modules // ──────────────────────────────────────────────────────────────────────────── diff --git a/src/runtime/shell/mod.rs b/src/runtime/shell/mod.rs index b54455a9679e..c48e020648fa 100644 --- a/src/runtime/shell/mod.rs +++ b/src/runtime/shell/mod.rs @@ -146,7 +146,7 @@ pub(crate) use bun_shell_parser::{escape_8bit, needs_escape_utf8_ascii_latin1}; pub(crate) mod ast { pub(crate) use bun_shell_parser::parse::SmolList; use bun_shell_parser::parse::ast as p; - pub(crate) use p::{BinaryOp, CondExprOp, IoKind, RedirectFlags}; + pub(crate) use p::{BinaryOp, CondExprOp, RedirectFlags}; pub(crate) type Script = p::Script<'static>; pub(crate) type Stmt = p::Stmt<'static>; diff --git a/src/runtime/shell/states/Cmd.rs b/src/runtime/shell/states/Cmd.rs index aaf8a1f47af4..b6ce58adef51 100644 --- a/src/runtime/shell/states/Cmd.rs +++ b/src/runtime/shell/states/Cmd.rs @@ -1,13 +1,17 @@ //! A shell primarily runs commands, so this is the main state node. //! //! Execution proceeds: expand assigns → expand redirect → expand argv atoms -//! → resolve to builtin or spawn subprocess → await exit. +//! → apply the redirect → resolve to builtin or spawn subprocess → await exit. use crate::shell::ExitCode; use crate::shell::ast; use crate::shell::builtin::{Builtin, Kind as BuiltinKind}; -use crate::shell::interpreter::{CowFd, Interpreter, Node, NodeId, ShellExecEnv, StateKind, log}; -use crate::shell::io::{IO, OutKind as IoOutKind}; +use crate::shell::interpreter::{ + Interpreter, Node, NodeId, ShellExecEnv, StateKind, is_pollable_from_mode, log, shell_openat, +}; +use crate::shell::io::{ArrayBufSink, CmdIO, CmdIn, CmdOut, IO, JsObject, OutFd}; +use crate::shell::io_reader::IOReader; +use crate::shell::io_writer::{self, IOWriter}; use crate::shell::shell_body::subproc::{Readable, ShellSubprocess, StdioKind}; use crate::shell::states::assigns::{AssignCtx, Assigns}; use crate::shell::states::base::Base; @@ -15,20 +19,28 @@ use crate::shell::states::expansion::Expansion; use crate::shell::subproc::{ShellIO, SpawnArgs}; use crate::shell::util::{OutKind, Stdio}; use crate::shell::yield_::Yield; +use crate::webcore::blob::store::Data as StoreData; +use crate::webcore::node_types::PathOrFileDescriptor; use bun_collections::VecExt; +use bun_jsc::PinnedArrayBuffer; pub(crate) struct Cmd { pub(crate) base: Base, pub node: bun_ptr::BackRef, - pub(crate) io: IO, + /// As inherited until `apply_redirect`, then with the redirect applied. + pub(crate) io: CmdIO, pub(crate) state: CmdState, pub args: Vec>, pub(crate) redirection_file: Vec, - pub(crate) redirection_fd: Option<*mut CowFd>, pub(crate) exec: Exec, pub(crate) exit_code: Option, + /// `apply_redirect` gave the command the stderr in `io`. + stderr_redirected: bool, } +// `Node` is as large as its largest state, and this is it. +const _: () = assert!(size_of::() <= 176); + #[derive(Default, strum::IntoStaticStr)] pub enum CmdState { #[default] @@ -53,6 +65,71 @@ pub(crate) enum Exec { Subproc(Box), } +/// Who uses the streams of a command. +#[derive(Clone, Copy)] +enum ExecForm { + /// A builtin: this process reads and writes the streams. + Builtin, + /// A child process gets the streams, or the command has no name. + Spawn, +} + +/// The streams that a redirect gives a command. `None`: as inherited. +#[derive(Default)] +struct Redirected { + stdin: Option, + stdout: Option, + stderr: Option, +} + +impl Redirected { + /// `out` is the target of `>`, `2>` or `&>`. + fn set_out(&mut self, streams: (bool, bool), out: CmdOut) { + match streams { + (true, true) => { + self.stderr = Some(out.dup()); + self.stdout = Some(out); + } + (true, false) => self.stdout = Some(out), + (false, true) => self.stderr = Some(out), + (false, false) => {} + } + } +} + +/// Why a redirect was not performed. +enum RedirectError { + /// A JS error is thrown. + Thrown, + /// The message for the stderr that the command inherits. + Message(Vec), +} + +impl RedirectError { + /// `target` did not open. + fn open(e: &bun_sys::Error, target: &[u8]) -> RedirectError { + use std::io::Write as _; + let mut message = Vec::new(); + let _ = write!( + &mut message, + "bun: {}: {}", + e.to_shell_system_error().message, + bstr::BStr::new(target) + ); + RedirectError::Message(message) + } +} + +/// Whether a redirect to a target is for stdout and for stderr. +/// +/// A target after `2>&1` or `1>&2` comes from the parser reading the next +/// word as one. Each exec form keeps what it did with it: a child gets both +/// streams on the target, a builtin gets only the stream that the flags name. +fn out_streams(flags: ast::RedirectFlags, form: ExecForm) -> (bool, bool) { + let both = matches!(form, ExecForm::Spawn) && flags.duplicate_out(); + (flags.stdout() || both, flags.stderr() || both) +} + impl Cmd { /// Borrow the AST node this `Cmd` was built from. /// @@ -164,15 +241,12 @@ impl BufferedIoClosed { /// /// `readable` is the subprocess's `stdout`/`stderr` `Readable`; if it was /// a pipe its buffered bytes are taken (ownership moves into - /// `state.Closed`) and, if the shell-side IO is `.pipe` and the AST - /// redirect didn't send this stream elsewhere, also tee'd into the - /// command-substitution aggregate buffer. + /// `state.Closed`) and also copied to `capture`, the buffer that holds + /// the captured output of the stream (`CmdOut::capture_buf`). fn close_out( slot: &mut Option, readable: &mut Readable, - io_is_pipe: bool, - redirects_elsewhere: bool, - shell_buf: *mut Vec, + capture: Option<*mut Vec>, ) { let Some(state) = slot.as_mut() else { return }; let Readable::Pipe(pipe) = readable else { @@ -181,14 +255,12 @@ impl BufferedIoClosed { *state = BufferedIoState::Closed(Vec::::default()); return; }; - // If the shell state is piped (inside a cmd substitution) aggregate - // the output of this command. - if io_is_pipe && !redirects_elsewhere && !shell_buf.is_null() { + if let Some(capture) = capture { let the_slice = pipe.slice(); - // SAFETY: `shell_buf` points into `ShellExecEnv::_buffered_*`, - // which the owning Cmd's `base.shell` keeps live for the duration - // of the command. Single-threaded. - unsafe { (*shell_buf).append_slice(the_slice) }; + // SAFETY: `capture` points into `ShellExecEnv::_buffered_*`, + // which the owning Cmd's `base.shell` (or an env above it) keeps + // live for the duration of the command. Single-threaded. + unsafe { (*capture).append_slice(the_slice) }; } // SAFETY: `Arc` interior mutability — the shell is // single-threaded and this is the same pattern `subproc::on_close_io` @@ -209,13 +281,13 @@ impl Cmd { interp.alloc_node(Node::Cmd(Cmd { base: Base::new(parent, shell), node: bun_ptr::BackRef::new(node), - io, + io: io.into(), state: CmdState::Idle, args: Vec::new(), redirection_file: Vec::new(), - redirection_fd: None, exec: Exec::None, exit_code: None, + stderr_redirected: false, })) } @@ -302,15 +374,19 @@ impl Cmd { } /// IOWriter completion callback for the error message written in - /// `WaitingWriteErr`: throw on write failure, otherwise finish the Cmd - /// with exit code 1. + /// `WaitingWriteErr`: finish the Cmd with exit code 1. A write to the + /// inherited stderr that failed is thrown. A write to the stderr that the + /// redirect gave the command is not: like the output of a builtin, the + /// message is lost and the script goes on. pub(crate) fn on_io_writer_chunk( interp: &Interpreter, this: NodeId, _written: usize, e: Option, ) -> Yield { - if let Some(err) = e { + if let Some(err) = e + && !interp.as_cmd(this).stderr_redirected + { interp.throw(crate::shell::ShellErr::from_system(err)); return Yield::Failed(this); } @@ -402,8 +478,7 @@ impl Cmd { CmdState::ExpandingRedirect { ref mut idx } => { *idx += 1; // Zero words or >1 word (glob/brace split) leave the buffer - // empty so the ambiguous-redirect check in - // `Builtin::init_redirections` / `init_subproc_redirections` fires. + // empty so the ambiguous-redirect check in `apply_redirect` fires. let mut buf = if out.bounds.is_empty() { out.buf } else { @@ -421,8 +496,8 @@ impl Cmd { Yield::Next(this) } - /// Resolves argv[0] to a builtin or falls through to subprocess spawn - /// (still gated). + /// Applies the redirect, then resolves argv[0] to a builtin or falls + /// through to subprocess spawn (still gated). fn transition_to_exec(interp: &Interpreter, this: NodeId) -> Yield { // NUL-terminate every arg so builtins can borrow them as `*const c_char`. for a in &mut interp.as_cmd_mut(this).args { @@ -431,30 +506,39 @@ impl Cmd { } } - // Empty/null argv[0] → exit - // with the exit code from a sole command-substitution (stashed by - // `child_done` from `Expansion::out_exit_code`), else 0. - let first_arg: Vec = { + let me = interp.as_cmd(this); + let node = me.node; + // Empty/null argv[0] → no command name. + let first_arg: Option> = match me.args.first() { + // strip the trailing NUL we just added + Some(a) if a.len() > 1 => Some(a[..a.len() - 1].to_vec()), + _ => None, + }; + let builtin = first_arg.as_deref().and_then(BuiltinKind::from_argv0); + + // The redirect comes before the search for the command (POSIX 2.9.1). + // So every exit below has the redirected streams, and a redirect that + // fails is the only thing the command reports. + let form = match builtin { + Some(_) => ExecForm::Builtin, + None => ExecForm::Spawn, + }; + if let Some(y) = Self::apply_redirect(interp, this, node.get(), form) { + return y; + } + + let Some(first_arg) = first_arg else { + // Exit with the exit code from a sole command-substitution + // (stashed by `child_done` from `Expansion::out_exit_code`), else 0. let me = interp.as_cmd(this); - match me.args.first() { - Some(a) if a.len() > 1 => { - // strip the trailing NUL we just added - a[..a.len() - 1].to_vec() - } - _ => { - let exit = me.exit_code.unwrap_or(0); - let parent = me.base.parent; - return interp.child_done(parent, this, exit); - } - } + let exit = me.exit_code.unwrap_or(0); + let parent = me.base.parent; + return interp.child_done(parent, this, exit); }; - if let Some(kind) = BuiltinKind::from_argv0(&first_arg) { + if let Some(kind) = builtin { log!("Cmd {} exec builtin={:?}", this, kind); - if let Some(y) = Builtin::init(interp, this, kind) { - return y; - } - debug_assert!(matches!(interp.as_cmd(this).exec, Exec::Builtin(_))); + Builtin::init(interp, this, kind); return Builtin::start(interp, this); } @@ -539,33 +623,23 @@ impl Cmd { resolved.push(0); interp.as_cmd_mut(this).args[0] = resolved; - // Convert shell IO → subprocess stdio. + // Convert the command's streams → subprocess stdio. let mut shellio = ShellIO::default(); - interp + if interp .as_cmd(this) .io - .to_subproc_stdio(&mut spawn_args.stdio, &mut shellio); - - // Apply file/jsbuf/`2>&1` redirects on - // top of the IO-derived stdio. - match Self::init_subproc_redirections( - interp, - this, - &mut spawn_args.stdio, - &mut spawn_args.redirect_stdout, - &mut spawn_args.redirect_stderr, - ) { - Ok(None) => {} - Ok(Some(y)) => { - drop(spawn_args); - drop(arena); - return y; - } - Err(_) => { - drop(spawn_args); - drop(arena); - return Yield::Failed(this); - } + .to_subproc_stdio( + interp, + &mut spawn_args.stdio, + &mut shellio, + &mut spawn_args.redirect_stdout, + &mut spawn_args.redirect_stderr, + ) + .is_err() + { + drop(spawn_args); + drop(arena); + return Yield::Failed(this); } // Stage the exec slot *before* spawning so PipeReader / process-exit @@ -682,182 +756,292 @@ impl Cmd { Yield::suspended() } - /// Applies the AST - /// redirect (`> file`, `< ${blob}`, `2>&1`, …) onto the subprocess stdio - /// triple. Returns `Ok(Some(yield))` when the redirect failed and a - /// failing-error write was queued; `Err` when a JS exception was raised. - fn init_subproc_redirections( + /// Performs the redirect of the command (`> file`, `< ${blob}`, `2>&1`, …) + /// on its `io`. Every user of the streams comes after it: a builtin, the + /// spawn of a child process, and the shell's message about a command that + /// does not start. + /// + /// `Some`: the redirect failed. `io` is as inherited, and the yield + /// finishes the command: the message is queued on the inherited stderr, + /// or a JS error is thrown. + fn apply_redirect( interp: &Interpreter, this: NodeId, - stdio: &mut [Stdio; 3], - redirect_stdout: &mut Option, - redirect_stderr: &mut Option, - ) -> crate::jsc::JsResult> { - const STDIN_NO: usize = 0; - const STDOUT_NO: usize = 1; - const STDERR_NO: usize = 2; - - let node: &ast::Cmd = interp.as_cmd(this).ast_node(); + node: &ast::Cmd, + form: ExecForm, + ) -> Option { let flags = node.redirect; - - let Some(redirect) = &node.redirect_file else { - if flags.duplicate_out() { + let redirected = match &node.redirect_file { + None if !flags.duplicate_out() => return None, + None => { + let io = &interp.as_cmd(this).io; + let mut redirected = Redirected::default(); + // The lexer keeps the stream that both become: `2>&1` is + // `STDOUT`, `1>&2` is `STDERR`. if flags.stdout() { - stdio[STDERR_NO] = Stdio::Dup2(crate::api::bun_spawn::stdio::Dup2 { - out: StdioKind::Stderr, - to: StdioKind::Stdout, - }); + redirected.stderr = Some(io.stdout.dup()); } if flags.stderr() { - stdio[STDOUT_NO] = Stdio::Dup2(crate::api::bun_spawn::stdio::Dup2 { - out: StdioKind::Stdout, - to: StdioKind::Stderr, - }); + redirected.stdout = Some(io.stderr.dup()); + } + Ok(redirected) + } + Some(ast::Redirect::Atom(_)) => { + // `child_done` NUL-terminated the one word the target expanded to. + let path = core::mem::take(&mut interp.as_cmd_mut(this).redirection_file); + match path.len().checked_sub(1) { + Some(len) => Self::redirect_to_path( + interp, + this, + form, + bun_core::ZStr::from_buf(&path, len), + ), + None => Err(Self::ambiguous_redirect(interp, this)), } } - return Ok(None); + Some(ast::Redirect::JsBuf(js)) => { + Self::redirect_to_js_object(interp, this, form, js.idx) + } + }; + let redirected = match redirected { + Ok(redirected) => redirected, + Err(RedirectError::Thrown) => return Some(Yield::Failed(this)), + Err(RedirectError::Message(message)) => { + return Some(Builtin::cmd_write_failing_error( + interp, + this, + format_args!("{}", bstr::BStr::new(&message)), + )); + } }; + let me = interp.as_cmd_mut(this); + if let Some(stdin) = redirected.stdin { + me.io.stdin = stdin; + } + if let Some(stdout) = redirected.stdout { + me.io.stdout = stdout; + } + if let Some(stderr) = redirected.stderr { + me.io.stderr = stderr; + me.stderr_redirected = true; + } + None + } + + /// The target of the redirect expanded to no word, or to more than one. + fn ambiguous_redirect(interp: &Interpreter, this: NodeId) -> RedirectError { + use std::io::Write as _; + let mut message = Vec::new(); + let _ = match interp.as_cmd(this).args.first() { + // NUL-terminated by `transition_to_exec`. + Some(argv0) => writeln!( + &mut message, + "bun: ambiguous redirect: at `{}`", + bstr::BStr::new(&argv0[..argv0.len() - 1]) + ), + None => writeln!(&mut message, "bun: ambiguous redirect"), + }; + RedirectError::Message(message) + } - match redirect { - ast::Redirect::JsBuf(val) => { - // Safe accessor — single `unsafe` deref lives in - // `Interpreter::global_this_ref`. - let global = interp - .global_this_ref() - .expect("JS values not allowed in this context"); - let idx = val.idx as usize; - if idx >= interp.jsobjs.len() { - return Err(global.throw(format_args!("Invalid JS object reference in shell"))); + /// The redirect names a file by its path: open it. A builtin reads it + /// through an `IOReader` and writes it through an `IOWriter`. A child + /// process gets the fd. + fn redirect_to_path( + interp: &Interpreter, + this: NodeId, + form: ExecForm, + path: &bun_core::ZStr, + ) -> Result { + let me = interp.as_cmd(this); + let flags = me.ast_node().redirect; + let cwd_fd = me.base.shell().cwd_fd; + let streams = out_streams(flags, form); + let mut redirected = Redirected::default(); + let opened = match form { + ExecForm::Builtin if flags.stdin() => { + shell_openat(cwd_fd, path, flags.to_flags(), 0o666).map(|fd| { + let reader = IOReader::init(fd, interp.event_loop); + reader.set_interp(interp.as_ctx_ptr()); + redirected.stdin = Some(CmdIn::Fd(reader)); + }) + } + ExecForm::Builtin => Self::open_writer(interp, cwd_fd, &path, flags.to_flags()) + .map(|out| redirected.set_out(streams, out)), + ExecForm::Spawn => shell_openat(cwd_fd, path, flags.to_flags(), 0o666).map(|fd| { + let file = bun_sys::File::from_fd(fd); + if flags.stdin() { + redirected.stdin = Some(CmdIn::File(file)); + } else { + redirected.set_out(streams, CmdOut::File(file)); } - let jsval = interp.jsobjs[idx]; - - if let Some(buf) = jsval.as_array_buffer(global) { - if flags.stdin() { - let bytes = buf.byte_slice(); - // An empty buffer delivers EOF immediately; `Stdio::Ignore` - // matches what `Stdio::extract`/`extract_blob` already do. - stdio[STDIN_NO] = if bytes.is_empty() { - Stdio::Ignore - } else { - Stdio::Blob(crate::webcore::blob::Any::from_owned_slice(bytes.to_vec())) - }; - } - let mut redirect_out = - |fd: usize, slot: &mut Option| { - let Some(buf) = crate::jsc::PinnedArrayBuffer::root(global, jsval) - else { - return Err(global.throw_out_of_memory()); - }; - stdio[fd] = Stdio::Pipe; - *slot = Some(buf); - Ok(()) - }; - if flags.duplicate_out() || flags.stdout() { - redirect_out(STDOUT_NO, redirect_stdout)?; - } - if flags.duplicate_out() || flags.stderr() { - redirect_out(STDERR_NO, redirect_stderr)?; - } - } else if let Some(blob_ref) = jsval.as_class_ref::() { - let blob = blob_ref.dupe(); - if flags.stdin() { - stdio[STDIN_NO].extract_blob( - global, - crate::webcore::blob::Any::Blob(blob), - STDIN_NO as i32, - )?; - } else if flags.stdout() { - stdio[STDOUT_NO].extract_blob( - global, - crate::webcore::blob::Any::Blob(blob), - STDOUT_NO as i32, - )?; - } else if flags.stderr() { - stdio[STDERR_NO].extract_blob( - global, - crate::webcore::blob::Any::Blob(blob), - STDERR_NO as i32, - )?; - } - } else if crate::webcore::ReadableStream::from_js(jsval, global)?.is_some() { - panic!("TODO SHELL READABLE STREAM"); - } else if let Some(req) = jsval.as_::() { - // SAFETY: `as_` returns a live JSC-owned `*mut Response`; - // `get_body_value` is `&self`. - let req = unsafe { &*req }; - req.get_body_value().to_blob_if_possible(); - if flags.stdin() { - let b = req.get_body_value().use_as_any_blob(); - stdio[STDIN_NO].extract_blob(global, b, STDIN_NO as i32)?; - } - if flags.stdout() { - let b = req.get_body_value().use_as_any_blob(); - stdio[STDOUT_NO].extract_blob(global, b, STDOUT_NO as i32)?; - } - if flags.stderr() { - let b = req.get_body_value().use_as_any_blob(); - stdio[STDERR_NO].extract_blob(global, b, STDERR_NO as i32)?; + }), + }; + match opened { + Ok(()) => Ok(redirected), + Err(e) => Err(RedirectError::open(&e, path.as_bytes())), + } + } + + /// Opens `target`, a path or an fd, for a builtin to write to. + fn open_writer( + interp: &Interpreter, + cwd_fd: bun_sys::Fd, + target: &P, + flags: i32, + ) -> bun_sys::Result { + let mut pollable = false; + let mut is_socket = false; + let mut is_nonblocking = false; + let fd = bun_io::open_for_writing_impl( + cwd_fd, + target, + flags, + 0o666, + &mut pollable, + &mut is_socket, + false, + &mut is_nonblocking, + (), + |_| {}, + is_pollable_from_mode, + shell_openat, + )?; + #[cfg(windows)] + let fd = { + use bun_sys::FdExt as _; + fd.make_lib_uv_owned_for_syscall(bun_sys::Tag::open, bun_sys::ErrorCase::CloseOnFail)? + }; + // Honor the `pollable` computed by `open_for_writing_impl` on + // POSIX so a FIFO/socket target (whose fd is now O_NONBLOCK) + // takes the pollable path; Windows keeps the async writer. + let writer = IOWriter::init( + fd, + io_writer::Flags { + pollable: if cfg!(windows) { true } else { pollable }, + nonblock: is_nonblocking, + is_socket, + ..Default::default() + }, + interp.event_loop, + ); + writer.set_interp(interp.as_ctx_ptr()); + Ok(CmdOut::Fd(OutFd { + writer, + captured: None, + })) + } + + /// The redirect names a JS object: `> ${object}` or `< ${object}`. + fn redirect_to_js_object( + interp: &Interpreter, + this: NodeId, + form: ExecForm, + idx: u32, + ) -> Result { + let Some((global, value)) = interp.js_object(idx) else { + interp.throw(crate::shell::ShellErr::Custom( + b"Invalid JS object reference in shell" + .to_vec() + .into_boxed_slice(), + )); + return Err(RedirectError::Thrown); + }; + let Some(object) = JsObject::from_value(value) else { + let _ = global.throw(format_args!( + "Unknown JS value used in shell: {}", + value.fmt_string(global) + )); + return Err(RedirectError::Thrown); + }; + let flags = interp.as_cmd(this).ast_node().redirect; + let streams = out_streams(flags, form); + let mut redirected = Redirected::default(); + // `None` is out of memory. + let pin = || { + let buf = PinnedArrayBuffer::root(global, value); + if buf.is_none() { + let _ = global.throw_out_of_memory(); + } + buf.ok_or(RedirectError::Thrown) + }; + + if flags.stdin() { + redirected.stdin = Some(match (form, object) { + // The spawn reads the object. + (ExecForm::Spawn, _) => CmdIn::Js(idx), + (ExecForm::Builtin, JsObject::ArrayBuffer(_)) => CmdIn::ArrayBuf(Box::new(pin()?)), + (ExecForm::Builtin, JsObject::Blob(blob)) => CmdIn::Blob(Box::new(blob.dupe())), + (ExecForm::Builtin, JsObject::Response(response)) => { + CmdIn::Blob(Box::new(response.get_body_value().use_())) + } + }); + return Ok(redirected); + } + + // Output to a Blob, or to the body of a Response, goes to the file it is. + let file = match object { + JsObject::ArrayBuffer(_) => { + match form { + ExecForm::Spawn => redirected.set_out(streams, CmdOut::JsBuf(idx)), + ExecForm::Builtin => { + // Each stream pins the buffer for itself and has its own offset. + if streams.0 { + let sink = ArrayBufSink { buf: pin()?, i: 0 }; + redirected.stdout = Some(CmdOut::ArrayBuf(Box::new(sink))); + } + if streams.1 { + let sink = ArrayBufSink { buf: pin()?, i: 0 }; + redirected.stderr = Some(CmdOut::ArrayBuf(Box::new(sink))); + } } - } else { - return Err(global.throw(format_args!( - "Unknown JS value used in shell: {}", - jsval.fmt_string(global) - ))); } + return Ok(redirected); } - ast::Redirect::Atom(_) => { - if interp.as_cmd(this).redirection_file.is_empty() { - let argv0 = interp - .as_cmd(this) - .args - .first() - .map(|a| &a[..a.len().saturating_sub(1)]) - .unwrap_or(b"") - .to_vec(); - return Ok(Some(Builtin::cmd_write_failing_error( - interp, - this, - format_args!( - "bun: ambiguous redirect: at `{}`\n", - bstr::BStr::new(&argv0) - ), - ))); + JsObject::Blob(blob) => blob_file(blob), + JsObject::Response(response) => { + let body = response.get_body_value(); + body.to_blob_if_possible(); + match body.use_as_any_blob() { + crate::webcore::blob::Any::Blob(blob) => blob_file(&blob), + _ => None, } - let path_buf: Vec = { - let raw = &interp.as_cmd(this).redirection_file; - let len = raw.len().saturating_sub(1); - let mut v = raw[..len].to_vec(); - v.push(0); - v - }; - let path = bun_core::ZStr::from_buf(&path_buf, path_buf.len() - 1); - log!("Expanded Redirect: {}\n", bstr::BStr::new(path.as_bytes())); - let cwd_fd = interp.as_cmd(this).base.shell().cwd_fd; - let redirfd = match crate::shell::interpreter::shell_openat( - cwd_fd, - path, - flags.to_flags(), - 0o666, - ) { - Ok(f) => f, - Err(e) => { - let sys_err = e.to_shell_system_error(); - return Ok(Some(Builtin::cmd_write_failing_error( - interp, - this, - format_args!( - "bun: {}: {}", - sys_err.message, - bstr::BStr::new(path.as_bytes()) - ), - ))); + } + }; + match file { + Some(BlobFile::Fd(fd)) => { + let out = match form { + // The builtin writes to a copy of the fd. The caller keeps its own. + ExecForm::Builtin => { + let cwd_fd = interp.as_cmd(this).base.shell().cwd_fd; + let target = bun_io::PathOrFileDescriptor::Fd(fd); + match Self::open_writer(interp, cwd_fd, &target, flags.to_flags()) { + Ok(out) => out, + Err(e) => return Err(RedirectError::open(&e, b"Bun.file(fd)")), + } } + ExecForm::Spawn => CmdOut::BorrowedFd(fd), + }; + redirected.set_out(streams, out); + Ok(redirected) + } + Some(BlobFile::Path(mut path)) => { + let len = path.len(); + path.push(0); + Self::redirect_to_path(interp, this, form, bun_core::ZStr::from_buf(&path, len)) + } + None => { + let _ = match form { + ExecForm::Builtin => global.throw(format_args!( + "Cannot redirect stdout/stderr to an immutable blob. Expected a file" + )), + ExecForm::Spawn => global.throw_invalid_arguments(format_args!( + "Blobs are immutable, and cannot be used for stdout/stderr" + )), }; - interp.as_cmd_mut(this).redirection_fd = Some(CowFd::init(redirfd)); - set_stdio_from_redirect(stdio, flags, redirfd); + Err(RedirectError::Thrown) } } - Ok(None) } /// Called by `Builtin::done` / subprocess exit handler. @@ -878,14 +1062,13 @@ impl Cmd { pub(crate) fn deinit_from_finalizer(interp: &Interpreter, this: NodeId) { { let me = interp.as_cmd_mut(this); - match &mut me.exec { - Exec::Builtin(b) => b.defuse_array_buf_pins(), - Exec::Subproc(sub) if !sub.child.is_null() => { - // SAFETY: `child` is the live heap::alloc'd subprocess; - // the call only writes its own fields. - unsafe { ShellSubprocess::defuse_array_buffer_unpins(sub.child) }; - } - _ => {} + me.io.defuse_array_buf_pins(); + if let Exec::Subproc(sub) = &me.exec + && !sub.child.is_null() + { + // SAFETY: `child` is the live heap::alloc'd subprocess; + // the call only writes its own fields. + unsafe { ShellSubprocess::defuse_array_buffer_unpins(sub.child) }; } } Self::deinit(interp, this); @@ -911,10 +1094,6 @@ impl Cmd { let me = interp.as_cmd_mut(this); me.args.clear(); me.redirection_file.clear(); - if let Some(fd) = me.redirection_fd.take() { - // SAFETY: `fd` is the +1 ref held in `me.redirection_fd`. - CowFd::deref(fd); - } core::mem::take(&mut me.exec) }; // `me`'s borrow ended above: the teardown below re-enters this Cmd @@ -989,9 +1168,39 @@ impl Cmd { kind: OutKind, err: Option, ) -> Yield { - match kind { - OutKind::Stdout => self.buffered_output_close_stdout(err), - OutKind::Stderr => self.buffered_output_close_stderr(err), + debug_assert!(matches!(self.exec, Exec::Subproc(_))); + log!( + "cmd close buffered {}", + match kind { + OutKind::Stdout => "stdout", + OutKind::Stderr => "stderr", + } + ); + if let Some(e) = err { + self.exit_code = Some(e.errno.unsigned_abs() as ExitCode); + } + if let Exec::Subproc(sub) = &mut self.exec { + // Raw deref keeps the borrow disjoint from `sub.buffered_closed` below. + // SAFETY: `child` is the live subprocess owned by this Cmd. + let child = unsafe { &mut *sub.child }; + let (out, slot, readable, stdio) = match kind { + OutKind::Stdout => ( + &self.io.stdout, + &mut sub.buffered_closed.stdout, + &mut child.stdout, + StdioKind::Stdout, + ), + OutKind::Stderr => ( + &self.io.stderr, + &mut sub.buffered_closed.stderr, + &mut child.stderr, + StdioKind::Stderr, + ), + }; + // The stream the command has says where its output is captured: + // the JS-side buffer, or the buffer of the shell env. + BufferedIoClosed::close_out(slot, readable, out.capture_buf(self.base.shell_mut())); + child.close_io(stdio); } self.finish_if_done() } @@ -1024,76 +1233,6 @@ impl Cmd { } Yield::Next(this_id) } - - fn buffered_output_close_stdout(&mut self, err: Option) { - debug_assert!(matches!(self.exec, Exec::Subproc(_))); - log!("cmd close buffered stdout"); - if let Some(e) = err { - self.exit_code = Some(e.errno.unsigned_abs() as ExitCode); - } - let redirect = self.ast_node().redirect; - let Exec::Subproc(sub) = &mut self.exec else { - return; - }; - // Raw deref keeps the borrow disjoint from `sub.buffered_closed` below. - // SAFETY: `child` is the live subprocess owned by this Cmd. - let child = unsafe { &mut *sub.child }; - // Tee into the JS-side captured buffer if stdout is an fd with a - // `captured` slot and the redirect didn't send stdout elsewhere. - if let IoOutKind::Fd(fd) = &self.io.stdout { - // SAFETY: single-threaded; the captured `Vec` lives in the - // owning `ShellExecEnv` and no other borrow of it is live here. - if let Some(captured) = unsafe { fd.captured_mut() } { - if !redirect.redirects_elsewhere(ast::IoKind::Stdout) { - if let Readable::Pipe(pipe) = &child.stdout { - captured.append_slice(pipe.slice()); - } - } - } - } - BufferedIoClosed::close_out( - &mut sub.buffered_closed.stdout, - &mut child.stdout, - matches!(self.io.stdout, IoOutKind::Pipe), - redirect.redirects_elsewhere(ast::IoKind::Stdout), - self.base.shell_mut().buffered_stdout(), - ); - child.close_io(StdioKind::Stdout); - } - - fn buffered_output_close_stderr(&mut self, err: Option) { - debug_assert!(matches!(self.exec, Exec::Subproc(_))); - log!("cmd close buffered stderr"); - if let Some(e) = err { - self.exit_code = Some(e.errno.unsigned_abs() as ExitCode); - } - let redirect = self.ast_node().redirect; - let Exec::Subproc(sub) = &mut self.exec else { - return; - }; - // Raw deref keeps the borrow disjoint from `sub.buffered_closed` below. - // SAFETY: `child` is the live subprocess owned by this Cmd. - let child = unsafe { &mut *sub.child }; - if let IoOutKind::Fd(fd) = &self.io.stderr { - // SAFETY: single-threaded; the captured `Vec` lives in the - // owning `ShellExecEnv` and no other borrow of it is live here. - if let Some(captured) = unsafe { fd.captured_mut() } { - if !redirect.redirects_elsewhere(ast::IoKind::Stderr) { - if let Readable::Pipe(pipe) = &child.stderr { - captured.append_slice(pipe.slice()); - } - } - } - } - BufferedIoClosed::close_out( - &mut sub.buffered_closed.stderr, - &mut child.stderr, - matches!(self.io.stderr, IoOutKind::Pipe), - redirect.redirects_elsewhere(ast::IoKind::Stderr), - self.base.shell_mut().buffered_stderr(), - ); - child.close_io(StdioKind::Stderr); - } } /// True when argv0 is the literal word `export`, the only declaration @@ -1129,19 +1268,19 @@ fn is_assignment_word(atom: &ast::Atom) -> bool { .all(|&b| b.is_ascii_alphanumeric() || b == b'_') } -fn set_stdio_from_redirect(stdio: &mut [Stdio; 3], flags: ast::RedirectFlags, fd: bun_sys::Fd) { - if flags.stdin() { - stdio[0] = Stdio::Fd(fd); - } - if flags.duplicate_out() { - stdio[1] = Stdio::Fd(fd); - stdio[2] = Stdio::Fd(fd); - } else { - if flags.stdout() { - stdio[1] = Stdio::Fd(fd); - } - if flags.stderr() { - stdio[2] = Stdio::Fd(fd); - } - } +/// The file that a Blob is: `Bun.file(fd)`, or a path. +enum BlobFile { + Fd(bun_sys::Fd), + Path(Vec), +} + +/// `None`: the Blob holds its bytes in memory. +fn blob_file(blob: &crate::webcore::Blob) -> Option { + let StoreData::File(file) = &blob.store()?.data else { + return None; + }; + Some(match &file.pathlike { + PathOrFileDescriptor::Fd(fd) => BlobFile::Fd(*fd), + PathOrFileDescriptor::Path(path) => BlobFile::Path(path.slice().to_vec()), + }) } diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index fa99bc8fe0db..fc6ee83f2d0a 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1348,8 +1348,8 @@ impl Readable { *self = Readable::Closed; fd.close(); } - // .fd is borrowed from the shell's IOWriter (see IO.OutKind.to_subproc_stdio) or - // a CowFd redirect; the owner closes it. + // .fd is borrowed from the Cmd's stream (see `CmdOut::to_subproc_stdio`): + // the shell's IOWriter or the target of a redirect. The owner closes it. Readable::Fd => { *self = Readable::Closed; } diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index 558c917fa360..6f4ca955857d 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -685,29 +685,6 @@ impl From for Value { } impl Value { - /// Downcast a `JSValue` to the `Body.Value` it owns, if any. - /// - /// `Body.Value` is not itself a JS class — it lives inside a `Request` or - /// `Response` wrapper — so the generic `JSValue::as_::()` path - /// cannot be used. Instead, try both wrapper classes and return the inner - /// body pointer. - /// - /// Returns a raw pointer; the storage is owned - /// by the JSC heap cell and outlives the call only as long as `value` is - /// kept alive by the caller. - pub(crate) fn from_request_or_response(value: JSValue) -> Option<*mut Value> { - if value.is_empty_or_undefined_or_null() { - return None; - } - if let Some(req) = value.as_class_ref::() { - return Some(std::ptr::from_mut::(req.get_body_value())); - } - if let Some(res) = value.as_class_ref::() { - return Some(std::ptr::from_mut::(res.get_body_value())); - } - None - } - pub(crate) fn was_string(&self) -> bool { match self { Value::InternalBlob(blob) => blob.was_string, diff --git a/src/shell_parser/parse.rs b/src/shell_parser/parse.rs index 7a29d7d3b5a4..22801455339e 100644 --- a/src/shell_parser/parse.rs +++ b/src/shell_parser/parse.rs @@ -439,18 +439,13 @@ pub mod ast { const STDOUT = 1 << 1; const STDERR = 1 << 2; const APPEND = 1 << 3; - /// 1>&2 === stdout=true and duplicate_out=true - /// 2>&1 === stderr=true and duplicate_out=true + /// Set with the stream that both streams become: + /// 2>&1 === stdout=true and duplicate_out=true + /// 1>&2 === stderr=true and duplicate_out=true const DUPLICATE_OUT = 1 << 4; } } - #[derive(Clone, Copy, PartialEq, Eq)] - pub enum IoKind { - Stdout, - Stderr, - } - impl RedirectFlags { #[inline] pub fn stdin(self) -> bool { @@ -476,25 +471,6 @@ pub mod ast { // bitflags already generates `is_empty()`; expose under this // spelling too for callers that use it. - pub fn redirects_elsewhere(self, io_kind: IoKind) -> bool { - match io_kind { - IoKind::Stdout => { - if self.duplicate_out() { - !self.stdout() - } else { - self.stdout() - } - } - IoKind::Stderr => { - if self.duplicate_out() { - !self.stderr() - } else { - self.stderr() - } - } - } - } - pub fn to_flags(self) -> i32 { // `bun_shell_parser` is sys-tier-free so it cannot depend on // `bun_sys::O`; mirror those constants here. On POSIX `bun.O.*` is diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 7081a555cbad..8df3a9445168 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -2162,21 +2162,33 @@ describe("deno_task", () => { }); }); - // An external command used to abort the process on this target. - test("a JS object that a redirect cannot use", async () => { + // An external command used to abort the process on a ReadableStream. + test("a JS object that a redirect cannot write to", async () => { await using proc = Bun.spawn({ cmd: [ bunExe(), "-e", ` import { $ } from "bun"; - const settle = p => p.then(r => "resolved " + r.exitCode, e => "rejected " + e.message); - const out = []; + const settle = async run => { + try { + return "resolved " + (await run()).exitCode; + } catch (e) { + return "rejected " + e.message; + } + }; + const targets = { + stream: () => new ReadableStream(), + blob: () => new Blob(["x"]), + response: () => new Response(new Blob(["x"])), + }; + const out = {}; for (const name of ["echo", process.execPath, "nosuch_cmd_zz"]) { - out.push(await settle($\`\${name} --version > \${new ReadableStream()}\`.quiet())); - out.push(await settle($\`\${name} --version > \${new Blob(["x"])}\`.quiet())); + for (const [kind, target] of Object.entries(targets)) { + (out[kind] ??= []).push(await settle(() => $\`\${name} --version > \${target()}\`.quiet())); + } } - console.log(JSON.stringify(out, null, 2)); + console.log(JSON.stringify(out)); `, ], env: bunEnv, @@ -2184,16 +2196,15 @@ describe("deno_task", () => { stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - const stream = "rejected Unknown JS value used in shell: [object ReadableStream]"; + const builtin = "rejected Cannot redirect stdout/stderr to an immutable blob. Expected a file"; + const external = "rejected Blobs are immutable, and cannot be used for stdout/stderr"; + const rejected = expect.stringMatching(/^rejected /); expect({ result: JSON.parse(stdout || "null"), stderr, exitCode }).toEqual({ - result: [ - stream, - "rejected Cannot redirect stdout/stderr to an immutable blob. Expected a file", - stream, - "rejected Blobs are immutable, and cannot be used for stdout/stderr", - stream, - "rejected Blobs are immutable, and cannot be used for stdout/stderr", - ], + result: { + stream: [rejected, rejected, rejected], + blob: [builtin, external, external], + response: [builtin, external, external], + }, stderr: "", exitCode: 0, }); @@ -2276,6 +2287,44 @@ describe("deno_task", () => { closeSync(fd); } }); + + test("external &> Bun.file(fd) gets both streams", async () => { + using dir = tempDir("shell-redirect-fd", {}); + const path = join(String(dir), "out.txt"); + const fd = openSync(path, "w"); + try { + const { stdout, stderr, exitCode } = await $`${BUN} -e ${both} &> ${Bun.file(fd)}`.quiet(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + file: readFileSync(path, "utf8"), + exitCode, + }).toEqual({ stdout: "", stderr: "", file: "out\nerr\n", exitCode: 0 }); + } finally { + closeSync(fd); + } + }); + + // A Response of a file is that file, like `> path`. + test.each([ + ["external", (response: Response) => $`${BUN} -e ${"console.log('hi')"} > ${response}`], + ["builtin", (response: Response) => $`echo hi > ${response}`], + ] as const)("%s > Response(Bun.file(path))", async (_name, run) => { + using dir = tempDir("shell-redirect-response", { "out.txt": "0123456789\n" }); + const path = join(String(dir), "out.txt"); + const { stdout, stderr, exitCode } = await run(new Response(Bun.file(path))).quiet(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + file: readFileSync(path, "utf8"), + exitCode, + }).toEqual({ stdout: "", stderr: "", file: "hi\n", exitCode: 0 }); + }); + + TestBuilder.command`bun --version > $EMPTY_ZZ` + .exitCode(1) + .stderr("bun: ambiguous redirect: at `bun`\n") + .runAsTest("an ambiguous redirect names the command as typed"); }); }); From eb42ffa9b3723cee8dcce5c8e4d67d0b676fad17 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 17 Oct 2026 21:00:32 +0000 Subject: [PATCH 3/5] shell: redirect targets that are an fd, and 2>&1 onto an fd on Windows - `2>&1` and `1>&2` of an external command on Windows: when the stream is an fd, the child gets that fd on both streams. `Dup2` there is a pipe that only a captured stream reads, so the output was lost under `bun exec` and in a pipeline. A shell writer whose fd libuv took over names the stream of the process that it is on. - A `Bun.file(fd)` target: the command gets a copy of the fd when the redirect is applied, for a builtin and for a child. An fd that is not open is a redirect that fails. A builtin no longer sets O_NONBLOCK on the fd of the caller. - The message about a command that does not start: the writer on the target waits for a FIFO, a socket or a tty with a poll. `&> file` gives stderr the file, so the message needs no copy of the fd. - A `Response` that a redirect rejects keeps its body. - Tests: a redirect in place of a pipe closes the shell's end of the pipe when it is applied. The worker fd test waits for the `close` event of the worker and has no timeout. --- src/runtime/shell/Builtin.rs | 10 +- src/runtime/shell/IO.rs | 129 ++++++++++-------- src/runtime/shell/interpreter.rs | 17 ++- src/runtime/shell/states/Cmd.rs | 50 ++++--- test/js/bun/shell/bunshell.test.ts | 110 ++++++++++++++- test/js/bun/shell/file-io.test.ts | 78 +++++++++++ .../shell/shell-worker-terminate-leak.test.ts | 99 +++++++------- 7 files changed, 355 insertions(+), 138 deletions(-) diff --git a/src/runtime/shell/Builtin.rs b/src/runtime/shell/Builtin.rs index 1f40be35faa8..168ffba2c059 100644 --- a/src/runtime/shell/Builtin.rs +++ b/src/runtime/shell/Builtin.rs @@ -293,7 +293,15 @@ impl Builtin { use std::io::Write as _; let mut buf = Vec::new(); let _ = buf.write_fmt(args); - interp.as_cmd_mut(cmd).io.stderr.make_writable(interp); + if interp + .as_cmd_mut(cmd) + .io + .stderr + .make_writable(interp) + .is_err() + { + return Yield::Failed(cmd); + } if let Some(safeguard) = interp.as_cmd(cmd).io.stderr.needs_io() { interp.as_cmd_mut(cmd).state = CmdState::WaitingWriteErr; let child = io_writer::ChildPtr::new(cmd, io_writer::WriterTag::Cmd); diff --git a/src/runtime/shell/IO.rs b/src/runtime/shell/IO.rs index af0a8f57558b..921970f09e3e 100644 --- a/src/runtime/shell/IO.rs +++ b/src/runtime/shell/IO.rs @@ -12,7 +12,7 @@ use bun_sys::Fd; use crate::api::bun_spawn::stdio::{Capture, Dup2, Stdio}; use crate::shell::builtin::{IoKind, Kind}; -use crate::shell::interpreter::{Interpreter, OutputNeedsIOSafeGuard, ShellExecEnv, shell_dup}; +use crate::shell::interpreter::{Interpreter, OutputNeedsIOSafeGuard, ShellExecEnv, is_pollable}; use crate::shell::io_reader::IOReader; use crate::shell::io_writer::{self, IOWriter}; use crate::shell::shell_body::subproc::{ShellIO, StdioKind}; @@ -141,10 +141,10 @@ pub(crate) enum CmdOut { Fd(OutFd), /// The captured buffer of the shell env: `_buffered_stdout` or `_buffered_stderr`. Buf(IoKind), - /// `> file` for a command that is not a builtin. The fd stays blocking, as a child needs it. + /// `> file` for a command that is not a builtin: the file, or a copy of + /// the fd of a `Bun.file(fd)`. The fd stays blocking, as a child needs it. File(bun_sys::File), - /// An fd that this stream does not own: the `File` of the other stream - /// (`&> file`), or the fd of a `Bun.file(fd)`. + /// The fd of the `File` that the other stream owns: `&> file`. BorrowedFd(Fd), /// `> ${buffer}` for a builtin. ArrayBuf(Box), @@ -213,25 +213,24 @@ impl CmdIO { redirect_stderr: &mut Option, ) -> JsResult<()> { stdio[0] = self.stdin.to_subproc_stdio(interp)?; - stdio[1] = self.stdout.to_subproc_stdio( - interp, - StdioKind::Stdout, - &mut shellio.stdout, - redirect_stdout, - )?; - stdio[2] = if self.stderr.is_same_stream(&self.stdout) { - // `2>&1` or `1>&2`: the child gets the one stream on stdout. - Stdio::Dup2(Dup2 { - out: StdioKind::Stderr, - to: StdioKind::Stdout, - }) + stdio[1] = self + .stdout + .to_subproc_stdio(interp, &mut shellio.stdout, redirect_stdout)?; + stdio[2] = if !self.stderr.is_same_stream(&self.stdout) { + self.stderr + .to_subproc_stdio(interp, &mut shellio.stderr, redirect_stderr)? } else { - self.stderr.to_subproc_stdio( - interp, - StdioKind::Stderr, - &mut shellio.stderr, - redirect_stderr, - )? + // `2>&1` or `1>&2`: the child gets the one stream on stdout. + match &stdio[1] { + // `Dup2` on Windows is a pipe that this process reads for the + // child (`spawn_process_windows`), and only a captured stdout + // has a reader. An fd goes to the child on both streams. + Stdio::Fd(fd) if cfg!(windows) => Stdio::Fd(*fd), + _ => Stdio::Dup2(Dup2 { + out: StdioKind::Stderr, + to: StdioKind::Stdout, + }), + } }; Ok(()) } @@ -311,7 +310,7 @@ impl CmdOut { } } - /// The same stream again: for `2>&1` and `1>&2`, and for the second stream + /// The same stream again: for `2>&1` and `1>&2`, and for the other stream /// of `&> file`. A `Buf` keeps its target, so stderr after `2>&1` appends /// to the stdout buffer. pub(crate) fn dup(&self) -> CmdOut { @@ -438,44 +437,54 @@ impl CmdOut { } } - /// Makes a stream that `Cmd::apply_redirect` set up for a child process - /// into one that the shell itself can write to: the command did not start, - /// and the shell reports that on it. A target that cannot be written to - /// becomes `Ignore`. - pub(crate) fn make_writable(&mut self, interp: &Interpreter) { - let fd = match core::mem::replace(self, CmdOut::Ignore) { - // The `IOWriter` takes over the fd. - CmdOut::File(file) => Ok(file.into_raw()), - // The owner keeps its fd. The `IOWriter` closes a copy. - CmdOut::BorrowedFd(fd) => shell_dup(fd), - CmdOut::JsBuf(idx) => { - if let Some(buf) = interp - .js_object(idx) - .and_then(|(global, value)| PinnedArrayBuffer::root(global, value)) - { - *self = CmdOut::ArrayBuf(Box::new(ArrayBufSink { buf, i: 0 })); - } - return; - } - writable @ (CmdOut::Fd(_) | CmdOut::Buf(_) | CmdOut::ArrayBuf(_) | CmdOut::Ignore) => { - *self = writable; - return; - } - }; - let Ok(fd) = fd else { return }; + /// A stream that writes to `fd` and closes it. The fd keeps the blocking + /// mode that it has, which its other users share. A FIFO, a socket or a + /// tty that is full is waited for with a poll, so it does not stop this + /// thread. + pub(crate) fn writer(interp: &Interpreter, fd: Fd) -> CmdOut { let writer = IOWriter::init( fd, io_writer::Flags { - pollable: cfg!(windows), + pollable: is_pollable(fd), ..Default::default() }, interp.event_loop, ); writer.set_interp(interp.as_ctx_ptr()); - *self = CmdOut::Fd(OutFd { + CmdOut::Fd(OutFd { writer, captured: None, - }); + }) + } + + /// Makes the stderr that `Cmd::apply_redirect` set up for a child process + /// into one that the shell itself can write to: the command did not start, + /// and the shell reports that on it. `Err`: a JS error is thrown. + pub(crate) fn make_writable(&mut self, interp: &Interpreter) -> JsResult<()> { + match self { + CmdOut::Fd(_) | CmdOut::Buf(_) | CmdOut::ArrayBuf(_) | CmdOut::Ignore => {} + CmdOut::JsBuf(idx) => { + // `Cmd::apply_redirect` stored the index after this lookup worked. + let Some((global, value)) = interp.js_object(*idx) else { + *self = CmdOut::Ignore; + return Ok(()); + }; + let Some(buf) = PinnedArrayBuffer::root(global, value) else { + return Err(global.throw_out_of_memory()); + }; + *self = CmdOut::ArrayBuf(Box::new(ArrayBufSink { buf, i: 0 })); + } + CmdOut::File(_) => { + // The writer takes over the fd. + if let CmdOut::File(file) = core::mem::replace(self, CmdOut::Ignore) { + *self = CmdOut::writer(interp, file.into_raw()); + } + } + CmdOut::BorrowedFd(_) => { + unreachable!("stderr owns the fd of its target; see `Redirected::set_out`") + } + } + Ok(()) } /// The stdio that a child process gets for this stream. @@ -483,7 +492,6 @@ impl CmdOut { fn to_subproc_stdio( &self, interp: &Interpreter, - kind: StdioKind, shellio: &mut Option>, redirect_buf: &mut Option, ) -> JsResult { @@ -506,17 +514,22 @@ impl CmdOut { if fd != Fd::INVALID { Stdio::Fd(fd) } else { - // Windows: fd was moved to libuv → inherit (libuv - // already manages it). On POSIX `IOWriter::fd()` is - // always the live fd, so this branch is unreachable. - Stdio::Inherit + // Windows: fd was moved to libuv. The shell's own + // stdout and stderr writers are on the streams of + // this process, which the child gets by name: after + // `2>&1` or `1>&2` the stream is not at its own slot. + // Another writer is inherited, as before. On POSIX + // `IOWriter::fd()` is always the live fd, so this + // branch is unreachable. + match interp.process_stream(&val.writer) { + Some(fd) => Stdio::Fd(fd), + None => Stdio::Inherit, + } } } } CmdOut::Buf(_) => Stdio::Pipe, CmdOut::File(file) => Stdio::Fd(file.handle()), - // `> ${Bun.stdout}` is the stdout that the child has already, as in `Stdio::extract_blob`. - CmdOut::BorrowedFd(fd) if *fd == kind.to_fd() => Stdio::Inherit, CmdOut::BorrowedFd(fd) => Stdio::Fd(*fd), CmdOut::JsBuf(idx) => { // `Cmd::apply_redirect` stored the index after this lookup worked. diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index d3fec6e35ea4..856baf0012e7 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -1118,6 +1118,21 @@ impl Interpreter { self.root_io.get() } + /// The stdout or stderr of this process, when `writer` is the writer that + /// `setup_io_before_run` made on a copy of it. + pub(crate) fn process_stream(&self, writer: &std::sync::Arc) -> Option { + use crate::shell::io::OutKind; + let io = self.root_io(); + let is_writer = |out: &OutKind| matches!(out, OutKind::Fd(fd) if std::sync::Arc::ptr_eq(&fd.writer, writer)); + if is_writer(&io.stdout) { + Some(Fd::stdout()) + } else if is_writer(&io.stderr) { + Some(Fd::stderr()) + } else { + None + } + } + pub(crate) fn compute_estimated_size_for_gc(&self) -> usize { let mut size = core::mem::size_of::(); size += self.args.get().memory_cost(); @@ -2241,7 +2256,7 @@ fn open_null_device() -> bun_sys::Result { /// .file.mode`; `EventLoopHandle` is still a shim, so we `fstat` the (already /// dup'd) fd here instead. On `fstat` failure we conservatively return `false` /// (non-pollable → synchronous write path), matching Windows behavior. -fn is_pollable(fd: Fd) -> bool { +pub(crate) fn is_pollable(fd: Fd) -> bool { #[cfg(windows)] { let _ = fd; diff --git a/src/runtime/shell/states/Cmd.rs b/src/runtime/shell/states/Cmd.rs index b6ce58adef51..187e199715d6 100644 --- a/src/runtime/shell/states/Cmd.rs +++ b/src/runtime/shell/states/Cmd.rs @@ -7,7 +7,8 @@ use crate::shell::ExitCode; use crate::shell::ast; use crate::shell::builtin::{Builtin, Kind as BuiltinKind}; use crate::shell::interpreter::{ - Interpreter, Node, NodeId, ShellExecEnv, StateKind, is_pollable_from_mode, log, shell_openat, + Interpreter, Node, NodeId, ShellExecEnv, StateKind, is_pollable_from_mode, log, shell_dup, + shell_openat, }; use crate::shell::io::{ArrayBufSink, CmdIO, CmdIn, CmdOut, IO, JsObject, OutFd}; use crate::shell::io_reader::IOReader; @@ -87,8 +88,10 @@ impl Redirected { fn set_out(&mut self, streams: (bool, bool), out: CmdOut) { match streams { (true, true) => { - self.stderr = Some(out.dup()); - self.stdout = Some(out); + // stderr owns a `File`: the message about a command that does + // not start is then written with no copy of the fd. + self.stdout = Some(out.dup()); + self.stderr = Some(out); } (true, false) => self.stdout = Some(out), (false, true) => self.stderr = Some(out), @@ -123,8 +126,8 @@ impl RedirectError { /// Whether a redirect to a target is for stdout and for stderr. /// /// A target after `2>&1` or `1>&2` comes from the parser reading the next -/// word as one. Each exec form keeps what it did with it: a child gets both -/// streams on the target, a builtin gets only the stream that the flags name. +/// word as one. A child gets both streams on the target. A builtin gets only +/// the stream that the flags name. fn out_streams(flags: ast::RedirectFlags, form: ExecForm) -> (bool, bool) { let both = matches!(form, ExecForm::Spawn) && flags.duplicate_out(); (flags.stdout() || both, flags.stderr() || both) @@ -866,7 +869,7 @@ impl Cmd { redirected.stdin = Some(CmdIn::Fd(reader)); }) } - ExecForm::Builtin => Self::open_writer(interp, cwd_fd, &path, flags.to_flags()) + ExecForm::Builtin => Self::open_writer(interp, cwd_fd, path, flags.to_flags()) .map(|out| redirected.set_out(streams, out)), ExecForm::Spawn => shell_openat(cwd_fd, path, flags.to_flags(), 0o666).map(|fd| { let file = bun_sys::File::from_fd(fd); @@ -883,11 +886,11 @@ impl Cmd { } } - /// Opens `target`, a path or an fd, for a builtin to write to. - fn open_writer( + /// Opens the file at `path` for a builtin to write to. + fn open_writer( interp: &Interpreter, cwd_fd: bun_sys::Fd, - target: &P, + path: &bun_core::ZStr, flags: i32, ) -> bun_sys::Result { let mut pollable = false; @@ -895,7 +898,7 @@ impl Cmd { let mut is_nonblocking = false; let fd = bun_io::open_for_writing_impl( cwd_fd, - target, + &path, flags, 0o666, &mut pollable, @@ -1002,25 +1005,28 @@ impl Cmd { JsObject::Response(response) => { let body = response.get_body_value(); body.to_blob_if_possible(); - match body.use_as_any_blob() { - crate::webcore::blob::Any::Blob(blob) => blob_file(&blob), + // Only a body that is a file is taken. A target that is + // rejected below keeps its body. + let file = match &*body { + crate::webcore::body::Value::Blob(blob) => blob_file(blob), _ => None, + }; + if file.is_some() { + drop(body.use_as_any_blob()); } + file } }; match file { Some(BlobFile::Fd(fd)) => { + // The command gets a copy of the fd. The caller keeps its own. + let fd = match shell_dup(fd) { + Ok(fd) => fd, + Err(e) => return Err(RedirectError::open(&e, b"Bun.file(fd)")), + }; let out = match form { - // The builtin writes to a copy of the fd. The caller keeps its own. - ExecForm::Builtin => { - let cwd_fd = interp.as_cmd(this).base.shell().cwd_fd; - let target = bun_io::PathOrFileDescriptor::Fd(fd); - match Self::open_writer(interp, cwd_fd, &target, flags.to_flags()) { - Ok(out) => out, - Err(e) => return Err(RedirectError::open(&e, b"Bun.file(fd)")), - } - } - ExecForm::Spawn => CmdOut::BorrowedFd(fd), + ExecForm::Builtin => CmdOut::writer(interp, fd), + ExecForm::Spawn => CmdOut::File(bun_sys::File::from_fd(fd)), }; redirected.set_out(streams, out); Ok(redirected) diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index 8df3a9445168..d46b4f9489aa 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -2151,6 +2151,14 @@ describe("deno_task", () => { ); }); + // The command gets a copy of the fd when the redirect is applied. + test.skipIf(isWindows)("2> Bun.file(fd) with an fd that is not open", async () => { + await sameAsBuiltin( + name => $`${name} 2> ${Bun.file(2 ** 30)}`, + stderr => expect(stderr).toContain("Bun.file(fd)"), + ); + }); + test("no command name", async () => { using dir = tempDir("shell-redirect-fails", {}); const builtin = await $`echo < missing_input_zz`.cwd(String(dir)).quiet(); @@ -2198,10 +2206,11 @@ describe("deno_task", () => { const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); const builtin = "rejected Cannot redirect stdout/stderr to an immutable blob. Expected a file"; const external = "rejected Blobs are immutable, and cannot be used for stdout/stderr"; - const rejected = expect.stringMatching(/^rejected /); + // The message names the stream: the command was not searched for. + const stream = expect.stringMatching(/^rejected .*ReadableStream/); expect({ result: JSON.parse(stdout || "null"), stderr, exitCode }).toEqual({ result: { - stream: [rejected, rejected, rejected], + stream: [stream, stream, stream], blob: [builtin, external, external], response: [builtin, external, external], }, @@ -2209,6 +2218,20 @@ describe("deno_task", () => { exitCode: 0, }); }); + + // Only a Response of a file is a target. The others are not read. + test("a Response that is rejected keeps its body", async () => { + const results = []; + for (const name of ["echo", BUN, "nosuch_cmd_zz"]) { + const response = new Response("keep"); + const rejected = await $`${name} --version > ${response}`.quiet().then( + () => false, + () => true, + ); + results.push({ rejected, bodyUsed: response.bodyUsed, body: await response.text() }); + } + expect(results).toEqual(Array(3).fill({ rejected: true, bodyUsed: false, body: "keep" })); + }); }); }); @@ -2244,6 +2267,16 @@ describe("deno_task", () => { .testMini() .runAsTest("external 2>&1 in a command substitution"); + // The shell wrote to the stream before the command gets it. + TestBuilder.command`echo first; ${BUN} -e ${both} 2>&1` + .stdout("first\nout\nerr\n") + .testMini() + .runAsTest("external 2>&1 after a builtin wrote to stdout"); + TestBuilder.command`echo first 1>&2; ${BUN} -e ${both} 1>&2` + .stderr("first\nout\nerr\n") + .testMini() + .runAsTest("external 1>&2 after a builtin wrote to stderr"); + TestBuilder.command`ls nosuch_dir_zz 2>> err.txt` .file("err.txt", "pre\n") .exitCode(1) @@ -2275,14 +2308,19 @@ describe("deno_task", () => { using dir = tempDir("shell-redirect-fd", {}); const path = join(String(dir), "out.txt"); const fd = openSync(path, "w"); + // The command writes to a copy of the fd. The fd of the caller keeps its mode: no O_NONBLOCK. + const flags = () => + isLinux ? readFileSync(`/proc/self/fdinfo/${fd}`, "utf8").match(/^flags:\s*\d+/m)?.[0] : ""; try { + const before = flags(); const { stdout, stderr, exitCode } = await run(Bun.file(fd)).quiet(); expect({ stdout: stdout.toString(), stderr: stderr.toString(), file: readFileSync(path, "utf8"), + flags: flags(), exitCode, - }).toEqual({ stdout: "", stderr: "", file: "hi\n", exitCode: 0 }); + }).toEqual({ stdout: "", stderr: "", file: "hi\n", flags: before, exitCode: 0 }); } finally { closeSync(fd); } @@ -2326,6 +2364,72 @@ describe("deno_task", () => { .stderr("bun: ambiguous redirect: at `bun`\n") .runAsTest("an ambiguous redirect names the command as typed"); }); + + // A redirect gives the command another stream in place of the pipe. The + // shell then closes its end of that pipe, as bash and dash do. It does + // not keep the end open until the command exits. + describe("a redirect in place of a pipe", () => { + // Each command waits for a file that the other command writes. + const waitFor = (file: string) => ` + const deadline = Date.now() + 10_000; + while (!existsSync(${JSON.stringify(file)})) { + if (Date.now() > deadline) process.exit(9); + await Bun.sleep(1); + }`; + + test("< file: a write to the pipe fails while the command runs", async () => { + using dir = tempDir("shell-redirect-pipe", { "in.txt": "from the file\n" }); + const writer = ` + import { existsSync, writeFileSync, writeSync } from "fs"; + ${waitFor("reader-runs")} + let result = "wrote"; + try { + writeSync(1, "x"); + } catch { + result = "failed"; + } + writeFileSync("writer-result", result);`; + // The shell starts this command after it applied the redirect. + const reader = ` + import { existsSync, writeFileSync } from "fs"; + writeFileSync("reader-runs", ""); + ${waitFor("writer-result")} + process.stdout.write(await Bun.stdin.text());`; + const { stdout, stderr, exitCode } = await $`${BUN} -e ${writer} | ${BUN} -e ${reader} < in.txt` + .cwd(String(dir)) + .quiet() + .nothrow(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + writer: readFileSync(join(String(dir), "writer-result"), "utf8"), + exitCode, + }).toEqual({ stdout: "from the file\n", stderr: "", writer: "failed", exitCode: 0 }); + }); + + test("> file: the reader of the pipe gets the end of input while the command runs", async () => { + using dir = tempDir("shell-redirect-pipe", {}); + const writer = ` + import { existsSync } from "fs"; + ${waitFor("reader-got-end")} + console.log("to the file");`; + const reader = ` + import { writeFileSync } from "fs"; + const input = await Bun.stdin.text(); + writeFileSync("reader-got-end", ""); + console.log("end of input after " + input.length + " bytes");`; + const { stdout, stderr, exitCode } = await $`${BUN} -e ${writer} > out.txt | ${BUN} -e ${reader}` + .cwd(String(dir)) + .quiet() + .nothrow(); + expect({ + stdout: stdout.toString(), + stderr: stderr.toString(), + file: readFileSync(join(String(dir), "out.txt"), "utf8"), + exitCode, + }).toEqual({ stdout: "end of input after 0 bytes\n", stderr: "", file: "to the file\n", exitCode: 0 }); + }); + }); }); describe("pwd", async () => { diff --git a/test/js/bun/shell/file-io.test.ts b/test/js/bun/shell/file-io.test.ts index 5b10f0351a8e..b8866bd7f337 100644 --- a/test/js/bun/shell/file-io.test.ts +++ b/test/js/bun/shell/file-io.test.ts @@ -125,6 +125,84 @@ describe("IOWriter file output redirection", () => { fs.closeSync(holder); } }); + + // The target of a command that is not a builtin is open as a child needs + // it: blocking. The shell's own message about a command that does not + // start must wait for a FIFO that is full, not block the thread in write(2). + test.concurrent.skipIf(!isPosix)("message of a command that does not start waits for a full FIFO", async () => { + using dir = tempDir("shell-fifo-redirect-message", { + "fixture.ts": /* ts */ ` + import { $ } from "bun"; + import * as fs from "node:fs"; + const fifo = process.env.FIFO!; + const rfd = fs.openSync(fifo, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); + const wfd = fs.openSync(fifo, fs.constants.O_WRONLY | fs.constants.O_NONBLOCK); + // Fill the pipe, so that the message does not fit until this script reads. + let filled = 0; + for (const chunk of [Buffer.alloc(4096, "#"), Buffer.alloc(1, "#")]) { + try { while (true) filled += fs.writeSync(wfd, chunk); } catch {} + } + + let shell: Awaited> | undefined; + const pending = $\`nosuch_cmd_zz 2> \${fifo}\`.quiet().nothrow().then(r => (shell = r)); + + // The only reader is this thread. It gets here only if the shell did not block. + const buf = Buffer.alloc(65536); + const chunks: Buffer[] = []; + const read = () => { + let n = 0; + try { n = fs.readSync(rfd, buf); } catch (e: any) { if (e.code !== "EAGAIN") throw e; } + if (n > 0) chunks.push(Buffer.from(buf.subarray(0, n))); + return n; + }; + while (!shell) { + read(); + await new Promise(r => setImmediate(r)); + } + await pending; + fs.closeSync(wfd); + while (read() > 0) {} + fs.closeSync(rfd); + console.log( + JSON.stringify({ + exitCode: shell!.exitCode, + stderr: shell!.stderr.toString(), + message: Buffer.concat(chunks).subarray(filled).toString(), + }), + ); + `, + }); + const fifo = join(String(dir), "err.fifo"); + await using mk = Bun.spawn({ cmd: [Bun.which("mkfifo")!, fifo], env: bunEnv }); + await mk.exited; + + // Hold a read end in the parent so the FIFO survives a child abort + // without the tempDir teardown racing an open writer. + const holder = fs.openSync(fifo, fs.constants.O_RDONLY | fs.constants.O_NONBLOCK); + try { + await using proc = Bun.spawn({ + cmd: [bunExe(), "fixture.ts"], + cwd: String(dir), + env: { ...bunEnv, FIFO: fifo }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + let parsed: unknown; + try { + parsed = JSON.parse(stdout.trim().split("\n").pop() ?? ""); + } catch { + parsed = stdout; + } + expect({ parsed, stderr, exitCode }).toEqual({ + parsed: { exitCode: 1, stderr: "", message: "bun: command not found: nosuch_cmd_zz\n" }, + stderr: "", + exitCode: 0, + }); + } finally { + fs.closeSync(holder); + } + }); }); describe("writer queue and bump behavior", () => { diff --git a/test/js/bun/shell/shell-worker-terminate-leak.test.ts b/test/js/bun/shell/shell-worker-terminate-leak.test.ts index ec6caa15c400..b7738429e423 100644 --- a/test/js/bun/shell/shell-worker-terminate-leak.test.ts +++ b/test/js/bun/shell/shell-worker-terminate-leak.test.ts @@ -183,59 +183,52 @@ test.skipIf(!isASAN || isWindows)( LEAK_TEST_TIMEOUT, ); -test.skipIf(!isLinux)( - "worker.terminate() with a live Bun.$ file redirect closes the redirection fd", - async () => { - // A leak checker does not see an fd that stays open, so read /proc. - using dir = tempDir("shell-worker-terminate-fd", { - "main.ts": ` - import { readdirSync, readlinkSync } from "node:fs"; - const holdsOut = () => - readdirSync("/proc/self/fd").some(fd => { - try { - return readlinkSync("/proc/self/fd/" + fd).endsWith("/out.txt"); - } catch { - return false; - } - }); - const worker = new Worker(new URL("./worker.ts", import.meta.url).href); - const { promise, resolve, reject } = Promise.withResolvers(); - worker.onmessage = resolve; - worker.onerror = e => reject(e.error ?? e.message); - await promise; - const before = holdsOut(); - await worker.terminate(); - const deadline = Date.now() + 20_000; - while (holdsOut() && Date.now() < deadline) await Bun.sleep(10); - console.log(JSON.stringify({ before, after: holdsOut() })); - `, - "worker.ts": ` - const running = Bun.$\`sh -c "echo ok > flag.txt && exec sleep 100" > out.txt\`.nothrow(); - (async () => { - await running; - })(); - while (!(await Bun.file("flag.txt").exists())) { - await Bun.sleep(5); - } - postMessage("ok"); - `, - }); - await using proc = Bun.spawn({ - cmd: [bunExe(), "main.ts"], - cwd: String(dir), - env: bunEnv, - stdout: "pipe", - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, stderr, exitCode }).toEqual({ - stdout: JSON.stringify({ before: true, after: false }) + "\n", - stderr: "", - exitCode: 0, - }); - }, - 60_000, -); +test.skipIf(!isLinux)("worker.terminate() with a live Bun.$ file redirect closes the redirection fd", async () => { + // A leak checker does not see an fd that stays open, so read /proc. + using dir = tempDir("shell-worker-terminate-fd", { + "main.ts": ` + import { readdirSync, readlinkSync } from "node:fs"; + const holdsOut = () => + readdirSync("/proc/self/fd").some(fd => { + try { + return readlinkSync("/proc/self/fd/" + fd).endsWith("/out.txt"); + } catch { + return false; + } + }); + const worker = new Worker(new URL("./worker.ts", import.meta.url).href); + const started = Promise.withResolvers(); + worker.onmessage = started.resolve; + worker.onerror = e => started.reject(e.error ?? e.message); + // The worker's VM is destroyed, and the shell interpreter with it, before "close". + const closed = Promise.withResolvers(); + worker.addEventListener("close", closed.resolve); + await started.promise; + const before = holdsOut(); + worker.terminate(); + await closed.promise; + console.log(JSON.stringify({ before, after: holdsOut() })); + `, + "worker.ts": ` + // \`then\` starts the interpreter. It opens out.txt and starts the child before it returns. + Bun.$\`sleep 100 > out.txt\`.nothrow().then(() => {}); + postMessage("ok"); + `, + }); + await using proc = Bun.spawn({ + cmd: [bunExe(), "main.ts"], + cwd: String(dir), + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ before: true, after: false }) + "\n", + stderr: "", + exitCode: 0, + }); +}); test.skipIf(!isLinux)( "worker.terminate() kills a live Bun.$ subprocess", From 2d54363c170867428495ea01695575ddcc6a7357 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 17 Oct 2026 21:28:02 +0000 Subject: [PATCH 4/5] shell: shorten the comments of the redirect step Every comment that the change adds or edits is one line now. A few that repeated the code are gone. --- src/runtime/shell/Builtin.rs | 17 ++---- src/runtime/shell/IO.rs | 89 +++++++------------------------- src/runtime/shell/builtin/yes.rs | 3 +- src/runtime/shell/interpreter.rs | 7 +-- src/runtime/shell/states/Cmd.rs | 62 ++++++---------------- src/runtime/shell/subproc.rs | 3 +- src/shell_parser/parse.rs | 4 +- 7 files changed, 43 insertions(+), 142 deletions(-) diff --git a/src/runtime/shell/Builtin.rs b/src/runtime/shell/Builtin.rs index 168ffba2c059..265cdab442fd 100644 --- a/src/runtime/shell/Builtin.rs +++ b/src/runtime/shell/Builtin.rs @@ -257,8 +257,7 @@ impl Builtin { bun_core::ZStr::from_cstr(unsafe { core::ffi::CStr::from_ptr(p) }) } - /// Construct a `Builtin` for `kind` and install it into the owning Cmd's - /// `exec` slot. The caller then calls `Builtin::start`. + /// Installs a `Builtin` for `kind` in the owning Cmd's `exec` slot. pub(crate) fn init(interp: &Interpreter, cmd: NodeId, kind: Kind) { use crate::shell::states::cmd::Exec; @@ -280,11 +279,7 @@ impl Builtin { })); } - /// Writes a message of the shell about the command to the command's - /// stderr, and finishes the command with exit code 1. Before - /// `Cmd::apply_redirect` that is the stderr the command inherits: an - /// expansion error and a redirect that cannot be set up are reported - /// there. After it, it is the stderr that the redirect gave the command. + /// Writes the shell's message to the command's stderr. The command ends with exit code 1. pub(crate) fn cmd_write_failing_error( interp: &Interpreter, cmd: NodeId, @@ -307,9 +302,7 @@ impl Builtin { let child = io_writer::ChildPtr::new(cmd, io_writer::WriterTag::Cmd); return interp.as_cmd(cmd).io.stderr.enqueue(child, &buf, safeguard); } - // No-IO path: the message goes to the captured buffer or the - // `> ${buffer}` target, and the command finishes synchronously - // (Cmd::on_io_writer_chunk's behaviour). + // No IOWriter: write to the captured buffer or the `> ${buffer}` target and finish now. let me = interp.as_cmd_mut(cmd); let shell = me.base.shell; // SAFETY: `shell` is `me.base.shell`, live for the Cmd's lifetime. @@ -558,7 +551,3 @@ impl Builtin { Self::done(interp, cmd, exit_code) } } - -// Cleanup: every `Impl` variant owns its state via `Box`/`Vec`/`Arc`. So -// cleanup is fully covered by `Drop` on `Box` (called from -// `Cmd::deinit`). No explicit deinit needed. diff --git a/src/runtime/shell/IO.rs b/src/runtime/shell/IO.rs index 921970f09e3e..1e9de0dba410 100644 --- a/src/runtime/shell/IO.rs +++ b/src/runtime/shell/IO.rs @@ -1,8 +1,7 @@ //! Carries stdin/stdout/stderr for a state node. //! //! `IO` is a plain `Clone` value; `IOReader`/`IOWriter` are `Arc`-refcounted. -//! A command turns the `IO` it inherits into a [`CmdIO`] and applies its -//! redirect to that. +//! A command applies its redirect to a [`CmdIO`] made from the `IO` it inherits. use std::sync::Arc; @@ -105,14 +104,7 @@ impl OutKind { } } -// ────────────────────────────────────────────────────────────────────────── -// CmdIO -// ────────────────────────────────────────────────────────────────────────── - -/// stdin, stdout and stderr of one command. `Cmd::init` builds it from the -/// `IO` that the command inherits. `Cmd::apply_redirect` then points the -/// redirected streams at their target. A builtin, a child process and the -/// shell's own messages about the command all use these streams. +/// The streams of one command. `Cmd::apply_redirect` points them at the target of its redirect. pub(crate) struct CmdIO { pub(crate) stdin: CmdIn, pub(crate) stdout: CmdOut, @@ -129,8 +121,7 @@ pub(crate) enum CmdIn { ArrayBuf(Box), /// `< ${blob}` or `< ${response}` for a builtin. Blob(Box), - /// `< ${object}` for a command that is not a builtin: the index of the - /// object in `Interpreter::jsobjs`. The spawn reads it. + /// `< ${object}` for a child process: the index of the object in `Interpreter::jsobjs`. Js(u32), Ignore, } @@ -141,15 +132,13 @@ pub(crate) enum CmdOut { Fd(OutFd), /// The captured buffer of the shell env: `_buffered_stdout` or `_buffered_stderr`. Buf(IoKind), - /// `> file` for a command that is not a builtin: the file, or a copy of - /// the fd of a `Bun.file(fd)`. The fd stays blocking, as a child needs it. + /// `> file` or `> ${Bun.file(fd)}` for a child process: an fd that stays blocking. File(bun_sys::File), /// The fd of the `File` that the other stream owns: `&> file`. BorrowedFd(Fd), /// `> ${buffer}` for a builtin. ArrayBuf(Box), - /// `> ${buffer}` for a command that is not a builtin: the index of the - /// buffer in `Interpreter::jsobjs`. The spawn pins it. + /// `> ${buffer}` for a child process: the index of the buffer in `Interpreter::jsobjs`. JsBuf(u32), Ignore, } @@ -199,10 +188,7 @@ impl From for CmdIO { } impl CmdIO { - /// Maps the streams onto the stdio of a child process. `shellio` keeps the - /// `IOWriter` of a captured stream, so that `PipeReader` can also write - /// what it reads to it. `redirect_stdout`/`redirect_stderr` get the pin of - /// a `> ${buffer}` target. + /// Maps the streams onto the stdio of a child process. #[inline] pub(crate) fn to_subproc_stdio( &self, @@ -222,9 +208,7 @@ impl CmdIO { } else { // `2>&1` or `1>&2`: the child gets the one stream on stdout. match &stdio[1] { - // `Dup2` on Windows is a pipe that this process reads for the - // child (`spawn_process_windows`), and only a captured stdout - // has a reader. An fd goes to the child on both streams. + // Windows: `Dup2` is a pipe that only a captured stdout reads. Stdio::Fd(fd) if cfg!(windows) => Stdio::Fd(*fd), _ => Stdio::Dup2(Dup2 { out: StdioKind::Stderr, @@ -235,10 +219,7 @@ impl CmdIO { Ok(()) } - /// `PinnedArrayBuffer::drop`'s unpin would write to a `JSC::ArrayBuffer` - /// impl the heap sweep already deleted; see - /// `ShellSubprocess::defuse_array_buffer_unpins`. VM-shutdown finalizer - /// only. + /// VM-shutdown finalizer only; see `ShellSubprocess::defuse_array_buffer_unpins`. #[cfg(not(windows))] pub(crate) fn defuse_array_buf_pins(&mut self) { if let CmdIn::ArrayBuf(buf) = &mut self.stdin { @@ -270,8 +251,7 @@ impl CmdIn { }; match JsObject::from_value(value) { Some(JsObject::ArrayBuffer(value)) => { - // The child gets the bytes that the buffer holds now. - // An empty buffer is EOF, as in `Stdio::extract_blob`. + // A copy of the bytes now. Empty is EOF, as in `Stdio::extract_blob`. if let Some(buf) = value.as_array_buffer(global) && !buf.byte_slice().is_empty() { @@ -310,9 +290,7 @@ impl CmdOut { } } - /// The same stream again: for `2>&1` and `1>&2`, and for the other stream - /// of `&> file`. A `Buf` keeps its target, so stderr after `2>&1` appends - /// to the stdout buffer. + /// The same stream again: for `2>&1`, `1>&2` and the other stream of `&> file`. pub(crate) fn dup(&self) -> CmdOut { match self { CmdOut::Fd(fd) => CmdOut::Fd(fd.clone()), @@ -342,8 +320,7 @@ impl CmdOut { } } - /// The buffer that gets the output of a child when the pipe it wrote to - /// closes: the JS-side capture of an `Fd`, or the shell env's buffer. + /// Where the output that a child wrote to a pipe is captured. pub(crate) fn capture_buf(&self, shell: &mut ShellExecEnv) -> Option<*mut Vec> { match self { CmdOut::Fd(fd) => fd.captured, @@ -352,15 +329,7 @@ impl CmdOut { } } - /// Writes `buf` without an `IOWriter`: to the captured buffer, to the - /// `> ${buffer}` target, or nowhere. - /// - /// Returns `Err(ENOSPC)` when an ArrayBuffer target is already full. - /// **WARNING**: caller must have checked `needs_io() == None` first. - /// - /// # Safety - /// `shell` must point to the live `ShellExecEnv` of the command that owns - /// this stream (`cmd.base.shell`); only dereferenced for [`CmdOut::Buf`]. + /// `needs_io()` must be `None`. Safety: `shell` is the live env of the owning `Cmd`. pub(crate) unsafe fn write_no_io_to( &mut self, shell: *mut ShellExecEnv, @@ -374,16 +343,12 @@ impl CmdOut { panic!("write_no_io called on fd output; caller must check needs_io()") } CmdOut::Buf(target) => { - // Writes go straight through to `buffered_{stdout,stderr}`. - // SAFETY: caller contract — shell env outlives the Cmd node - // (single-threaded); the buffer is a live `ShellExecEnv` Bufio. + // SAFETY: caller contract: `shell` is live, and so is its buffer. unsafe { (*env_buf(&mut *shell, *target)).append_slice(buf) }; Ok(buf.len()) } CmdOut::ArrayBuf(sink) => { - // `len = buf.len` stays usize so `i + len > byte_len` is - // computed at usize width and cannot overflow; only the - // stored cursor is u32. + // usize arithmetic: only the stored cursor is u32. let idx = sink.i as usize; let total = sink.buf.byte_len; if idx >= total { @@ -405,11 +370,7 @@ impl CmdOut { } } - /// Queue `buf` on this stream's IOWriter and arrange for `child`'s - /// `on_io_writer_chunk` to fire when the chunk completes. Delegates to - /// `fd.writer.enqueue` passing `fd.captured` as the tee bytelist. - /// - /// `_safeguard` proves the caller checked `needs_io()`. + /// Queues `buf` on the IOWriter; `child` gets `on_io_writer_chunk` when it completes. pub(crate) fn enqueue( &self, child: io_writer::ChildPtr, @@ -422,8 +383,7 @@ impl CmdOut { } } - /// Format with the optional `"{kind}: "` prefix and enqueue on the - /// underlying IOWriter. + /// Formats with the optional `"{kind}: "` prefix and enqueues on the IOWriter. pub(crate) fn enqueue_fmt( &self, child: io_writer::ChildPtr, @@ -437,10 +397,7 @@ impl CmdOut { } } - /// A stream that writes to `fd` and closes it. The fd keeps the blocking - /// mode that it has, which its other users share. A FIFO, a socket or a - /// tty that is full is waited for with a poll, so it does not stop this - /// thread. + /// A stream that writes to `fd` and closes it. The blocking mode of the fd is not changed. pub(crate) fn writer(interp: &Interpreter, fd: Fd) -> CmdOut { let writer = IOWriter::init( fd, @@ -457,9 +414,7 @@ impl CmdOut { }) } - /// Makes the stderr that `Cmd::apply_redirect` set up for a child process - /// into one that the shell itself can write to: the command did not start, - /// and the shell reports that on it. `Err`: a JS error is thrown. + /// Makes a stderr set up for a child writable by the shell. `Err`: a JS error is thrown. pub(crate) fn make_writable(&mut self, interp: &Interpreter) -> JsResult<()> { match self { CmdOut::Fd(_) | CmdOut::Buf(_) | CmdOut::ArrayBuf(_) | CmdOut::Ignore => {} @@ -514,13 +469,7 @@ impl CmdOut { if fd != Fd::INVALID { Stdio::Fd(fd) } else { - // Windows: fd was moved to libuv. The shell's own - // stdout and stderr writers are on the streams of - // this process, which the child gets by name: after - // `2>&1` or `1>&2` the stream is not at its own slot. - // Another writer is inherited, as before. On POSIX - // `IOWriter::fd()` is always the live fd, so this - // branch is unreachable. + // Windows: libuv owns the fd now. Name the process stream if it is one. match interp.process_stream(&val.writer) { Some(fd) => Stdio::Fd(fd), None => Stdio::Inherit, diff --git a/src/runtime/shell/builtin/yes.rs b/src/runtime/shell/builtin/yes.rs index d901fa0e07b4..7a8112b0c5ef 100644 --- a/src/runtime/shell/builtin/yes.rs +++ b/src/runtime/shell/builtin/yes.rs @@ -137,8 +137,7 @@ impl Yes { safeguard: OutputNeedsIOSafeGuard, ) -> Yield { let child = ChildPtr::new(cmd, WriterTag::Builtin); - // `stdout` and the builtin are disjoint fields of the `Cmd` — - // split-borrow so the tiled buffer is enqueued zero-copy. + // Split-borrow the Cmd's stdout and the builtin: the tiled buffer is enqueued zero-copy. let (stdout, yes) = Self::split_stdout_state(interp.as_cmd_mut(cmd)); stdout.enqueue(child, &yes.buffer[..yes.buffer_used], safeguard) } diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index 856baf0012e7..2513ddb30f1b 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -1118,8 +1118,7 @@ impl Interpreter { self.root_io.get() } - /// The stdout or stderr of this process, when `writer` is the writer that - /// `setup_io_before_run` made on a copy of it. + /// `Fd::stdout()` or `Fd::stderr()` when `writer` is the shell's own writer on that stream. pub(crate) fn process_stream(&self, writer: &std::sync::Arc) -> Option { use crate::shell::io::OutKind; let io = self.root_io(); @@ -1173,9 +1172,7 @@ impl Interpreter { Some(crate::jsc::JSGlobalObject::opaque_ref(g)) } - /// The `${...}` object number `idx` of the script, and the global it - /// lives in. `None` when the script has no such object: its text can - /// spell any number. + /// The `${...}` object number `idx` and its global. `None`: the script has no such object. pub(crate) fn js_object( &self, idx: u32, diff --git a/src/runtime/shell/states/Cmd.rs b/src/runtime/shell/states/Cmd.rs index 187e199715d6..750d616eff1c 100644 --- a/src/runtime/shell/states/Cmd.rs +++ b/src/runtime/shell/states/Cmd.rs @@ -88,8 +88,7 @@ impl Redirected { fn set_out(&mut self, streams: (bool, bool), out: CmdOut) { match streams { (true, true) => { - // stderr owns a `File`: the message about a command that does - // not start is then written with no copy of the fd. + // stderr owns a `File`, so `make_writable` needs no copy of the fd. self.stdout = Some(out.dup()); self.stderr = Some(out); } @@ -123,11 +122,7 @@ impl RedirectError { } } -/// Whether a redirect to a target is for stdout and for stderr. -/// -/// A target after `2>&1` or `1>&2` comes from the parser reading the next -/// word as one. A child gets both streams on the target. A builtin gets only -/// the stream that the flags name. +/// Which of (stdout, stderr) go to the target. A child gets both for a target after `2>&1`. fn out_streams(flags: ast::RedirectFlags, form: ExecForm) -> (bool, bool) { let both = matches!(form, ExecForm::Spawn) && flags.duplicate_out(); (flags.stdout() || both, flags.stderr() || both) @@ -244,8 +239,7 @@ impl BufferedIoClosed { /// /// `readable` is the subprocess's `stdout`/`stderr` `Readable`; if it was /// a pipe its buffered bytes are taken (ownership moves into - /// `state.Closed`) and also copied to `capture`, the buffer that holds - /// the captured output of the stream (`CmdOut::capture_buf`). + /// `state.Closed`) and also copied to `capture` (`CmdOut::capture_buf`). fn close_out( slot: &mut Option, readable: &mut Readable, @@ -260,9 +254,7 @@ impl BufferedIoClosed { }; if let Some(capture) = capture { let the_slice = pipe.slice(); - // SAFETY: `capture` points into `ShellExecEnv::_buffered_*`, - // which the owning Cmd's `base.shell` (or an env above it) keeps - // live for the duration of the command. Single-threaded. + // SAFETY: `capture` is a buffer of a live `ShellExecEnv` (`CmdOut::capture_buf`). unsafe { (*capture).append_slice(the_slice) }; } // SAFETY: `Arc` interior mutability — the shell is @@ -377,10 +369,8 @@ impl Cmd { } /// IOWriter completion callback for the error message written in - /// `WaitingWriteErr`: finish the Cmd with exit code 1. A write to the - /// inherited stderr that failed is thrown. A write to the stderr that the - /// redirect gave the command is not: like the output of a builtin, the - /// message is lost and the script goes on. + /// `WaitingWriteErr`: throw if the write to the inherited stderr failed, else finish the Cmd + /// with exit code 1. pub(crate) fn on_io_writer_chunk( interp: &Interpreter, this: NodeId, @@ -499,8 +489,8 @@ impl Cmd { Yield::Next(this) } - /// Applies the redirect, then resolves argv[0] to a builtin or falls - /// through to subprocess spawn (still gated). + /// Resolves argv[0] to a builtin or falls through to subprocess spawn + /// (still gated). fn transition_to_exec(interp: &Interpreter, this: NodeId) -> Yield { // NUL-terminate every arg so builtins can borrow them as `*const c_char`. for a in &mut interp.as_cmd_mut(this).args { @@ -520,8 +510,6 @@ impl Cmd { let builtin = first_arg.as_deref().and_then(BuiltinKind::from_argv0); // The redirect comes before the search for the command (POSIX 2.9.1). - // So every exit below has the redirected streams, and a redirect that - // fails is the only thing the command reports. let form = match builtin { Some(_) => ExecForm::Builtin, None => ExecForm::Spawn, @@ -531,8 +519,7 @@ impl Cmd { } let Some(first_arg) = first_arg else { - // Exit with the exit code from a sole command-substitution - // (stashed by `child_done` from `Expansion::out_exit_code`), else 0. + // The exit code of a sole command substitution (stashed by `child_done`), else 0. let me = interp.as_cmd(this); let exit = me.exit_code.unwrap_or(0); let parent = me.base.parent; @@ -759,14 +746,7 @@ impl Cmd { Yield::suspended() } - /// Performs the redirect of the command (`> file`, `< ${blob}`, `2>&1`, …) - /// on its `io`. Every user of the streams comes after it: a builtin, the - /// spawn of a child process, and the shell's message about a command that - /// does not start. - /// - /// `Some`: the redirect failed. `io` is as inherited, and the yield - /// finishes the command: the message is queued on the inherited stderr, - /// or a JS error is thrown. + /// Applies the redirect to `io`. `Some`: it failed, and the yield ends the command. fn apply_redirect( interp: &Interpreter, this: NodeId, @@ -779,8 +759,7 @@ impl Cmd { None => { let io = &interp.as_cmd(this).io; let mut redirected = Redirected::default(); - // The lexer keeps the stream that both become: `2>&1` is - // `STDOUT`, `1>&2` is `STDERR`. + // The flag names the stream that both become: `2>&1` is `STDOUT`. if flags.stdout() { redirected.stderr = Some(io.stdout.dup()); } @@ -847,9 +826,7 @@ impl Cmd { RedirectError::Message(message) } - /// The redirect names a file by its path: open it. A builtin reads it - /// through an `IOReader` and writes it through an `IOWriter`. A child - /// process gets the fd. + /// Opens the path that the redirect names, for a builtin or for a child process. fn redirect_to_path( interp: &Interpreter, this: NodeId, @@ -915,9 +892,7 @@ impl Cmd { use bun_sys::FdExt as _; fd.make_lib_uv_owned_for_syscall(bun_sys::Tag::open, bun_sys::ErrorCase::CloseOnFail)? }; - // Honor the `pollable` computed by `open_for_writing_impl` on - // POSIX so a FIFO/socket target (whose fd is now O_NONBLOCK) - // takes the pollable path; Windows keeps the async writer. + // POSIX polls a FIFO or socket target (now O_NONBLOCK). Windows keeps the async writer. let writer = IOWriter::init( fd, io_writer::Flags { @@ -1005,8 +980,7 @@ impl Cmd { JsObject::Response(response) => { let body = response.get_body_value(); body.to_blob_if_possible(); - // Only a body that is a file is taken. A target that is - // rejected below keeps its body. + // Take only a body that is a file: a rejected target keeps its body. let file = match &*body { crate::webcore::body::Value::Blob(blob) => blob_file(blob), _ => None, @@ -1072,8 +1046,7 @@ impl Cmd { if let Exec::Subproc(sub) = &me.exec && !sub.child.is_null() { - // SAFETY: `child` is the live heap::alloc'd subprocess; - // the call only writes its own fields. + // SAFETY: `child` is the live subprocess; the call only writes its own fields. unsafe { ShellSubprocess::defuse_array_buffer_unpins(sub.child) }; } } @@ -1186,8 +1159,7 @@ impl Cmd { self.exit_code = Some(e.errno.unsigned_abs() as ExitCode); } if let Exec::Subproc(sub) = &mut self.exec { - // Raw deref keeps the borrow disjoint from `sub.buffered_closed` below. - // SAFETY: `child` is the live subprocess owned by this Cmd. + // SAFETY: `child` is this Cmd's live subprocess; the raw deref keeps `sub` borrowable. let child = unsafe { &mut *sub.child }; let (out, slot, readable, stdio) = match kind { OutKind::Stdout => ( @@ -1203,8 +1175,6 @@ impl Cmd { StdioKind::Stderr, ), }; - // The stream the command has says where its output is captured: - // the JS-side buffer, or the buffer of the shell env. BufferedIoClosed::close_out(slot, readable, out.capture_buf(self.base.shell_mut())); child.close_io(stdio); } diff --git a/src/runtime/shell/subproc.rs b/src/runtime/shell/subproc.rs index fc6ee83f2d0a..bfe946f1ca19 100644 --- a/src/runtime/shell/subproc.rs +++ b/src/runtime/shell/subproc.rs @@ -1348,8 +1348,7 @@ impl Readable { *self = Readable::Closed; fd.close(); } - // .fd is borrowed from the Cmd's stream (see `CmdOut::to_subproc_stdio`): - // the shell's IOWriter or the target of a redirect. The owner closes it. + // .fd belongs to the Cmd's stream (`CmdOut::to_subproc_stdio`), which closes it. Readable::Fd => { *self = Readable::Closed; } diff --git a/src/shell_parser/parse.rs b/src/shell_parser/parse.rs index 22801455339e..83cbe82c71c0 100644 --- a/src/shell_parser/parse.rs +++ b/src/shell_parser/parse.rs @@ -439,9 +439,7 @@ pub mod ast { const STDOUT = 1 << 1; const STDERR = 1 << 2; const APPEND = 1 << 3; - /// Set with the stream that both streams become: - /// 2>&1 === stdout=true and duplicate_out=true - /// 1>&2 === stderr=true and duplicate_out=true + /// `2>&1` is STDOUT | DUPLICATE_OUT and `1>&2` is STDERR | DUPLICATE_OUT. const DUPLICATE_OUT = 1 << 4; } } From 673a44fbd43f5b49cce0b70c44caa80b94a07cad Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 17 Oct 2026 21:56:35 +0000 Subject: [PATCH 5/5] shell: a rejected Response keeps its stream, and 2>&1 on Windows never makes an unread pipe - A `Response` target is a file only when its body is a file `Blob`. The body is not converted first, so a stream body that is rejected is not read. - Windows: after `2>&1`, a child whose stdout is inherited gets the stdout of the process on stderr too. `Dup2` there made a pipe that nothing read. --- src/runtime/shell/IO.rs | 1 + src/runtime/shell/states/Cmd.rs | 1 - test/js/bun/shell/bunshell.test.ts | 23 ++++++++++++++++------- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/src/runtime/shell/IO.rs b/src/runtime/shell/IO.rs index 1e9de0dba410..1ead932f541e 100644 --- a/src/runtime/shell/IO.rs +++ b/src/runtime/shell/IO.rs @@ -210,6 +210,7 @@ impl CmdIO { match &stdio[1] { // Windows: `Dup2` is a pipe that only a captured stdout reads. Stdio::Fd(fd) if cfg!(windows) => Stdio::Fd(*fd), + Stdio::Inherit if cfg!(windows) => Stdio::Fd(Fd::stdout()), _ => Stdio::Dup2(Dup2 { out: StdioKind::Stderr, to: StdioKind::Stdout, diff --git a/src/runtime/shell/states/Cmd.rs b/src/runtime/shell/states/Cmd.rs index 750d616eff1c..4a238c3b4a91 100644 --- a/src/runtime/shell/states/Cmd.rs +++ b/src/runtime/shell/states/Cmd.rs @@ -979,7 +979,6 @@ impl Cmd { JsObject::Blob(blob) => blob_file(blob), JsObject::Response(response) => { let body = response.get_body_value(); - body.to_blob_if_possible(); // Take only a body that is a file: a rejected target keeps its body. let file = match &*body { crate::webcore::body::Value::Blob(blob) => blob_file(blob), diff --git a/test/js/bun/shell/bunshell.test.ts b/test/js/bun/shell/bunshell.test.ts index d46b4f9489aa..2f72897a292c 100644 --- a/test/js/bun/shell/bunshell.test.ts +++ b/test/js/bun/shell/bunshell.test.ts @@ -2223,14 +2223,23 @@ describe("deno_task", () => { test("a Response that is rejected keeps its body", async () => { const results = []; for (const name of ["echo", BUN, "nosuch_cmd_zz"]) { - const response = new Response("keep"); - const rejected = await $`${name} --version > ${response}`.quiet().then( - () => false, - () => true, - ); - results.push({ rejected, bodyUsed: response.bodyUsed, body: await response.text() }); + for (const body of ["keep", new Blob(["keep"]).stream()]) { + const response = new Response(body); + const rejected = await $`${name} --version > ${response}`.quiet().then( + () => false, + () => true, + ); + results.push({ + rejected, + bodyUsed: response.bodyUsed, + streamLocked: typeof body !== "string" && body.locked, + text: await response.text(), + }); + } } - expect(results).toEqual(Array(3).fill({ rejected: true, bodyUsed: false, body: "keep" })); + expect(results).toEqual( + Array(6).fill({ rejected: true, bodyUsed: false, streamLocked: false, text: "keep" }), + ); }); }); });