worker: don't re-enter JS from socket on_open / CJS require / process emit once terminate() has fired the trap

Merged
RO
3mo ago
+262-11

A worker's terminate() lands while the worker thread is inside a native dispatch, the first JS entry fires the termination trap and leaves the sticky TerminationException pending, and the second JS entry (or a downcast of the exception value) trips a debug-build assertion and aborts the whole process. Six sites of this class:

NewSocket::on_open / on_keylog / on_session (socket_body.rs)

on_open was the only NewSocket dispatch entry point without a shutdown guard before its JS calls, and unlike the others it enters JS twice: handlers.resolve_promise() (the Bun.connect() promise) and then callback.call() (the user's open handler). When terminate() lands between WebWorker::spin's has_requested_terminate() check and the socket dispatch inside auto_tick_active, the first call raises the TerminationException and the second trips Interpreter::executeCallImpl's scope.assertNoException():

ASSERTION FAILED: !exception()
ExceptionScope.h(61) : void JSC::ExceptionScope::assertNoException()
  <- Interpreter::executeCallImpl <- JSC::call <- Bun__JSValue__call
  <- NewSocket::on_open (callback.call of the JS `open` handler)
  <- us_internal_dispatch_ready_poll <- us_loop_run_bun_tick <- auto_tick_active
  <- WebWorker::spin

Reproduces from Bun.connect, net.connect, and tls.connect alike (they share on_open<SSL>). on_keylog and on_session (TLS-only, reached via node:tls/node:http2's keylog/session callbacks) have the same two-JS-entry shape behind the weaker is_shutting_down() gate.

Fix: gate on_open/on_keylog/on_session on script_execution_status() != Running (which covers a worker's has_requested_terminate(); is_shutting_down() is still false at this point), and in on_open bail out of the second JS entry if resolve_promise left an exception pending.

finishRequireWithError (JSCommonJSModule.cpp)

finishRequireWithError (and two sibling error-path cleanup sites) did tryClearException() then requireMap()->remove(). tryClearException() cannot clear a termination, so when terminate() landed mid-require() of a deep CJS graph, JSMap::remove ran with the exception still pending, bailed out, and ASSERT(wasRemoved) aborted:

ASSERTION FAILED: wasRemoved
JSCommonJSModule.cpp(1273) : EncodedJSValue Bun::finishRequireWithError(...)

Fix: skip the require-map cleanup when vm.hasPendingTerminationException() and let the termination propagate.

Bun__handleUncaughtException / Bun__handleUnhandledRejection / Bun__emitHandledPromiseEvent / Bun__promises__emitUnhandledRejectionWarning (BunProcess.cpp)

Bun__handleUncaughtException probes process._fatalException (a JS get(), where the trap fires) and then calls wrapped.emit("uncaughtException") with the termination still pending after tryClearException():

ASSERTION FAILED: !exception()
ExceptionScope.h(61) : void JSC::ExceptionScope::assertNoException()
  <- Interpreter::executeCallImpl <- JSC::call <- EventEmitter::fireEventListeners
  <- Bun__handleUncaughtException (wrapped.emit "uncaughtException")
  <- VirtualMachine::uncaught_exception <- report_unhandled_error
  <- Bun__JSTimeout__call <- drain_timers <- auto_tick_active <- WebWorker::spin

Bun__emitHandledPromiseEvent and Bun__promises__emitUnhandledRejectionWarning have the same CLEAR_IF_EXCEPTION → next-emit shape. The Rust callers only gate on is_shutting_down().

Fix: guard each emit entry point and each tryClearException() follow-up on vm.hasPendingTerminationException(), matching the existing NodeTimerObject.cpp pattern.

ErrorCodeCache::createError (ErrorCode.cpp)

The fallback branch did tryClearException() then uncheckedDowncast<JSObject>(thrown_exception->value()); on termination that value is not a JSObject and the downcast reaches reportZappedCellAndCrash (seen from node:http2's ERR_HTTP2_STREAM_ERROR path inside a terminating worker).

Fix: on termination, return the (possibly partial) created_error instead; the caller's throwException is a no-op with termination pending.

Verification

Three skipIf(!isDebug) tests (release WebKit compiles these asserts out). On a debug build, without the src/ changes:

(fail) terminate() while a worker's Bun.connect() open is firing ...      stderr: "ASSERTION FAILED: !exception() ... ExceptionScope.h(61)"
(fail) terminate() while a worker is inside require() ...                 stderr: "ASSERTION FAILED: wasRemoved ... JSCommonJSModule.cpp(1273)"
(fail) terminate() while a worker is emitting process 'uncaughtException' stderr: "ASSERTION FAILED: !exception() ... ExceptionScope.h(61)"

With the src/ changes all three pass (socket ~50s, CJS ~6s, process ~33s on debug+ASAN). The on_keylog/on_session/ErrorCodeCache sites are reached via node:http2 over TLS and reproduce on a release-asan build; they use the same script_execution_status() / hasPendingTerminationException() guards proven by the three tests above.

no test proof · iteration 3 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/workers/worker-terminate-lifetime.test.ts