Skip to content

Commit 3c8e06c

Browse files
committed
Fix race condition in checkMailbox
The `checkMailbox` callback can occur after the thread has terminated. In this case calling into native code can trigger the `makeAbortWrapper` wrapper that is put around each native function which then results in a "program has already aborted!" error being thrown. Once solution to this is to make sure that the function which are called do not have `makeAbortWrapper` applied to them. This was the technique I used in #18754, but the list of functions became stale when emscripten_proxy_execute_task_queue was removed in #18852. A better solution is to wrap to whole function in callUserCallback, which takes case of checking if the runtime is alive before calling into native code. Fixes: #20067
1 parent fe2e532 commit 3c8e06c

File tree

3 files changed

+21
-8
lines changed

3 files changed

+21
-8
lines changed

src/lib/libcore.js

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2113,8 +2113,19 @@ addToLibrary({
21132113
},
21142114

21152115
#else // MINIMAL_RUNTIME
2116-
// MINIMAL_RUNTIME doesn't support the runtimeKeepalive stuff
2117-
$callUserCallback: (func) => func(),
2116+
$callUserCallback: (func) => {
2117+
// MINIMAL_RUNTIME doesn't support the runtimeKeepalive stuff, but under
2118+
// some circumstances it supportes `runtimeExited`
2119+
#if EXIT_RUNTIME
2120+
if (runtimeExited) {
2121+
#if ASSERTIONS
2122+
err('user callback triggered after runtime exited or application aborted. Ignoring.');
2123+
#endif
2124+
return;
2125+
}
2126+
#endif
2127+
func();
2128+
},
21182129
#endif // MINIMAL_RUNTIME
21192130

21202131
$asmjsMangle: (x) => {

src/lib/libpthread.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,19 +1215,23 @@ var LibraryPThread = {
12151215
'pthread_self',
12161216
'_emscripten_check_mailbox',
12171217
'_emscripten_thread_mailbox_await'],
1218-
$checkMailbox: () => {
1218+
$checkMailbox: () => callUserCallback(() => {
12191219
// Only check the mailbox if we have a live pthread runtime. We implement
12201220
// pthread_self to return 0 if there is no live runtime.
1221+
//
1222+
// TODO(sbc): Is this check still needed? `callUserCallback` is supposed to
1223+
// ensure the runtime is alive, and if `_pthread_self` is NULL then the
1224+
// runtime certainly is *not* alive, so this should be a redundant check.
12211225
var pthread_ptr = _pthread_self();
12221226
if (pthread_ptr) {
12231227
// If we are using Atomics.waitAsync as our notification mechanism, wait
12241228
// for a notification before processing the mailbox to avoid missing any
12251229
// work that could otherwise arrive after we've finished processing the
12261230
// mailbox and before we're ready for the next notification.
12271231
__emscripten_thread_mailbox_await(pthread_ptr);
1228-
callUserCallback(__emscripten_check_mailbox);
1232+
__emscripten_check_mailbox();
12291233
}
1230-
},
1234+
}),
12311235

12321236
_emscripten_thread_mailbox_await__deps: ['$checkMailbox'],
12331237
_emscripten_thread_mailbox_await: (pthread_ptr) => {

tools/emscripten.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -897,9 +897,7 @@ def install_debug_wrapper(sym):
897897
return False
898898
# Likewise `__trap` can occur before the runtime is initialized since it is used in
899899
# abort.
900-
# pthread_self is currently called in some cases after the runtime has exited.
901-
# TODO: Look into removing these, and improving our robustness around thread termination.
902-
return sym not in {'__trap', 'pthread_self'}
900+
return sym != '__trap'
903901

904902

905903
def should_export(sym):

0 commit comments

Comments
 (0)