Allow late print calls from terminating pthreads#27012
Open
brendandahl wants to merge 1 commit into
Open
Conversation
When a pthreaded application exits under PROXY_TO_PTHREAD and EXIT_RUNTIME, the worker thread signals the main thread to exit using the system queue. However, there can also be messages from postMessage in-flight from the worker. This results in a race condition where the main thread processes the exit and terminates the worker (synchronously stubbing out its onmessage handler to discard further messages) before processing print messages already posted by the worker before exiting. As a result, valid printed output is lost, and the test fails due to unexpected assertion warnings. Resolve this by allowing late-arriving print and printErr commands through the terminated-worker message stub. Fixes flaky test test_pthread_print_override_modularize. Fixes emscripten-core#19683
sbc100
reviewed
May 26, 2026
| // rather than discarding them, which ensures standard output is not dropped | ||
| // on exit and avoids unexpected "received command from terminated worker" | ||
| // assertion failures. | ||
| if (cmd === {{{ CMD_CALL_HANDLER }}} && (d.handler === 'print' || d.handler === 'printErr')) { |
Collaborator
There was a problem hiding this comment.
Should we maybe allow this for all CMD_CALL_HANDLER?
sbc100
reviewed
May 26, 2026
| } | ||
| #if ASSERTIONS | ||
| var cmd = e['data'].cmd; | ||
| err(`received "${cmd}" command from terminated worker: ${worker.workerID}`); |
Collaborator
There was a problem hiding this comment.
Perhaps we should replace received with ignoring here? And also maybe replace err with dbg?
sbc100
reviewed
May 26, 2026
| @@ -596,8 +595,18 @@ var LibraryPThread = { | |||
| // out its message handler here. This avoids having to check in each of | |||
| // the onmessage handlers if the message was coming from a valid worker. | |||
| worker.onmessage = (e) => { | |||
Collaborator
There was a problem hiding this comment.
I wonder if we should consider just removing this stubbing of the onmessage handler?
Collaborator
Author
There was a problem hiding this comment.
Maybe, I didn't look much at the history, but the comment made it seem important.
sbc100
reviewed
May 26, 2026
| // flight in the message queue of the main thread. We should execute them | ||
| // rather than discarding them, which ensures standard output is not dropped | ||
| // on exit and avoids unexpected "received command from terminated worker" | ||
| // assertion failures. |
Collaborator
There was a problem hiding this comment.
assertion failures -> error logs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a pthreaded application exits under PROXY_TO_PTHREAD and EXIT_RUNTIME, the worker thread signals the main
thread to exit using the system queue. However, there can also be messages from postMessage in-flight from the worker.
This results in a race condition where the main thread processes the exit and terminates the worker (synchronously stubbing out its onmessage handler to discard further messages) before processing print messages already posted by the worker before exiting. As a result, valid printed output is lost, and the test fails due to unexpected assertion warnings.
Resolve this by allowing late-arriving print and printErr commands through the terminated-worker message stub. Fixes flaky test test_pthread_print_override_modularize.
Fixes #19683