Skip to content

Commit

Permalink
Fix CTRL + C behavior in bun run so it doesn't ^[[A (#13762)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarred-Sumner authored Sep 6, 2024
1 parent ea12db4 commit debaa2c
Show file tree
Hide file tree
Showing 4 changed files with 194 additions and 9 deletions.
39 changes: 37 additions & 2 deletions src/bun.js/api/bun/process.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2045,21 +2045,54 @@ pub const sync = struct {
return spawnWithArgv(options, @ptrCast(args.items.ptr), @ptrCast(envp));
}

// Forward signals from parent to the child process.
extern "C" fn Bun__registerSignalsForForwarding() void;
extern "C" fn Bun__unregisterSignalsForForwarding() void;

// The PID to forward signals to.
// Set to 0 when unregistering.
extern "C" var Bun__currentSyncPID: i64;

// Race condition: a signal could be sent before spawnProcessPosix returns.
// We need to make sure to send it after the process is spawned.
extern "C" fn Bun__sendPendingSignalIfNecessary() void;

fn spawnPosix(
options: *const Options,
argv: [*:null]?[*:0]const u8,
envp: [*:null]?[*:0]const u8,
) !Maybe(Result) {
Bun__currentSyncPID = 0;
Bun__registerSignalsForForwarding();
defer {
Bun__unregisterSignalsForForwarding();
bun.crash_handler.resetOnPosix();
}
const process = switch (try spawnProcessPosix(&options.toSpawnOptions(), argv, envp)) {
.err => |err| return .{ .err = err },
.result => |proces| proces,
};
Bun__currentSyncPID = @intCast(process.pid);

Bun__sendPendingSignalIfNecessary();

var out = [2]std.ArrayList(u8){
std.ArrayList(u8).init(bun.default_allocator),
std.ArrayList(u8).init(bun.default_allocator),
};
var out_fds = [2]bun.FileDescriptor{ process.stdout orelse bun.invalid_fd, process.stderr orelse bun.invalid_fd };
var success = false;
defer {
// If we're going to return an error,
// let's make sure to clean up the output buffers
// and kill the process
if (!success) {
for (&out) |*array_list| {
array_list.clearAndFree();
}
_ = std.c.kill(process.pid, 1);
}

for (out_fds) |fd| {
if (fd != bun.invalid_fd) {
_ = bun.sys.close(fd);
Expand Down Expand Up @@ -2090,13 +2123,14 @@ pub const sync = struct {
for (&out_fds_to_wait_for, &out, &out_fds) |*fd, *bytes, *out_fd| {
if (fd.* == bun.invalid_fd) continue;
while (true) {
bytes.ensureUnusedCapacity(16384) catch bun.outOfMemory();
bytes.ensureUnusedCapacity(16384) catch {
return .{ .err = bun.sys.Error.fromCode(.NOMEM, .recv) };
};
switch (bun.sys.recvNonBlock(fd.*, bytes.unusedCapacitySlice())) {
.err => |err| {
if (err.isRetry() or err.getErrno() == .PIPE) {
break;
}
_ = std.c.kill(process.pid, 1);
return .{ .err = err };
},
.result => |bytes_read| {
Expand Down Expand Up @@ -2165,6 +2199,7 @@ pub const sync = struct {
}
}

success = true;
return .{
.result = Result{
.status = status,
Expand Down
113 changes: 112 additions & 1 deletion src/bun.js/bindings/c-bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -623,4 +623,115 @@ extern "C" void Bun__disableSOLinger(SOCKET fd)
setsockopt(fd, SOL_SOCKET, SO_LINGER, (char*)&l, sizeof(l));
}

#endif
#endif

// Handle signals in bun.spawnSync.
// If we receive a signal, we want to forward the signal to the child process.
#if OS(LINUX) || OS(DARWIN)
#include <signal.h>
#include <pthread.h>

// Note: We only ever use bun.spawnSync on the main thread.
extern "C" int64_t Bun__currentSyncPID = 0;
static int Bun__pendingSignalToSend = 0;
static struct sigaction previous_actions[NSIG];

// This list of signals is copied from npm.
// https://github.com/npm/cli/blob/fefd509992a05c2dfddbe7bc46931c42f1da69d7/workspaces/arborist/lib/signals.js#L26-L57
#define FOR_EACH_POSIX_SIGNAL(M) \
M(SIGABRT); \
M(SIGALRM); \
M(SIGHUP); \
M(SIGINT); \
M(SIGTERM); \
M(SIGVTALRM); \
M(SIGXCPU); \
M(SIGXFSZ); \
M(SIGUSR2); \
M(SIGTRAP); \
M(SIGSYS); \
M(SIGQUIT); \
M(SIGIOT); \
M(SIGIO);

#if OS(LINUX)
#define FOR_EACH_LINUX_ONLY_SIGNAL(M) \
M(SIGPOLL); \
M(SIGPWR); \
M(SIGSTKFLT);

#endif

#if OS(DARWIN)
#define FOR_EACH_SIGNAL(M) FOR_EACH_POSIX_SIGNAL(M)
#endif

#if OS(LINUX)
#define FOR_EACH_SIGNAL(M) \
FOR_EACH_POSIX_SIGNAL(M) \
FOR_EACH_LINUX_ONLY_SIGNAL(M)
#endif

static void Bun__forwardSignalFromParentToChildAndRestorePreviousAction(pid_t pid, int sig)
{
sigset_t restore_mask;
sigset_t mask;
sigemptyset(&mask);
sigaddset(&mask, sig);
sigemptyset(&restore_mask);
sigaddset(&restore_mask, sig);
pthread_sigmask(SIG_BLOCK, &mask, &restore_mask);
kill(pid, sig);
pthread_sigmask(SIG_UNBLOCK, &restore_mask, nullptr);
}

extern "C" void Bun__sendPendingSignalIfNecessary()
{
int sig = Bun__pendingSignalToSend;
Bun__pendingSignalToSend = 0;
int pid = Bun__currentSyncPID;
if (sig == 0 || pid == 0)
return;

Bun__forwardSignalFromParentToChildAndRestorePreviousAction(pid, sig);
}

extern "C" void Bun__registerSignalsForForwarding()
{
Bun__pendingSignalToSend = 0;
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
sigemptyset(&sa.sa_mask);
sa.sa_flags = SA_RESETHAND;
sa.sa_handler = [](int sig) {
if (Bun__currentSyncPID == 0) {
Bun__pendingSignalToSend = sig;
return;
}

Bun__forwardSignalFromParentToChildAndRestorePreviousAction(Bun__currentSyncPID, sig);
};

#define REGISTER_SIGNAL(SIG) \
if (sigaction(SIG, &sa, &previous_actions[SIG]) == -1) { \
}

FOR_EACH_SIGNAL(REGISTER_SIGNAL)

#undef REGISTER_SIGNAL
}

extern "C" void Bun__unregisterSignalsForForwarding()
{
Bun__currentSyncPID = 0;

#define UNREGISTER_SIGNAL(SIG) \
if (sigaction(SIG, &previous_actions[SIG], NULL) == -1) { \
}

FOR_EACH_SIGNAL(UNREGISTER_SIGNAL)
memset(previous_actions, 0, sizeof(previous_actions));
#undef UNREGISTER_SIGNAL
}

#endif
16 changes: 10 additions & 6 deletions src/crash_handler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -763,19 +763,23 @@ pub fn updatePosixSegfaultHandler(act: ?*std.posix.Sigaction) !void {

var windows_segfault_handle: ?windows.HANDLE = null;

pub fn resetOnPosix() void {
var act = std.posix.Sigaction{
.handler = .{ .sigaction = handleSegfaultPosix },
.mask = std.posix.empty_sigset,
.flags = (std.posix.SA.SIGINFO | std.posix.SA.RESTART | std.posix.SA.RESETHAND),
};
updatePosixSegfaultHandler(&act) catch {};
}

pub fn init() void {
if (!enable) return;
switch (bun.Environment.os) {
.windows => {
windows_segfault_handle = windows.kernel32.AddVectoredExceptionHandler(0, handleSegfaultWindows);
},
.mac, .linux => {
var act = std.posix.Sigaction{
.handler = .{ .sigaction = handleSegfaultPosix },
.mask = std.posix.empty_sigset,
.flags = (std.posix.SA.SIGINFO | std.posix.SA.RESTART | std.posix.SA.RESETHAND),
};
updatePosixSegfaultHandler(&act) catch {};
resetOnPosix();
},
else => @compileError("TODO"),
}
Expand Down
35 changes: 35 additions & 0 deletions test/regression/issue/ctrl-c.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { test, expect, it } from "bun:test";
import { join } from "path";
import { bunEnv, bunExe, isWindows, tempDirWithFiles } from "harness";

test.skipIf(isWindows)("verify that we forward SIGINT from parent to child in bun run", () => {
const dir = tempDirWithFiles("ctrlc", {
"index.js": `
let count = 0;
process.exitCode = 1;
process.once("SIGINT", () => {
process.kill(process.pid, "SIGKILL");
});
setTimeout(() => {}, 999999)
process.kill(process.ppid, "SIGINT");
`,
"package.json": `
{
"name": "ctrlc",
"scripts": {
"start": "${bunExe()} index.js"
}
}
`,
});

const result = Bun.spawnSync({
cmd: [bunExe(), "start"],
cwd: dir,
env: bunEnv,
stdout: "inherit",
stderr: "inherit",
});
expect(result.exitCode).toBe(null);
expect(result.signalCode).toBe("SIGKILL");
});

0 comments on commit debaa2c

Please sign in to comment.