fix(worker): don't keep the main thread alive for a global message listener#32482
fix(worker): don't keep the main thread alive for a global message listener#324820xfandom wants to merge 1 commit into
Conversation
…stener
Adding a "message" listener to the global scope refs the event loop so a Worker
stays alive to receive messages from its parent. On the main thread there is no
parent, so `globalThis.onmessage = ...` (or addEventListener("message", ...))
kept the process running forever instead of exiting like Node.
Skip the event-loop ref/unref when the global scope is on the main thread;
worker threads still get the keepalive.
Fixes oven-sh#24256
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughIn ChangesMain-thread message listener event loop fix
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
What does this PR do?
Fixes #24256. Setting a global
messagelistener on the main thread kept the process running forever instead of exiting:WorkerGlobalScope::onDidChangeListenerImplrefs the event loop whenever amessagelistener is added, so a Worker stays alive to receive messages from its parent. That is correct for worker threads, but on the main thread there is no parent message channel, so the ref is never balanced and the process never exits.The fix skips the event-loop ref/unref when the global scope is on the main thread (
ScriptExecutionContext::isMainThread()). Worker threads still get the keepalive, so they continue to receive messages. The same path backsaddEventListener("message", ...), so both spellings are covered.How did you verify your code works?
Added a parameterized test in
test/js/web/workers/worker.test.tsthat spawns a main-thread process setting a globalmessagelistener via bothglobalThis.onmessage =andaddEventListener("message", ...), and asserts it exits on its own (exitCode === 0,signalCode === null). The test times out (hangs) on the releasedbunand passes on the debug build.Also confirmed a Worker that sets
globalThis.onmessagestill receives messages from its parent (existing worker suite passes: 25 pass / 1 todo).