WIP fix for #14982

packages/bun-usockets/src/eventing/epoll_kqueue.c

@@ -247,7 +247,7 @@ void us_loop_run(struct us_loop_t *loop) {
     }
 }
 
-extern int Bun__JSC_onBeforeWait(void*);
+extern int Bun__JSC_onBeforeWaitWithTimer(void* vm, int hasTimer);
 extern void Bun__JSC_onAfterWait(void*);
 
 void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout) {
@@ -267,7 +267,7 @@ void us_loop_run_bun_tick(struct us_loop_t *loop, const struct timespec* timeout
 
     int needs_after_wait = 0;
     if (loop->data.jsc_vm) {
-        needs_after_wait = Bun__JSC_onBeforeWait(loop->data.jsc_vm);
+        needs_after_wait = Bun__JSC_onBeforeWaitWithTimer(loop->data.jsc_vm, 1);
     }
 
     /* Fetch ready polls */

src/bun.js/bindings/BunJSCEventLoop.cpp

@@ -2,22 +2,30 @@
 
 #include <JavaScriptCore/VM.h>
 #include <JavaScriptCore/Heap.h>
+#include <JavaScriptCore/IncrementalSweeper.h>
 
-extern "C" int Bun__JSC_onBeforeWait(JSC::VM* vm)
+extern "C" int Bun__JSC_onBeforeWaitWithTimer(JSC::VM* vm, int hasTimer)
 {
-    UNUSED_PARAM(vm);
-    // TODO: use JSC timers, run the incremental sweeper.
-    // That will fix this.
-    // In the meantime, we're disabling this due to https://github.com/oven-sh/bun/issues/14982
-    // if (vm->heap.hasAccess()) {
-    //     vm->heap.releaseAccess();
-    //     return 1;
-    // }
+    if (vm->heap.hasAccess()) {
+        if (hasTimer) {
+            // TODO: this should pass the deadline based on the timeout passed to the event loop code.
+            // So if you have a setTimeout() call thats happening in 2ms, we should never delay 2ms.
+            // Also, all of the JSRunLoopTimer code should be using our TimerHeap from Zig.
+            vm->heap.sweeper().doWork(*vm);
+        }
+
+        vm->heap.releaseAccess();
+        return 1;
+    }
     return 0;
 }
 
+extern "C" int Bun__JSC_onBeforeWait(JSC::VM* vm)
+{
+    return Bun__JSC_onBeforeWaitWithTimer(vm, 0);
+}
+
 extern "C" void Bun__JSC_onAfterWait(JSC::VM* vm)
 {
-    UNUSED_PARAM(vm);
-    // vm->heap.acquireAccess();
+    vm->heap.acquireAccess();
 }

src/bun.js/bindings/ZigGlobalObject.cpp

@@ -871,6 +871,8 @@ extern "C" JSC__JSGlobalObject* Zig__GlobalObject__create(void* console_client,
 
     WebCore::JSVMClientData::create(&vm, Bun__getVM());
 
+    vm.heap.disableStopIfNecessaryTimer();
+
     const auto createGlobalObject = [&]() -> Zig::GlobalObject* {
         if (UNLIKELY(executionContextId > -1)) {
             auto* structure = Zig::GlobalObject::createStructure(vm);

src/bun.js/bindings/bindings.zig

@@ -6329,16 +6329,19 @@ pub const VM = extern struct {
     extern fn Bun__JSC_onAfterWait(vm: *VM) void;
     pub const ReleaseHeapAccess = struct {
         vm: *VM,
-        needs_to_release: bool,
+        needs_to_acquire: bool,
         pub fn acquire(this: *const ReleaseHeapAccess) void {
-            if (this.needs_to_release) {
+            if (this.needs_to_acquire) {
                 Bun__JSC_onAfterWait(this.vm);
             }
         }
     };
 
+    /// Temporarily give up access to the heap, allowing other work to proceed. Call acquire() on
+    /// the return value at scope exit. If you did not already have heap access, release and acquire
+    /// are both safe no-ops.
     pub fn releaseHeapAccess(vm: *VM) ReleaseHeapAccess {
-        return .{ .vm = vm, .needs_to_release = Bun__JSC_onBeforeWait(vm) != 0 };
+        return .{ .vm = vm, .needs_to_acquire = Bun__JSC_onBeforeWait(vm) != 0 };
     }
 
     pub fn create(heap_type: HeapType) *VM {

src/bun.js/javascript.zig

@@ -743,7 +743,7 @@ const AutoKiller = struct {
         while (this.processes.popOrNull()) |process| {
             if (!process.key.hasExited()) {
                 log("process.kill {d}", .{process.key.pid});
-                count += @as(u32, @intFromBool(process.key.kill(bun.SignalCode.default) == .result));
+                count += @as(u32, @intFromBool(process.key.kill(@intFromEnum(bun.SignalCode.SIGKILL)) == .result));
             }
         }
         return count;

src/bun.js/module_loader.zig

@@ -1649,7 +1649,7 @@ pub const ModuleLoader = struct {
                         const heap_access = if (!disable_transpilying)
                             jsc_vm.jsc.releaseHeapAccess()
                         else
-                            JSC.VM.ReleaseHeapAccess{ .vm = jsc_vm.jsc, .needs_to_release = false };
+                            JSC.VM.ReleaseHeapAccess{ .vm = jsc_vm.jsc, .needs_to_acquire = false };
                         defer heap_access.acquire();
 
                         break :brk jsc_vm.bundler.parseMaybeReturnFileOnly(

test/regression/issue/14982/14982.test.ts

@@ -0,0 +1,85 @@
+import { expect, test, it, describe } from "bun:test";
+import { bunEnv, bunExe } from "../../../harness";
+import { join } from "path";
+
+describe("issue 14982", () => {
+  it("does not hang in commander", async () => {
+    const process = Bun.spawn([bunExe(), join(__dirname, "commander-hang.fixture.ts"), "test"], {
+      stdin: "inherit",
+      stdout: "pipe",
+      stderr: "inherit",
+      env: bunEnv,
+    });
+    await process.exited;
+    expect(process.exitCode).toBe(0);
+    expect(await new Response(process.stdout).text()).toBe("Test command\n");
+  }, 1000);
+
+  it("does not crash when sent SIGUSR1 by a child", async () => {
+    const process = Bun.spawn([bunExe(), join(__dirname, "raise.js")], {
+      stdin: "inherit",
+      stdout: "pipe",
+      stderr: "inherit",
+      env: bunEnv,
+    });
+    await process.exited;
+    expect(process.exitCode).toBe(0);
+    expect(await new Response(process.stdout).text()).toBe("exited with 0\n");
+  });
+
+  it("does not slow down express", async () => {
+    // requests per second with bun 1.1.34 on @190n's work laptop
+    const baseline = 95939;
+
+    const server = Bun.spawn([bunExe(), join(__dirname, "express-server.js")], {
+      stdin: "inherit",
+      stdout: "inherit",
+      stderr: "inherit",
+    });
+
+    // let it start listening
+    await Bun.sleep(100);
+
+    const oha = Bun.spawn(["oha", "http://localhost:3000", "-j", "-n", "1000000", "-c", "40"], {
+      stdin: "inherit",
+      stdout: "pipe",
+      stderr: "inherit",
+    });
+
+    const results = await new Response(oha.stdout).json();
+    const rps = results.summary.requestsPerSec;
+    const slowdownPercent = ((baseline - rps) / baseline) * 100;
+    expect(slowdownPercent).toBeLessThan(5);
+
+    server.kill();
+  }, 15000);
+
+  it("does not slow down fastify", async () => {
+    // requests per second with bun 1.1.34 on @190n's work laptop
+    const baseline = 161178;
+
+    const server = Bun.spawn([bunExe(), join(__dirname, "fastify-server.mjs")], {
+      stdin: "inherit",
+      stdout: "inherit",
+      stderr: "inherit",
+    });
+
+    // let it start listening
+    await Bun.sleep(100);
+
+    const oha = Bun.spawn(["oha", "http://localhost:3000", "-j", "-n", "1000000", "-c", "40"], {
+      stdin: "inherit",
+      stdout: "pipe",
+      stderr: "inherit",
+    });
+
+    const results = await new Response(oha.stdout).json();
+    const rps = results.summary.requestsPerSec;
+    const slowdownPercent = ((baseline - rps) / baseline) * 100;
+    expect(slowdownPercent).toBeLessThan(5);
+
+    server.kill();
+  }, 15000);
+
+  // decreases AbortSignal spam memory usage
+});

test/regression/issue/14982/README.md

@@ -0,0 +1,15 @@
+# 14982
+
+To install dependencies:
+
+```bash
+bun install
+```
+
+To run:
+
+```bash
+bun run index.ts
+```
+
+This project was created using `bun init` in bun v1.1.34. [Bun](https://bun.sh) is a fast all-in-one JavaScript runtime.

test/regression/issue/14982/commander-hang.fixture.ts

@@ -0,0 +1,3 @@
+import { program } from "commander";
+
+program.name("test").command("test", "Test command").parse();

test/regression/issue/14982/express-server.js

@@ -0,0 +1,20 @@
+const express = require("express");
+const app = express();
+
+let count = 0;
+
+app.get("/", (req, res) => {
+  count++;
+  if (count >= 1_000_000) {
+    setTimeout(() => process.exit(0), 1);
+  }
+
+  if (count % 50_000 === 0) {
+    console.log("RSS", (process.memoryUsage.rss() / 1024 / 1024) | 0, "MB");
+  }
+  res.send("Hello World!");
+});
+
+app.listen(3000, () => {
+  console.log("Example app listening on port 3000");
+});

test/regression/issue/14982/fastify-server.mjs

@@ -0,0 +1,20 @@
+import Fastify from "fastify";
+
+let startOfLastRequest = performance.now();
+const fastify = Fastify({});
+
+// Declare a route
+fastify.get("/", (request, reply) => {
+  const now = performance.now();
+  // if (startOfLastRequest && now - startOfLastRequest > 0.5) {
+  // 	console.log("Elapsed", Math.trunc(now - startOfLastRequest), "ms");
+  // }
+  // startOfLastRequest = now;
+  reply.send({ hello: "world" });
+});
+
+// Run the server!
+fastify.listen({ port: 3000 }, (err, address) => {
+  if (err) throw err;
+  // Server is now listening on ${address}
+});

test/regression/issue/14982/package.json

@@ -0,0 +1,16 @@
+{
+  "name": "14982",
+  "module": "commander-hang.fixture.ts",
+  "type": "module",
+  "devDependencies": {
+    "@types/bun": "latest"
+  },
+  "peerDependencies": {
+    "typescript": "^5.0.0"
+  },
+  "dependencies": {
+    "commander": "^12.1.0",
+    "express": "^4.21.1",
+    "fastify": "^5.1.0"
+  }
+}

test/regression/issue/14982/raise.js

@@ -0,0 +1,7 @@
+import { spawn } from "node:child_process";
+import { join } from "path";
+
+const child = spawn(join(import.meta.dirname, "./raiser"), [], { stdio: "inherit" });
+child.on("close", code => {
+  console.log(`exited with ${code}`);
+});

test/regression/issue/14982/raiser.c

@@ -0,0 +1,8 @@
+#include <signal.h>
+#include <unistd.h>
+
+int main(void) {
+  usleep(250000);
+  kill(getppid(), SIGUSR1);
+  return 0;
+}

test/regression/issue/14982/test-test.ts

@@ -0,0 +1,8 @@
+import { Command } from "commander";
+
+new Command("test")
+  .action(() => {
+    console.log("Test command");
+    process.exit(0);
+  })
+  .parse();

test/regression/issue/14982/tsconfig.json

@@ -0,0 +1,27 @@
+{
+  "compilerOptions": {
+    // Enable latest features
+    "lib": ["ESNext", "DOM"],
+    "target": "ESNext",
+    "module": "ESNext",
+    "moduleDetection": "force",
+    "jsx": "react-jsx",
+    "allowJs": true,
+
+    // Bundler mode
+    "moduleResolution": "bundler",
+    "allowImportingTsExtensions": true,
+    "verbatimModuleSyntax": true,
+    "noEmit": true,
+
+    // Best practices
+    "strict": true,
+    "skipLibCheck": true,
+    "noFallthroughCasesInSwitch": true,
+
+    // Some stricter flags (disabled by default)
+    "noUnusedLocals": false,
+    "noUnusedParameters": false,
+    "noPropertyAccessFromIndexSignature": false
+  }
+}