Allow late print calls from terminating pthreads#27012
Conversation
| @@ -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) => { | |||
There was a problem hiding this comment.
I wonder if we should consider just removing this stubbing of the onmessage handler?
There was a problem hiding this comment.
Maybe, I didn't look much at the history, but the comment made it seem important.
There was a problem hiding this comment.
I tried removing the stub and it seems to pass tests locally. This was added back in db35f20 and that was added as a speculative fix, so it seems like we could get rid of it.
There was a problem hiding this comment.
Its certainly seems possible that bad/strange things could happen if we process messages from workers that have been terminated... but I'm not sure how to balance that potential risk with the risk of not processing the messages :)
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 messages after termination. Fixes flaky test test_pthread_print_override_modularize. Fixes emscripten-core#19683
brendandahl
force-pushed
the
flaky-test_pthread_print_override_modularize
branch
from
8db019b to
f14aa9b
Compare
|
Can you update the PR description? Can you reference the PR that this is effectively reverting? |
|
Interestingly test_pthread_print_override_modularize is now failing on deno with a missing hello world message. |
Is it just flaky? |
2 participants
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