mirror of
https://github.com/oven-sh/bun
synced 2026-02-10 19:08:50 +00:00
Fix process hanging when global onmessage is set in main thread
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 <noreply@anthropic.com>
This commit is contained in:
@@ -17,24 +17,29 @@ void WorkerGlobalScope::onDidChangeListenerImpl(EventTarget& self, const AtomStr
|
||||
{
|
||||
if (eventType == eventNames().messageEvent) {
|
||||
auto& global = static_cast<WorkerGlobalScope&>(self);
|
||||
auto* context = global.scriptExecutionContext();
|
||||
// Only ref/unref the event loop if we're in a worker thread, not the main thread.
|
||||
// In the main thread, onmessage handlers shouldn't keep the process alive.
|
||||
bool shouldRefEventLoop = context && !context->isMainThread();
|
||||
|
||||
switch (kind) {
|
||||
case Add:
|
||||
if (global.m_messageEventCount == 0) {
|
||||
global.scriptExecutionContext()->refEventLoop();
|
||||
if (global.m_messageEventCount == 0 && shouldRefEventLoop) {
|
||||
context->refEventLoop();
|
||||
}
|
||||
global.m_messageEventCount++;
|
||||
break;
|
||||
case Remove:
|
||||
global.m_messageEventCount--;
|
||||
if (global.m_messageEventCount == 0) {
|
||||
global.scriptExecutionContext()->unrefEventLoop();
|
||||
if (global.m_messageEventCount == 0 && shouldRefEventLoop) {
|
||||
context->unrefEventLoop();
|
||||
}
|
||||
break;
|
||||
// I dont think clear in this context is ever called. If it is (search OnDidChangeListenerKind::Clear for the impl),
|
||||
// it may actually call once per event, in a way the Remove code above would suffice.
|
||||
case Clear:
|
||||
if (global.m_messageEventCount > 0) {
|
||||
global.scriptExecutionContext()->unrefEventLoop();
|
||||
if (global.m_messageEventCount > 0 && shouldRefEventLoop) {
|
||||
context->unrefEventLoop();
|
||||
}
|
||||
global.m_messageEventCount = 0;
|
||||
break;
|
||||
|
||||
70
test/js/web/workers/onmessage-main-thread.test.ts
Normal file
70
test/js/web/workers/onmessage-main-thread.test.ts
Normal file
@@ -0,0 +1,70 @@
|
||||
import { expect, test } from "bun:test";
|
||||
import { bunEnv, bunExe, tempDir } from "harness";
|
||||
|
||||
test("setting global onmessage in main thread should not prevent process exit", async () => {
|
||||
// This test verifies that setting a global onmessage handler in the main thread
|
||||
// doesn't keep the event loop alive and prevent the process from exiting.
|
||||
// This was a bug where packages like 'lzma' that detect Web Worker environments
|
||||
// by checking `typeof onmessage !== 'undefined'` would inadvertently keep the
|
||||
// process alive.
|
||||
|
||||
using dir = tempDir("onmessage-test", {
|
||||
"test.js": `
|
||||
// Set a global onmessage handler (simulating what the lzma package does)
|
||||
onmessage = function(e) {
|
||||
console.log('received message:', e);
|
||||
};
|
||||
console.log('OK');
|
||||
// Process should exit here, not hang
|
||||
`,
|
||||
});
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "test.js"],
|
||||
env: bunEnv,
|
||||
cwd: String(dir),
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stdout.trim()).toBe("OK");
|
||||
expect(stderr).toBe("");
|
||||
expect(exitCode).toBe(0);
|
||||
}, 5000); // 5 second timeout - should exit quickly
|
||||
|
||||
test("setting global onmessage in worker thread should work normally", async () => {
|
||||
// This test verifies that onmessage in a worker thread still works correctly
|
||||
// and doesn't exit prematurely.
|
||||
|
||||
using dir = tempDir("onmessage-worker-test", {
|
||||
"worker.js": `
|
||||
onmessage = function(e) {
|
||||
postMessage('received: ' + e.data);
|
||||
};
|
||||
`,
|
||||
"main.js": `
|
||||
const worker = new Worker(new URL('worker.js', import.meta.url).href);
|
||||
worker.postMessage('hello');
|
||||
worker.onmessage = (e) => {
|
||||
console.log(e.data);
|
||||
worker.terminate();
|
||||
};
|
||||
`,
|
||||
});
|
||||
|
||||
await using proc = Bun.spawn({
|
||||
cmd: [bunExe(), "main.js"],
|
||||
env: bunEnv,
|
||||
cwd: String(dir),
|
||||
stdout: "pipe",
|
||||
stderr: "pipe",
|
||||
});
|
||||
|
||||
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
|
||||
|
||||
expect(stdout.trim()).toBe("received: hello");
|
||||
expect(stderr).toBe("");
|
||||
expect(exitCode).toBe(0);
|
||||
}, 5000);
|
||||
Reference in New Issue
Block a user