reset signal handlers in Bun.spawn (#4405)

* see if this fixes it

* We don't need this

* Remove extra flag

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
This commit is contained in:
Jarred Sumner
2023-08-30 00:16:08 -07:00
committed by GitHub
parent f2553d2454
commit e3dc5b6b4c
7 changed files with 66 additions and 32 deletions

19
.vscode/launch.json generated vendored
View File

@@ -4,8 +4,6 @@
// it makes our tests very slow
// But it helps catch memory bugs
// SIGHUP must be ignored or the debugger will pause when a spawned subprocess exits:
// { "initCommands": ["process handle -p false -s false -n false SIGHUP"] }
"version": "0.2.0",
"configurations": [
{
@@ -21,7 +19,6 @@
"BUN_DEBUG_QUIET_LOGS": "1",
"BUN_GARBAGE_COLLECTOR_LEVEL": "2"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -36,7 +33,6 @@
"FORCE_COLOR": "1",
"BUN_DEBUG_QUIET_LOGS": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
@@ -51,7 +47,6 @@
"env": {
"FORCE_COLOR": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -66,7 +61,6 @@
"FORCE_COLOR": "1",
"BUN_DEBUG_QUIET_LOGS": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -81,7 +75,6 @@
"FORCE_COLOR": "1",
"BUN_DEBUG_QUIET_LOGS": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -96,7 +89,6 @@
"BUN_DEBUG_QUIET_LOGS": "1",
"BUN_GARBAGE_COLLECTOR_LEVEL": "2"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -111,7 +103,6 @@
"FORCE_COLOR": "1",
"BUN_DEBUG_QUIET_LOGS": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -126,7 +117,6 @@
"FORCE_COLOR": "1",
"BUN_DEBUG_QUIET_LOGS": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -140,7 +130,6 @@
"FORCE_COLOR": "1",
"NODE_ENV": "development"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -155,7 +144,6 @@
"BUN_DEBUG_QUIET_LOGS": "1",
"BUN_GARBAGE_COLLECTOR_LEVEL": "2"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -168,7 +156,6 @@
"env": {
"FORCE_COLOR": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -181,7 +168,6 @@
"env": {
"FORCE_COLOR": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -194,7 +180,6 @@
"env": {
"FORCE_COLOR": "1"
},
"initCommands": ["process handle -p false -s false -n false SIGHUP"],
"console": "internalConsole"
},
{
@@ -216,9 +201,7 @@
"console": "internalConsole",
"env": {
"BUN_CONFIG_MINIFY_WHITESPACE": "1"
},
// SIGHUP must be ignored or the debugger will pause when a spawned subprocess exits.
"initCommands": ["process handle -p false -s false -n false SIGHUP"]
}
},
{
"type": "lldb",

View File

@@ -75,6 +75,14 @@ pub const PosixSpawn = struct {
else => |err| return unexpectedErrno(err),
}
}
pub fn resetSignals(this: *Attr) !void {
if (posix_spawnattr_reset_signals(&this.attr) != 0) {
return error.SystemResources;
}
}
extern fn posix_spawnattr_reset_signals(attr: *system.posix_spawnattr_t) c_int;
};
pub const Actions = struct {

View File

@@ -1070,6 +1070,7 @@ pub const Subprocess = struct {
var PATH = jsc_vm.bundler.env.get("PATH") orelse "";
var argv: std.ArrayListUnmanaged(?[*:0]const u8) = undefined;
var cmd_value = JSValue.zero;
var detached: bool = false;
var args = args_;
{
if (args.isEmptyOrUndefinedOrNull()) {
@@ -1243,6 +1244,12 @@ pub const Subprocess = struct {
}
}
}
if (args.get(globalThis, "detached")) |detached_val| {
if (detached_val.isBoolean()) {
detached = detached_val.toBoolean();
}
}
}
}
@@ -1251,17 +1258,29 @@ pub const Subprocess = struct {
return .zero;
};
var flags: i32 = bun.C.POSIX_SPAWN_SETSIGDEF | bun.C.POSIX_SPAWN_SETSIGMASK;
if (comptime Environment.isMac) {
flags |= bun.C.POSIX_SPAWN_CLOEXEC_DEFAULT;
}
if (detached) {
flags |= bun.C.POSIX_SPAWN_SETSID;
}
defer attr.deinit();
var actions = PosixSpawn.Actions.init() catch |err| return globalThis.handleError(err, "in posix_spawn");
if (comptime Environment.isMac) {
attr.set(
bun.C.POSIX_SPAWN_CLOEXEC_DEFAULT | bun.C.POSIX_SPAWN_SETSIGDEF | bun.C.POSIX_SPAWN_SETSIGMASK,
) catch |err| return globalThis.handleError(err, "in posix_spawn");
attr.set(@intCast(flags)) catch |err| return globalThis.handleError(err, "in posix_spawn");
} else if (comptime Environment.isLinux) {
attr.set(
bun.C.linux.POSIX_SPAWN.SETSIGDEF | bun.C.linux.POSIX_SPAWN.SETSIGMASK,
) catch |err| return globalThis.handleError(err, "in posix_spawn");
attr.set(@intCast(flags)) catch |err| return globalThis.handleError(err, "in posix_spawn");
}
attr.resetSignals() catch {
globalThis.throw("Failed to reset signals in posix_spawn", .{});
return .zero;
};
defer actions.deinit();
if (!override_env and env_array.items.len == 0) {
@@ -1344,14 +1363,14 @@ pub const Subprocess = struct {
const kernel = @import("../../../analytics.zig").GenerateHeader.GeneratePlatform.kernelVersion();
// pidfd_nonblock only supported in 5.10+
const flags: u32 = if (!is_sync and kernel.orderWithoutTag(.{ .major = 5, .minor = 10, .patch = 0 }).compare(.gte))
const pidfd_flags: u32 = if (!is_sync and kernel.orderWithoutTag(.{ .major = 5, .minor = 10, .patch = 0 }).compare(.gte))
std.os.O.NONBLOCK
else
0;
const fd = std.os.linux.pidfd_open(
pid,
flags,
pidfd_flags,
);
switch (std.os.linux.getErrno(fd)) {

View File

@@ -0,0 +1,18 @@
#include <spawn.h>
#include <signal.h>
extern "C" int posix_spawnattr_reset_signals(posix_spawnattr_t* attr)
{
sigset_t signal_set;
sigfillset(&signal_set);
if (posix_spawnattr_setsigdefault(attr, &signal_set) != 0) {
return 1;
}
sigemptyset(&signal_set);
if (posix_spawnattr_setsigmask(attr, &signal_set) != 0) {
return 1;
}
return 0;
}

View File

@@ -888,9 +888,13 @@ function normalizeSpawnArguments(file, args, options) {
cwd = getValidatedPath(cwd, "options.cwd");
}
// TODO: Detached check
// TODO: Gid check
// TODO: Uid check
var detached = false;
const { detached: detachedOption } = options;
if (detachedOption != null) {
detached = !!detachedOption;
}
// Validate the shell, if present.
if (options.shell != null && typeof options.shell !== "boolean" && typeof options.shell !== "string") {
@@ -945,7 +949,7 @@ function normalizeSpawnArguments(file, args, options) {
// TODO: Windows env support here...
return { ...options, file, args, cwd, envPairs };
return { ...options, detached, file, args, cwd, envPairs };
}
function checkExecSyncError(ret, args, cmd) {
@@ -1155,7 +1159,7 @@ class ChildProcess extends EventEmitter {
const bunStdio = getBunStdioFromOptions(stdio);
var env = options.envPairs || undefined;
const detachedOption = options.detached;
this.#encoding = options.encoding || undefined;
this.#stdioOptions = bunStdio;
this.#handle = Bun.spawn({
@@ -1165,6 +1169,7 @@ class ChildProcess extends EventEmitter {
stderr: bunStdio[2],
cwd: options.cwd || undefined,
env: env || process.env,
detached: typeof detachedOption !== "undefined" ? !!detachedOption : false,
onExit: (handle, exitCode, signalCode, err) => {
this.#handle = handle;
this.pid = this.#handle.pid;

File diff suppressed because one or more lines are too long

View File

@@ -541,6 +541,7 @@ pub const POSIX_SPAWN_SETSIGDEF = @as(c_int, 0x04);
pub const POSIX_SPAWN_SETSIGMASK = @as(c_int, 0x08);
pub const POSIX_SPAWN_SETSCHEDPARAM = @as(c_int, 0x10);
pub const POSIX_SPAWN_SETSCHEDULER = @as(c_int, 0x20);
pub const POSIX_SPAWN_SETSID = @as(c_int, 0x80);
const posix_spawn_file_actions_addfchdir_np_type = *const fn (actions: *posix_spawn_file_actions_t, filedes: fd_t) c_int;
const posix_spawn_file_actions_addchdir_np_type = *const fn (actions: *posix_spawn_file_actions_t, path: [*:0]const u8) c_int;