Fix process hanging when global onmessage is set in main thread #24496

robobun · 2025-11-08T03:49:55Z

Summary

Fixes an issue where requiring packages like lzma that set a global onmessage handler would prevent the Bun process from exiting.

Problem

Setting a global onmessage handler in the main thread was incorrectly keeping the event loop alive and preventing the process from terminating. This affected packages like lzma that detect Web Worker environments by checking typeof onmessage !== 'undefined'.

Example of the bug:

require("lzma");
console.log("OK");
// Process would hang here instead of exiting

Root Cause

In BunWorkerGlobalScope.cpp, adding a message event listener would always call refEventLoop(), even in the main thread. This was intended for worker threads where message listeners should keep the worker alive while waiting for messages from the parent.

Solution

Added a check to only ref/unref the event loop for message listeners when we're in a worker thread (not the main thread). Main thread message handlers are now correctly treated as passive listeners that don't prevent process exit.

bool shouldRefEventLoop = context && !context->isMainThread();

Testing

✅ Created comprehensive tests in test/js/web/workers/onmessage-main-thread.test.ts
✅ Verified main thread onmessage handlers don't prevent exit
✅ Verified worker thread onmessage handlers still work correctly
✅ Tested with the original lzma package reproduction case
✅ Tests pass with fix, fail with old behavior (USE_SYSTEM_BUN=1)

Changes

Modified: src/bun.js/bindings/BunWorkerGlobalScope.cpp
Added: test/js/web/workers/onmessage-main-thread.test.ts

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

Setting a global onmessage handler in the main thread (e.g., by the lzma package) was incorrectly keeping the event loop alive and preventing the process from exiting. The issue was in BunWorkerGlobalScope.cpp where adding a message event listener would always call refEventLoop(), even in the main thread. This was intended for worker threads where message listeners should keep the worker alive while waiting for messages from the parent. The fix adds a check to only ref/unref the event loop for message listeners when we're in a worker thread (not the main thread). Main thread message handlers are now correctly treated as passive listeners that don't prevent process exit. Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

robobun · 2025-11-08T03:50:05Z

^{Updated 10:17 PM PT - Nov 7th, 2025}

❌ Your commit 91d9023d has 3 failures in Build #31287 (All Failures):

test/v8/v8.test.ts - 1 failing on 🐧 12 x64-asan
test/js/web/workers/onmessage-main-thread.test.ts - core dumped on 🐧 12 x64-asan (new)
test/js/node/path/browserify.test.js - 1 failing on 🪟 2019 x64-baseline
test/js/node/path/browserify.test.js - 1 failing on 🪟 2019 x64

🧪 To try this PR locally:

bunx bun-pr 24496

That installs a local version of the PR into your bun-24496 executable, so you can run:

bun-24496 --bun

coderabbitai · 2025-11-08T03:53:01Z

Walkthrough

Adds context-aware ref/unref guards to prevent the main thread from remaining alive when an onmessage listener is registered, while preserving correct behavior for worker threads. Includes tests to verify process exit behavior with global onmessage handlers.

Changes

Cohort / File(s)	Summary
Event loop ref/unref guard `src/bun.js/bindings/BunWorkerGlobalScope.cpp`	Introduces shouldRefEventLoop flag based on execution context to conditionally call refEventLoop/unrefEventLoop. Refactors Add/Remove/Clear branches and replaces direct global.scriptExecutionContext() calls with local context references. Adds comments explaining intent to avoid keeping event loop alive on main thread.
Process lifecycle tests `test/js/web/workers/onmessage-main-thread.test.ts`	Adds test file with two test cases: verifies that setting global onmessage in main thread does not prevent process exit, and verifies that onmessage handler works correctly in worker threads. Both tests check stdout, stderr, and exit codes.

Pre-merge checks

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing process hanging when global onmessage is set in main thread, which directly matches the core issue addressed in the changeset.
Description check	✅ Passed	The description provides comprehensive coverage including problem statement, root cause analysis, solution explanation, testing verification, and file changes. Both required template sections (What does this PR do? and How did you verify?) are thoroughly addressed.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 35a57ff and 91d9023.

📒 Files selected for processing (2)

src/bun.js/bindings/BunWorkerGlobalScope.cpp (1 hunks)
test/js/web/workers/onmessage-main-thread.test.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

**/*.{cpp,h}

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible

Files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

**/*.cpp

📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)

**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue

Files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

test/**

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place all tests under the test/ directory

Files:

test/js/web/workers/onmessage-main-thread.test.ts

test/js/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place JavaScript and TypeScript tests under test/js/

Files:

test/js/web/workers/onmessage-main-thread.test.ts

test/js/web/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

Place Web API tests under test/js/web/, separated by category

Files:

test/js/web/workers/onmessage-main-thread.test.ts

test/**/*.{js,ts}

📄 CodeRabbit inference engine (.cursor/rules/writing-tests.mdc)

test/**/*.{js,ts}: Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test
Prefer data-driven tests (e.g., test.each) to reduce boilerplate
Use shared utilities from test/harness.ts where applicable

Files:

test/js/web/workers/onmessage-main-thread.test.ts

🧠 Learnings (23)

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.cpp : Create V8 handles using isolate->currentHandleScope()->createLocal<T>() within an active handle scope

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.{h,cpp} : Use JSC::WriteBarrier for heap-allocated references and implement visitChildren() for custom heap objects

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-08-30T00:13:36.815Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-10-01T21:59:54.571Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.{h,cpp} : Use localToJSValue() when converting V8 handles to JSC values before performing JSC operations

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.txt : Add new V8 API mangled symbols (without leading underscore) to src/symbols.txt

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-08-30T00:11:00.890Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.cpp : Initialize the LazyClassStructure in GlobalObject::finishCreation and visit it in GlobalObject::visitChildrenImpl

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/symbols.dyn : Add new V8 API mangled symbols (with leading underscore and semicolons) to src/symbols.dyn

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-10-01T21:49:27.862Z

Learnt from: taylordotfish
Repo: oven-sh/bun PR: 23169
File: src/bun.js/bindings/BunIDLConvert.h:29-42
Timestamp: 2025-10-01T21:49:27.862Z
Learning: In Bun's IDL bindings (src/bun.js/bindings/BunIDLConvert.h), IDLStrictNull intentionally treats both undefined and null as null (using isUndefinedOrNull()), matching WebKit's IDLNull & IDLNullable behavior. This is the correct implementation and should not be changed to only accept null.

Applied to files:

src/bun.js/bindings/BunWorkerGlobalScope.cpp

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/js/bun/**/*.{js,ts} : Place Bun API tests under test/js/bun/, separated by category (e.g., test/js/bun/glob/)

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/cli/**/*.{js,ts} : When testing Bun as a CLI, use spawn with bunExe() and bunEnv from harness, and capture stdout/stderr via pipes

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-09-08T00:41:12.052Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add a corresponding test case in test/v8/v8.test.ts that invokes checkSameOutput with the new function

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: Applies to test/**/*.{js,ts} : Write tests in JavaScript or TypeScript using Bun’s Jest-style APIs (test, describe, expect) and run with bun test

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-11-06T00:58:23.965Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-10-19T02:44:46.354Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: packages/bun-otel/context-propagation.test.ts:1-1
Timestamp: 2025-10-19T02:44:46.354Z
Learning: In the Bun repository, standalone packages under packages/ (e.g., bun-vscode, bun-inspector-protocol, bun-plugin-yaml, bun-plugin-svelte, bun-debug-adapter-protocol, bun-otel) co-locate their tests with package source code using *.test.ts files. This follows standard npm/monorepo patterns. The test/ directory hierarchy (test/js/bun/, test/cli/, test/js/node/) is reserved for testing Bun's core runtime APIs and built-in functionality, not standalone packages.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-08-30T00:09:39.100Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/dev-server-tests.mdc:0-0
Timestamp: 2025-08-30T00:09:39.100Z
Learning: Applies to test/bake/client-fixture.mjs : Client subprocess logic (page load, IPC, error/reload coordination) should live in client-fixture.mjs and be driven by Client

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-10-19T04:55:33.099Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-10-18T05:23:24.403Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-10-26T01:32:04.844Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-08-30T00:12:56.803Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: .cursor/rules/writing-tests.mdc:0-0
Timestamp: 2025-08-30T00:12:56.803Z
Learning: For Node.js compatibility bugs, validate expected behavior in Node.js first, then confirm failure on Bun canary

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-10-08T13:48:02.430Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 23373
File: test/js/bun/tarball/extract.test.ts:107-111
Timestamp: 2025-10-08T13:48:02.430Z
Learning: In Bun's test runner, use `expect(async () => { await ... }).toThrow()` to assert async rejections. Unlike Jest/Vitest, Bun does not require `await expect(...).rejects.toThrow()` - the async function wrapper with `.toThrow()` is the correct pattern for async error assertions in Bun tests.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

📚 Learning: 2025-10-25T17:20:19.041Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 24063
File: test/js/bun/telemetry/server-header-injection.test.ts:5-20
Timestamp: 2025-10-25T17:20:19.041Z
Learning: In the Bun telemetry codebase, tests are organized into two distinct layers: (1) Internal API tests in test/js/bun/telemetry/ use numeric InstrumentKind enum values to test Zig↔JS injection points and low-level integration; (2) Public API tests in packages/bun-otel/test/ use string InstrumentKind values ("http", "fetch", etc.) to test the public-facing BunSDK and instrumentation APIs. This separation allows internal tests to use efficient numeric enums for refactoring flexibility while the public API maintains a developer-friendly string-based interface.

Applied to files:

test/js/web/workers/onmessage-main-thread.test.ts

🧬 Code graph analysis (1)

test/js/web/workers/onmessage-main-thread.test.ts (1)

test/harness.ts (2)

tempDir (277-284)

bunExe (102-105)

🔇 Additional comments (5)

src/bun.js/bindings/BunWorkerGlobalScope.cpp (3)

27-28: LGTM! Consistent and correct guarding.

The Add and Remove cases correctly apply the shouldRefEventLoop guard to prevent main thread event loop references while preserving the message event count tracking for all contexts.

Also applies to: 34-35

38-45: LGTM! Guard applied consistently to Clear case.

The Clear case correctly applies the same guarding logic. The existing comment appropriately notes uncertainty about whether this path is ever exercised.

20-23: LGTM! API verified and null-check is safe.

The isMainThread() method (defined in ScriptExecutionContext.h as bool isMainThread() const { return m_identifier == 1; }) is correctly used to distinguish worker threads from the main thread. The code properly null-checks context before dereferencing, and the logic correctly prevents main thread message handlers from keeping the process alive.

test/js/web/workers/onmessage-main-thread.test.ts (2)

4-35: LGTM! Comprehensive test for main thread behavior.

The test correctly verifies that setting a global onmessage handler in the main thread does not prevent process exit. The test structure is sound:

Uses await using for proper cleanup

Spawns Bun to run a script (not using bun test)

Comprehensive assertions on stdout, stderr, and exit code

Appropriate 5-second timeout

37-70: LGTM! Comprehensive test for worker thread behavior.

The test correctly verifies that onmessage in a worker thread continues to function normally. The test structure properly:

Creates both worker and main thread scripts

Verifies bidirectional communication (postMessage → onmessage → postMessage → onmessage)

Terminates the worker after communication completes

Uses proper cleanup with await using

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions bot added the claude label Nov 8, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix process hanging when global onmessage is set in main thread #24496

Fix process hanging when global onmessage is set in main thread #24496

robobun commented Nov 8, 2025

Uh oh!

robobun commented Nov 8, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Nov 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix process hanging when global onmessage is set in main thread #24496

Are you sure you want to change the base?

Fix process hanging when global onmessage is set in main thread #24496

Conversation

robobun commented Nov 8, 2025

Summary

Problem

Root Cause

Solution

Testing

Changes

Uh oh!

robobun commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Nov 8, 2025

Walkthrough

Changes

Pre-merge checks

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

robobun commented Nov 8, 2025 •

edited

Loading