From a6d108a812f3a99910cd7ebf958ef3289cc733c3 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 14 Feb 2023 17:15:48 -0800 Subject: [PATCH] Avoid calling `process.exit` under node Instead we always prefer to let exceptions bubble out and get handles by whatever handlers are installed. Doing this did exposed a few places where we were calling back into the runtime after it had exited. This was previously being masked by the fact that we were bringing down the entire node process. This means that when running tests under node we will now be a little more sensitive to use-after-exit bugs in our runtime and test code, but I would argue this is good thing. This also means we no longer need to define logExceptionOnExit under node, which is node code size saving. --- emscripten.py | 23 ++++++++---- src/library_pthread.js | 11 ++---- src/shell.js | 36 ++++++------------- src/threadprofiler.js | 6 ++-- .../metadce/test_metadce_cxx_ctors1.jssize | 2 +- .../metadce/test_metadce_cxx_ctors2.jssize | 2 +- .../metadce/test_metadce_cxx_except.jssize | 2 +- .../test_metadce_cxx_except_wasm.jssize | 2 +- .../metadce/test_metadce_cxx_mangle.jssize | 2 +- .../metadce/test_metadce_cxx_noexcept.jssize | 2 +- .../metadce/test_metadce_hello_O0.jssize | 2 +- .../metadce/test_metadce_hello_O1.jssize | 2 +- .../metadce/test_metadce_hello_O2.jssize | 2 +- .../metadce/test_metadce_hello_O3.jssize | 2 +- .../metadce/test_metadce_hello_Os.jssize | 2 +- .../metadce/test_metadce_hello_Oz.jssize | 2 +- .../metadce/test_metadce_hello_dylink.jssize | 2 +- .../test_metadce_hello_export_nothing.jssize | 2 +- .../test_metadce_libcxxabi_message_O3.jssize | 2 +- ...dce_libcxxabi_message_O3_standalone.jssize | 2 +- test/other/metadce/test_metadce_mem_O3.jssize | 2 +- .../metadce/test_metadce_mem_O3_grow.jssize | 2 +- ...test_metadce_mem_O3_grow_standalone.jssize | 2 +- .../test_metadce_mem_O3_standalone.jssize | 2 +- .../test_metadce_mem_O3_standalone_lib.jssize | 2 +- ...test_metadce_mem_O3_standalone_narg.jssize | 2 +- ...metadce_mem_O3_standalone_narg_flto.jssize | 2 +- .../metadce/test_metadce_minimal_O0.jssize | 2 +- .../metadce/test_metadce_minimal_O1.jssize | 2 +- .../metadce/test_metadce_minimal_O2.jssize | 2 +- .../metadce/test_metadce_minimal_O3.jssize | 2 +- .../metadce/test_metadce_minimal_Os.jssize | 2 +- .../test_metadce_minimal_Oz-ctors.jssize | 2 +- .../metadce/test_metadce_minimal_Oz.jssize | 2 +- test/other/test_runtime_keepalive.cpp | 4 ++- test/other/test_unoptimized_code_size.js.size | 2 +- ...t_unoptimized_code_size_no_asserts.js.size | 2 +- .../test_unoptimized_code_size_strict.js.size | 2 +- 38 files changed, 70 insertions(+), 76 deletions(-) diff --git a/emscripten.py b/emscripten.py index 19459d9c2bc90..989aa6edf9c91 100644 --- a/emscripten.py +++ b/emscripten.py @@ -716,6 +716,22 @@ def create_sending(metadata, library_symbols): def make_export_wrappers(exports, delay_assignment): wrappers = [] + + # The emscripten stack functions are called very early (by writeStackCookie) before + # the runtime is initialized so we can't create these wrappers that check for + # runtimeInitialized. + # Likewise `__trap` can occur before the runtime is initialized since it is used in + # abort. + # pthread_self and _emscripten_proxy_execute_task_queue are currently called in some + # cases after the runtime has exited. + # TODO: Look into removing these, and improving our robustness around thread termination. + def install_wrapper(sym): + if sym.startswith('_asan_') or sym.startswith('emscripten_stack_'): + return False + if sym in ('__trap', 'pthread_self', '_emscripten_proxy_execute_task_queue'): + return False + return True + for name in exports: # Tags cannot be wrapped in createExportWrapper if name == '__cpp_exception': @@ -730,12 +746,7 @@ def make_export_wrappers(exports, delay_assignment): exported = '' wrapper += exported - # The emscripten stack functions are called very early (by writeStackCookie) before - # the runtime is initialized so we can't create these wrappers that check for - # runtimeInitialized. - # Likewise `__trap` can occur before the runtime is initialized since it is used in - # abort. - if settings.ASSERTIONS and not name.startswith('emscripten_stack_') and name != '__trap': + if settings.ASSERTIONS and install_wrapper(name): # With assertions enabled we create a wrapper that are calls get routed through, for # the lifetime of the program. if delay_assignment: diff --git a/src/library_pthread.js b/src/library_pthread.js index 4fa40aeee83ee..3255dc0b991e4 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -526,6 +526,9 @@ var LibraryPThread = { }, $terminateWorker: function(worker) { +#if PTHREADS_DEBUG + dbg('terminateWorker: ' + worker.workerID); +#endif worker.terminate(); // terminate() can be asynchronous, so in theory the worker can continue // to run for some amount of time after termination. However from our POV @@ -942,15 +945,7 @@ var LibraryPThread = { #if PROXY_TO_PTHREAD {{{ runtimeKeepalivePop() }}}; #endif -#if MINIMAL_RUNTIME _exit(returnCode); -#else - try { - _exit(returnCode); - } catch (e) { - handleException(e); - } -#endif }, emscripten_proxy_to_main_thread_js__deps: ['$withStackSave', '_emscripten_run_in_main_runtime_thread_js'], diff --git a/src/shell.js b/src/shell.js index 9ebc107d28f39..c9166edb70e3c 100644 --- a/src/shell.js +++ b/src/shell.js @@ -172,24 +172,6 @@ var read_, readBinary, setWindowTitle; -#if ENVIRONMENT_MAY_BE_SHELL || ENVIRONMENT_MAY_BE_NODE || ASSERTIONS -// Normally we don't log exceptions but instead let them bubble out the top -// level where the embedding environment (e.g. the browser) can handle -// them. -// However under v8 and node we sometimes exit the process direcly in which case -// its up to use us to log the exception before exiting. -// If we fix https://github.com/emscripten-core/emscripten/issues/15080 -// this may no longer be needed under node. -function logExceptionOnExit(e) { - if (e instanceof ExitStatus) return; - let toLog = e; - if (e && typeof e == 'object' && e.stack) { - toLog = [e, e.stack]; - } - err('exiting due to exception: ' + toLog); -} -#endif - #if ENVIRONMENT_MAY_BE_NODE if (ENVIRONMENT_IS_NODE) { #if ENVIRONMENT && ASSERTIONS @@ -244,7 +226,7 @@ if (ENVIRONMENT_IS_NODE) { #if RUNTIME_DEBUG dbg('node: uncaughtException: ' + ex) #endif - if (!(ex instanceof ExitStatus)) { + if (ex !== 'unwind' && !(ex instanceof ExitStatus) && !(ex.context instanceof ExitStatus)) { throw ex; } }); @@ -263,12 +245,8 @@ if (ENVIRONMENT_IS_NODE) { #endif quit_ = (status, toThrow) => { - if (keepRuntimeAlive()) { - process.exitCode = status; - throw toThrow; - } - logExceptionOnExit(toThrow); - process.exit(status); + process.exitCode = status; + throw toThrow; }; Module['inspect'] = function () { return '[Emscripten Module object]'; }; @@ -360,7 +338,13 @@ if (ENVIRONMENT_IS_SHELL) { throw toThrow; } #endif - logExceptionOnExit(toThrow); + if (!(toThrow instanceof ExitStatus)) { + let toLog = toThrow; + if (toThrow && typeof toThrow == 'object' && toThrow.stack) { + toLog = [toThrow, toThrow.stack]; + } + err('exiting due to exception: ' + toLog); + } quit(status); }; } diff --git a/src/threadprofiler.js b/src/threadprofiler.js index 7f348996226a0..21370b93fdac5 100644 --- a/src/threadprofiler.js +++ b/src/threadprofiler.js @@ -20,13 +20,15 @@ var emscriptenThreadProfiler = { document.body.appendChild(div); this.threadProfilerDiv = document.getElementById('threadprofiler'); } - setInterval(function() { emscriptenThreadProfiler.updateUi() }, this.uiUpdateIntervalMsecs); + var i = setInterval(function() { emscriptenThreadProfiler.updateUi() }, this.uiUpdateIntervalMsecs); + addOnExit(() => clearInterval(i)); }, initializeNode: function initializeNode() { addOnInit(() => { emscriptenThreadProfiler.dumpState(); - setInterval(function() { emscriptenThreadProfiler.dumpState() }, this.uiUpdateIntervalMsecs); + var i = setInterval(function() { emscriptenThreadProfiler.dumpState() }, this.uiUpdateIntervalMsecs); + addOnExit(() => clearInterval(i)); }); }, diff --git a/test/other/metadce/test_metadce_cxx_ctors1.jssize b/test/other/metadce/test_metadce_cxx_ctors1.jssize index c2e7df0e7c4b5..258a27db4d100 100644 --- a/test/other/metadce/test_metadce_cxx_ctors1.jssize +++ b/test/other/metadce/test_metadce_cxx_ctors1.jssize @@ -1 +1 @@ -26171 +26046 diff --git a/test/other/metadce/test_metadce_cxx_ctors2.jssize b/test/other/metadce/test_metadce_cxx_ctors2.jssize index ef8b596a9c400..df6676401a863 100644 --- a/test/other/metadce/test_metadce_cxx_ctors2.jssize +++ b/test/other/metadce/test_metadce_cxx_ctors2.jssize @@ -1 +1 @@ -26135 +26010 diff --git a/test/other/metadce/test_metadce_cxx_except.jssize b/test/other/metadce/test_metadce_cxx_except.jssize index 0fc360156f899..843c780975de7 100644 --- a/test/other/metadce/test_metadce_cxx_except.jssize +++ b/test/other/metadce/test_metadce_cxx_except.jssize @@ -1 +1 @@ -30708 +30583 diff --git a/test/other/metadce/test_metadce_cxx_except_wasm.jssize b/test/other/metadce/test_metadce_cxx_except_wasm.jssize index c6045c79a09d3..c1668a6c1f6f0 100644 --- a/test/other/metadce/test_metadce_cxx_except_wasm.jssize +++ b/test/other/metadce/test_metadce_cxx_except_wasm.jssize @@ -1 +1 @@ -25850 +25725 diff --git a/test/other/metadce/test_metadce_cxx_mangle.jssize b/test/other/metadce/test_metadce_cxx_mangle.jssize index 0fc360156f899..843c780975de7 100644 --- a/test/other/metadce/test_metadce_cxx_mangle.jssize +++ b/test/other/metadce/test_metadce_cxx_mangle.jssize @@ -1 +1 @@ -30708 +30583 diff --git a/test/other/metadce/test_metadce_cxx_noexcept.jssize b/test/other/metadce/test_metadce_cxx_noexcept.jssize index c2e7df0e7c4b5..258a27db4d100 100644 --- a/test/other/metadce/test_metadce_cxx_noexcept.jssize +++ b/test/other/metadce/test_metadce_cxx_noexcept.jssize @@ -1 +1 @@ -26171 +26046 diff --git a/test/other/metadce/test_metadce_hello_O0.jssize b/test/other/metadce/test_metadce_hello_O0.jssize index ee3f753b14deb..d4024fcbba7f2 100644 --- a/test/other/metadce/test_metadce_hello_O0.jssize +++ b/test/other/metadce/test_metadce_hello_O0.jssize @@ -1 +1 @@ -23266 +23199 diff --git a/test/other/metadce/test_metadce_hello_O1.jssize b/test/other/metadce/test_metadce_hello_O1.jssize index 72f3d2d69d081..e4d150b45c423 100644 --- a/test/other/metadce/test_metadce_hello_O1.jssize +++ b/test/other/metadce/test_metadce_hello_O1.jssize @@ -1 +1 @@ -8772 +8555 diff --git a/test/other/metadce/test_metadce_hello_O2.jssize b/test/other/metadce/test_metadce_hello_O2.jssize index 14e5d050f7b79..e086a733090d3 100644 --- a/test/other/metadce/test_metadce_hello_O2.jssize +++ b/test/other/metadce/test_metadce_hello_O2.jssize @@ -1 +1 @@ -6204 +6070 diff --git a/test/other/metadce/test_metadce_hello_O3.jssize b/test/other/metadce/test_metadce_hello_O3.jssize index 3ce941c9ea596..6762ad20ec827 100644 --- a/test/other/metadce/test_metadce_hello_O3.jssize +++ b/test/other/metadce/test_metadce_hello_O3.jssize @@ -1 +1 @@ -6030 +5896 diff --git a/test/other/metadce/test_metadce_hello_Os.jssize b/test/other/metadce/test_metadce_hello_Os.jssize index 3ce941c9ea596..6762ad20ec827 100644 --- a/test/other/metadce/test_metadce_hello_Os.jssize +++ b/test/other/metadce/test_metadce_hello_Os.jssize @@ -1 +1 @@ -6030 +5896 diff --git a/test/other/metadce/test_metadce_hello_Oz.jssize b/test/other/metadce/test_metadce_hello_Oz.jssize index 86278f6f2c228..e885b56e61b25 100644 --- a/test/other/metadce/test_metadce_hello_Oz.jssize +++ b/test/other/metadce/test_metadce_hello_Oz.jssize @@ -1 +1 @@ -5989 +5855 diff --git a/test/other/metadce/test_metadce_hello_dylink.jssize b/test/other/metadce/test_metadce_hello_dylink.jssize index 05216d319a262..d3fab69fd5293 100644 --- a/test/other/metadce/test_metadce_hello_dylink.jssize +++ b/test/other/metadce/test_metadce_hello_dylink.jssize @@ -1 +1 @@ -27883 +27757 diff --git a/test/other/metadce/test_metadce_hello_export_nothing.jssize b/test/other/metadce/test_metadce_hello_export_nothing.jssize index 899315a9e2b94..d464190f4f303 100644 --- a/test/other/metadce/test_metadce_hello_export_nothing.jssize +++ b/test/other/metadce/test_metadce_hello_export_nothing.jssize @@ -1 +1 @@ -4623 +4639 diff --git a/test/other/metadce/test_metadce_libcxxabi_message_O3.jssize b/test/other/metadce/test_metadce_libcxxabi_message_O3.jssize index 3ae79a93ab2b2..e96a113a815ce 100644 --- a/test/other/metadce/test_metadce_libcxxabi_message_O3.jssize +++ b/test/other/metadce/test_metadce_libcxxabi_message_O3.jssize @@ -1 +1 @@ -5292 +5170 diff --git a/test/other/metadce/test_metadce_libcxxabi_message_O3_standalone.jssize b/test/other/metadce/test_metadce_libcxxabi_message_O3_standalone.jssize index 0dd296e9adb3f..b399a63028457 100644 --- a/test/other/metadce/test_metadce_libcxxabi_message_O3_standalone.jssize +++ b/test/other/metadce/test_metadce_libcxxabi_message_O3_standalone.jssize @@ -1 +1 @@ -5365 +5238 diff --git a/test/other/metadce/test_metadce_mem_O3.jssize b/test/other/metadce/test_metadce_mem_O3.jssize index 243df731dfdf7..1f10f1976c57e 100644 --- a/test/other/metadce/test_metadce_mem_O3.jssize +++ b/test/other/metadce/test_metadce_mem_O3.jssize @@ -1 +1 @@ -6217 +6092 diff --git a/test/other/metadce/test_metadce_mem_O3_grow.jssize b/test/other/metadce/test_metadce_mem_O3_grow.jssize index c4ab99baca4ea..725a719f3bcb1 100644 --- a/test/other/metadce/test_metadce_mem_O3_grow.jssize +++ b/test/other/metadce/test_metadce_mem_O3_grow.jssize @@ -1 +1 @@ -6561 +6436 diff --git a/test/other/metadce/test_metadce_mem_O3_grow_standalone.jssize b/test/other/metadce/test_metadce_mem_O3_grow_standalone.jssize index dc9413a580e26..c9a4db7eba921 100644 --- a/test/other/metadce/test_metadce_mem_O3_grow_standalone.jssize +++ b/test/other/metadce/test_metadce_mem_O3_grow_standalone.jssize @@ -1 +1 @@ -5947 +5818 diff --git a/test/other/metadce/test_metadce_mem_O3_standalone.jssize b/test/other/metadce/test_metadce_mem_O3_standalone.jssize index 144f5c74f9246..bac191e478119 100644 --- a/test/other/metadce/test_metadce_mem_O3_standalone.jssize +++ b/test/other/metadce/test_metadce_mem_O3_standalone.jssize @@ -1 +1 @@ -5870 +5741 diff --git a/test/other/metadce/test_metadce_mem_O3_standalone_lib.jssize b/test/other/metadce/test_metadce_mem_O3_standalone_lib.jssize index 4a9e81e7efbce..d57d96f601aca 100644 --- a/test/other/metadce/test_metadce_mem_O3_standalone_lib.jssize +++ b/test/other/metadce/test_metadce_mem_O3_standalone_lib.jssize @@ -1 +1 @@ -5378 +5256 diff --git a/test/other/metadce/test_metadce_mem_O3_standalone_narg.jssize b/test/other/metadce/test_metadce_mem_O3_standalone_narg.jssize index 0dd296e9adb3f..b399a63028457 100644 --- a/test/other/metadce/test_metadce_mem_O3_standalone_narg.jssize +++ b/test/other/metadce/test_metadce_mem_O3_standalone_narg.jssize @@ -1 +1 @@ -5365 +5238 diff --git a/test/other/metadce/test_metadce_mem_O3_standalone_narg_flto.jssize b/test/other/metadce/test_metadce_mem_O3_standalone_narg_flto.jssize index 0dd296e9adb3f..b399a63028457 100644 --- a/test/other/metadce/test_metadce_mem_O3_standalone_narg_flto.jssize +++ b/test/other/metadce/test_metadce_mem_O3_standalone_narg_flto.jssize @@ -1 +1 @@ -5365 +5238 diff --git a/test/other/metadce/test_metadce_minimal_O0.jssize b/test/other/metadce/test_metadce_minimal_O0.jssize index 88b5477e994e2..5a097f8bc5725 100644 --- a/test/other/metadce/test_metadce_minimal_O0.jssize +++ b/test/other/metadce/test_metadce_minimal_O0.jssize @@ -1 +1 @@ -19529 +19563 diff --git a/test/other/metadce/test_metadce_minimal_O1.jssize b/test/other/metadce/test_metadce_minimal_O1.jssize index e3bda3519fc45..5e91da113af69 100644 --- a/test/other/metadce/test_metadce_minimal_O1.jssize +++ b/test/other/metadce/test_metadce_minimal_O1.jssize @@ -1 +1 @@ -4901 +4935 diff --git a/test/other/metadce/test_metadce_minimal_O2.jssize b/test/other/metadce/test_metadce_minimal_O2.jssize index 0235404e234aa..e67b201f450e6 100644 --- a/test/other/metadce/test_metadce_minimal_O2.jssize +++ b/test/other/metadce/test_metadce_minimal_O2.jssize @@ -1 +1 @@ -3782 +3798 diff --git a/test/other/metadce/test_metadce_minimal_O3.jssize b/test/other/metadce/test_metadce_minimal_O3.jssize index 86d81b047bc4c..1149062c66bf4 100644 --- a/test/other/metadce/test_metadce_minimal_O3.jssize +++ b/test/other/metadce/test_metadce_minimal_O3.jssize @@ -1 +1 @@ -3671 +3666 diff --git a/test/other/metadce/test_metadce_minimal_Os.jssize b/test/other/metadce/test_metadce_minimal_Os.jssize index 86d81b047bc4c..1149062c66bf4 100644 --- a/test/other/metadce/test_metadce_minimal_Os.jssize +++ b/test/other/metadce/test_metadce_minimal_Os.jssize @@ -1 +1 @@ -3671 +3666 diff --git a/test/other/metadce/test_metadce_minimal_Oz-ctors.jssize b/test/other/metadce/test_metadce_minimal_Oz-ctors.jssize index 39a573416a0f4..2ae555a2e692c 100644 --- a/test/other/metadce/test_metadce_minimal_Oz-ctors.jssize +++ b/test/other/metadce/test_metadce_minimal_Oz-ctors.jssize @@ -1 +1 @@ -3652 +3647 diff --git a/test/other/metadce/test_metadce_minimal_Oz.jssize b/test/other/metadce/test_metadce_minimal_Oz.jssize index 86d81b047bc4c..1149062c66bf4 100644 --- a/test/other/metadce/test_metadce_minimal_Oz.jssize +++ b/test/other/metadce/test_metadce_minimal_Oz.jssize @@ -1 +1 @@ -3671 +3666 diff --git a/test/other/test_runtime_keepalive.cpp b/test/other/test_runtime_keepalive.cpp index 005c309d59ac1..3bccbe9743484 100644 --- a/test/other/test_runtime_keepalive.cpp +++ b/test/other/test_runtime_keepalive.cpp @@ -21,7 +21,9 @@ int main() { callUserCallback(() => { out("in user callback: " + counter); }, 0); - setTimeout(timerCallback, 0); + if (!runtimeExited) { + setTimeout(timerCallback, 0); + } } setTimeout(timerCallback, 0); }); diff --git a/test/other/test_unoptimized_code_size.js.size b/test/other/test_unoptimized_code_size.js.size index dd0ff13527cfc..3e0dee581fe2d 100644 --- a/test/other/test_unoptimized_code_size.js.size +++ b/test/other/test_unoptimized_code_size.js.size @@ -1 +1 @@ -63173 +62732 diff --git a/test/other/test_unoptimized_code_size_no_asserts.js.size b/test/other/test_unoptimized_code_size_no_asserts.js.size index c6db919d89e75..289f89ae1d312 100644 --- a/test/other/test_unoptimized_code_size_no_asserts.js.size +++ b/test/other/test_unoptimized_code_size_no_asserts.js.size @@ -1 +1 @@ -38450 +37786 diff --git a/test/other/test_unoptimized_code_size_strict.js.size b/test/other/test_unoptimized_code_size_strict.js.size index ea2e77dace2a7..2992321e2821f 100644 --- a/test/other/test_unoptimized_code_size_strict.js.size +++ b/test/other/test_unoptimized_code_size_strict.js.size @@ -1 +1 @@ -62525 +62084