mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
Fix Windows shell crash with && operator and external commands (#22651)
## What does this PR do? Fixes https://github.com/oven-sh/bun/issues/22650 Fixes https://github.com/oven-sh/bun/issues/22615 Fixes https://github.com/oven-sh/bun/issues/22603 Fixes https://github.com/oven-sh/bun/issues/22602 Fixes a crash that occurred when running shell commands through `bun run` (package.json scripts) on Windows that use the `&&` operator followed by an external command. ### The Problem The minimal reproduction was: ```bash bun exec 'echo && node --version' ``` This would crash with: `panic(main thread): attempt to use null value` ### Root Causes Two issues were causing the crash: 1. **Missing top_level_dir**: When `runPackageScriptForeground` creates a MiniEventLoop for running package scripts, it wasn't setting the `top_level_dir` field. This caused a null pointer dereference when the shell tried to access it. 2. **MovableIfWindowsFd handling**: After PR #21800 introduced `MovableIfWindowsFd` to handle file descriptor ownership on Windows, the `IOWriter.fd` could be moved to libuv, leaving it null. When the shell tried to spawn an external command after a `&&` operator, it would crash trying to access this null fd. ### The Fix 1. Set `mini.top_level_dir = cwd` after initializing the MiniEventLoop in `run_command.zig` 2. In `IO.zig`, when the fd has been moved to libuv (is null), use `.inherit` for stdio instead of trying to pass the null fd ### How did you verify your code works? - Added a regression test that reproduces the issue - Verified the test fails without the fix and passes with it - Tested the minimal reproduction command directly - The fix correctly allows both commands in the `&&` chain to execute ```bash # Before fix: crashes > bun exec 'echo test && node --version' panic(main thread): attempt to use null value # After fix: works correctly > bun exec 'echo test && node --version' test v22.4.1 ``` <sub> also probably fixes #22615 and fixes #22603 and fixes #22602 </sub> --------- Co-authored-by: Zack Radisic <zack@theradisic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
This commit is contained in:
@@ -135,7 +135,7 @@ pub const Run = struct {
|
||||
null,
|
||||
);
|
||||
try bundle.runEnvLoader(false);
|
||||
const mini = jsc.MiniEventLoop.initGlobal(bundle.env);
|
||||
const mini = jsc.MiniEventLoop.initGlobal(bundle.env, null);
|
||||
mini.top_level_dir = ctx.args.absolute_working_dir orelse "";
|
||||
return bun.shell.Interpreter.initAndRunFromFile(ctx, mini, entry_path);
|
||||
}
|
||||
|
||||
@@ -40,7 +40,7 @@ pub threadlocal var global: *MiniEventLoop = undefined;
|
||||
|
||||
pub const ConcurrentTaskQueue = UnboundedQueue(AnyTaskWithExtraContext, .next);
|
||||
|
||||
pub fn initGlobal(env: ?*bun.DotEnv.Loader) *MiniEventLoop {
|
||||
pub fn initGlobal(env: ?*bun.DotEnv.Loader, cwd: ?[]const u8) *MiniEventLoop {
|
||||
if (globalInitialized) return global;
|
||||
const loop = MiniEventLoop.init(bun.default_allocator);
|
||||
global = bun.handleOom(bun.default_allocator.create(MiniEventLoop));
|
||||
@@ -54,6 +54,22 @@ pub fn initGlobal(env: ?*bun.DotEnv.Loader) *MiniEventLoop {
|
||||
loader.* = bun.DotEnv.Loader.init(map, bun.default_allocator);
|
||||
break :env_loader loader;
|
||||
};
|
||||
|
||||
// Set top_level_dir from provided cwd or get current working directory
|
||||
if (cwd) |dir| {
|
||||
global.top_level_dir = dir;
|
||||
} else if (global.top_level_dir.len == 0) {
|
||||
var buf: bun.PathBuffer = undefined;
|
||||
switch (bun.sys.getcwd(&buf)) {
|
||||
.result => |p| {
|
||||
global.top_level_dir = bun.default_allocator.dupe(u8, p) catch "";
|
||||
},
|
||||
.err => {
|
||||
global.top_level_dir = "";
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
globalInitialized = true;
|
||||
return global;
|
||||
}
|
||||
|
||||
@@ -756,7 +756,7 @@ pub const BunxCommand = struct {
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(this_transpiler.env)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(this_transpiler.env, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.prettyErrorln("<r><red>error<r>: bunx failed to install <b>{s}<r> due to error <b>{s}<r>", .{ install_param, @errorName(err) });
|
||||
|
||||
@@ -109,7 +109,7 @@ fn execTask(allocator: std.mem.Allocator, task_: string, cwd: string, _: string,
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch return;
|
||||
}
|
||||
@@ -1487,7 +1487,7 @@ pub const CreateCommand = struct {
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
});
|
||||
_ = try process.unwrap();
|
||||
|
||||
@@ -9,9 +9,7 @@ pub const ExecCommand = struct {
|
||||
null,
|
||||
);
|
||||
try bundle.runEnvLoader(false);
|
||||
const mini = bun.jsc.MiniEventLoop.initGlobal(bundle.env);
|
||||
var buf: bun.PathBuffer = undefined;
|
||||
|
||||
const cwd = switch (bun.sys.getcwd(&buf)) {
|
||||
.result => |p| p,
|
||||
.err => |e| {
|
||||
@@ -19,6 +17,7 @@ pub const ExecCommand = struct {
|
||||
Global.exit(1);
|
||||
},
|
||||
};
|
||||
const mini = bun.jsc.MiniEventLoop.initGlobal(bundle.env, cwd);
|
||||
const parts: []const []const u8 = &[_][]const u8{
|
||||
cwd,
|
||||
"[eval]",
|
||||
|
||||
@@ -532,7 +532,7 @@ pub fn runScriptsWithFilter(ctx: Command.Context) !noreturn {
|
||||
Global.exit(1);
|
||||
}
|
||||
|
||||
const event_loop = bun.jsc.MiniEventLoop.initGlobal(this_transpiler.env);
|
||||
const event_loop = bun.jsc.MiniEventLoop.initGlobal(this_transpiler.env, null);
|
||||
const shell_bin: [:0]const u8 = if (Environment.isPosix)
|
||||
RunCommand.findShell(this_transpiler.env.get("PATH") orelse "", fsinstance.top_level_dir) orelse return error.MissingShell
|
||||
else
|
||||
|
||||
@@ -461,7 +461,7 @@ pub const PmVersionCommand = struct {
|
||||
.cwd = cwd,
|
||||
.envp = null,
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.errGeneric("Failed to spawn git process: {s}", .{@errorName(err)});
|
||||
@@ -494,7 +494,7 @@ pub const PmVersionCommand = struct {
|
||||
.cwd = cwd,
|
||||
.envp = null,
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.err(err, "Failed to spawn git process", .{});
|
||||
@@ -541,7 +541,7 @@ pub const PmVersionCommand = struct {
|
||||
.stdin = .ignore,
|
||||
.envp = null,
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.errGeneric("Git add failed: {s}", .{@errorName(err)});
|
||||
@@ -575,7 +575,7 @@ pub const PmVersionCommand = struct {
|
||||
.stdin = .ignore,
|
||||
.envp = null,
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.errGeneric("Git commit failed: {s}", .{@errorName(err)});
|
||||
@@ -606,7 +606,7 @@ pub const PmVersionCommand = struct {
|
||||
.stdin = .ignore,
|
||||
.envp = null,
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.errGeneric("Git tag failed: {s}", .{@errorName(err)});
|
||||
|
||||
@@ -246,7 +246,7 @@ pub const RunCommand = struct {
|
||||
}
|
||||
|
||||
if (!use_system_shell) {
|
||||
const mini = bun.jsc.MiniEventLoop.initGlobal(env);
|
||||
const mini = bun.jsc.MiniEventLoop.initGlobal(env, cwd);
|
||||
const code = bun.shell.Interpreter.initAndRunFromSource(ctx, mini, name, copy_script.items, cwd) catch |err| {
|
||||
if (!silent) {
|
||||
Output.prettyErrorln("<r><red>error<r>: Failed to run script <b>{s}<r> due to error <b>{s}<r>", .{ name, @errorName(err) });
|
||||
@@ -294,7 +294,7 @@ pub const RunCommand = struct {
|
||||
.ipc = ipc_fd,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = jsc.EventLoopHandle.init(jsc.MiniEventLoop.initGlobal(env)),
|
||||
.loop = jsc.EventLoopHandle.init(jsc.MiniEventLoop.initGlobal(env, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
if (!silent) {
|
||||
@@ -467,7 +467,7 @@ pub const RunCommand = struct {
|
||||
.use_execve_on_macos = silent,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = jsc.EventLoopHandle.init(jsc.MiniEventLoop.initGlobal(env)),
|
||||
.loop = jsc.EventLoopHandle.init(jsc.MiniEventLoop.initGlobal(env, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
bun.handleErrorReturnTrace(err, @errorReturnTrace());
|
||||
|
||||
@@ -601,7 +601,7 @@ pub const UpgradeCommand = struct {
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.prettyErrorln("<r><red>error:<r> Failed to spawn Expand-Archive on {s} due to error {s}", .{ tmpname, @errorName(err) });
|
||||
|
||||
@@ -248,7 +248,7 @@ pub fn generateFiles(allocator: std.mem.Allocator, entry_point: string, dependen
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.err(err, "failed to install dependencies", .{});
|
||||
@@ -361,7 +361,7 @@ pub fn generateFiles(allocator: std.mem.Allocator, entry_point: string, dependen
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch |err| {
|
||||
Output.err(err, "failed to start app", .{});
|
||||
|
||||
@@ -198,7 +198,7 @@ pub fn onStart(opts: InitOpts) void {
|
||||
bun.http.default_arena = Arena.init();
|
||||
bun.http.default_allocator = bun.http.default_arena.allocator();
|
||||
|
||||
const loop = bun.jsc.MiniEventLoop.initGlobal(null);
|
||||
const loop = bun.jsc.MiniEventLoop.initGlobal(null, null);
|
||||
|
||||
if (Environment.isWindows) {
|
||||
_ = std.process.getenvW(comptime bun.strings.w("SystemRoot")) orelse {
|
||||
|
||||
@@ -25,7 +25,7 @@ pub fn openURL(url: stringZ) void {
|
||||
.stdin = .inherit,
|
||||
|
||||
.windows = if (Environment.isWindows) .{
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null)),
|
||||
.loop = bun.jsc.EventLoopHandle.init(bun.jsc.MiniEventLoop.initGlobal(null, null)),
|
||||
},
|
||||
}) catch break :maybe_fallback) {
|
||||
// don't fallback:
|
||||
|
||||
@@ -142,16 +142,14 @@ pub const OutKind = union(enum) {
|
||||
.capture = .{
|
||||
.buf = cap,
|
||||
},
|
||||
} else if (val.writer.fd.get()) |fd| .{
|
||||
// We have a valid fd that hasn't been moved to libuv
|
||||
.fd = fd,
|
||||
} else .{
|
||||
// Windows notes:
|
||||
// Since `val.writer.fd` is `MovableFD`, it could
|
||||
// technically be moved to libuv for ownership.
|
||||
//
|
||||
// But since this file descriptor never going to be touched by this
|
||||
// process, except to hand off to the subprocess when we
|
||||
// spawn it, we don't really care if the file descriptor
|
||||
// ends up being invalid.
|
||||
.fd = val.writer.fd.get().?,
|
||||
// On Windows, the fd might have been moved to libuv
|
||||
// In this case, the subprocess should inherit the stdio
|
||||
// since libuv is already managing it
|
||||
.inherit = {},
|
||||
};
|
||||
},
|
||||
.pipe => .pipe,
|
||||
|
||||
24
test/regression/issue/22650-shell-crash.test.ts
Normal file
24
test/regression/issue/22650-shell-crash.test.ts
Normal file
@@ -0,0 +1,24 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { bunEnv, bunExe } from "harness";
|
||||
|
||||
test("issue #22650 - shell crash with && operator followed by external command", async () => {
|
||||
// Minimal reproduction: echo && <external command>
|
||||
// This triggers the crash because after the first command succeeds,
|
||||
// the shell tries to spawn an external process but top_level_dir is not set
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "exec", "echo test && node --version"],
|
||||
env: bunEnv,
|
||||
stderr: "pipe",
|
||||
stdout: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
// Should not have any errors
|
||||
expect(stderr).toBe("");
|
||||
|
||||
// Should execute both commands successfully
|
||||
expect(stdout).toContain("test");
|
||||
expect(stdout).toMatch(/v\d+\.\d+\.\d+/); // Node version pattern
|
||||
expect(exitCode).toBe(0);
|
||||
});
|
||||
Reference in New Issue
Block a user