Skip to content

Commit fa339b7

Browse files
authored
Avoid calling process.exit under node (#18754)
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 a code size saving.
1 parent 72f205f commit fa339b7

38 files changed

+70
-76
lines changed

emscripten.py

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,22 @@ def create_sending(metadata, library_symbols):
716716

717717
def make_export_wrappers(exports, delay_assignment):
718718
wrappers = []
719+
720+
# The emscripten stack functions are called very early (by writeStackCookie) before
721+
# the runtime is initialized so we can't create these wrappers that check for
722+
# runtimeInitialized.
723+
# Likewise `__trap` can occur before the runtime is initialized since it is used in
724+
# abort.
725+
# pthread_self and _emscripten_proxy_execute_task_queue are currently called in some
726+
# cases after the runtime has exited.
727+
# TODO: Look into removing these, and improving our robustness around thread termination.
728+
def install_wrapper(sym):
729+
if sym.startswith('_asan_') or sym.startswith('emscripten_stack_'):
730+
return False
731+
if sym in ('__trap', 'pthread_self', '_emscripten_proxy_execute_task_queue'):
732+
return False
733+
return True
734+
719735
for name in exports:
720736
# Tags cannot be wrapped in createExportWrapper
721737
if name == '__cpp_exception':
@@ -730,12 +746,7 @@ def make_export_wrappers(exports, delay_assignment):
730746
exported = ''
731747
wrapper += exported
732748

733-
# The emscripten stack functions are called very early (by writeStackCookie) before
734-
# the runtime is initialized so we can't create these wrappers that check for
735-
# runtimeInitialized.
736-
# Likewise `__trap` can occur before the runtime is initialized since it is used in
737-
# abort.
738-
if settings.ASSERTIONS and not name.startswith('emscripten_stack_') and name != '__trap':
749+
if settings.ASSERTIONS and install_wrapper(name):
739750
# With assertions enabled we create a wrapper that are calls get routed through, for
740751
# the lifetime of the program.
741752
if delay_assignment:

src/library_pthread.js

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,9 @@ var LibraryPThread = {
526526
},
527527

528528
$terminateWorker: function(worker) {
529+
#if PTHREADS_DEBUG
530+
dbg('terminateWorker: ' + worker.workerID);
531+
#endif
529532
worker.terminate();
530533
// terminate() can be asynchronous, so in theory the worker can continue
531534
// to run for some amount of time after termination. However from our POV
@@ -942,15 +945,7 @@ var LibraryPThread = {
942945
#if PROXY_TO_PTHREAD
943946
{{{ runtimeKeepalivePop() }}};
944947
#endif
945-
#if MINIMAL_RUNTIME
946948
_exit(returnCode);
947-
#else
948-
try {
949-
_exit(returnCode);
950-
} catch (e) {
951-
handleException(e);
952-
}
953-
#endif
954949
},
955950

956951
emscripten_proxy_to_main_thread_js__deps: ['$withStackSave', '_emscripten_run_in_main_runtime_thread_js'],

src/shell.js

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -172,24 +172,6 @@ var read_,
172172
readBinary,
173173
setWindowTitle;
174174

175-
#if ENVIRONMENT_MAY_BE_SHELL || ENVIRONMENT_MAY_BE_NODE || ASSERTIONS
176-
// Normally we don't log exceptions but instead let them bubble out the top
177-
// level where the embedding environment (e.g. the browser) can handle
178-
// them.
179-
// However under v8 and node we sometimes exit the process direcly in which case
180-
// its up to use us to log the exception before exiting.
181-
// If we fix https://github.com/emscripten-core/emscripten/issues/15080
182-
// this may no longer be needed under node.
183-
function logExceptionOnExit(e) {
184-
if (e instanceof ExitStatus) return;
185-
let toLog = e;
186-
if (e && typeof e == 'object' && e.stack) {
187-
toLog = [e, e.stack];
188-
}
189-
err('exiting due to exception: ' + toLog);
190-
}
191-
#endif
192-
193175
#if ENVIRONMENT_MAY_BE_NODE
194176
if (ENVIRONMENT_IS_NODE) {
195177
#if ENVIRONMENT && ASSERTIONS
@@ -244,7 +226,7 @@ if (ENVIRONMENT_IS_NODE) {
244226
#if RUNTIME_DEBUG
245227
dbg('node: uncaughtException: ' + ex)
246228
#endif
247-
if (!(ex instanceof ExitStatus)) {
229+
if (ex !== 'unwind' && !(ex instanceof ExitStatus) && !(ex.context instanceof ExitStatus)) {
248230
throw ex;
249231
}
250232
});
@@ -263,12 +245,8 @@ if (ENVIRONMENT_IS_NODE) {
263245
#endif
264246

265247
quit_ = (status, toThrow) => {
266-
if (keepRuntimeAlive()) {
267-
process.exitCode = status;
268-
throw toThrow;
269-
}
270-
logExceptionOnExit(toThrow);
271-
process.exit(status);
248+
process.exitCode = status;
249+
throw toThrow;
272250
};
273251

274252
Module['inspect'] = function () { return '[Emscripten Module object]'; };
@@ -360,7 +338,13 @@ if (ENVIRONMENT_IS_SHELL) {
360338
throw toThrow;
361339
}
362340
#endif
363-
logExceptionOnExit(toThrow);
341+
if (!(toThrow instanceof ExitStatus)) {
342+
let toLog = toThrow;
343+
if (toThrow && typeof toThrow == 'object' && toThrow.stack) {
344+
toLog = [toThrow, toThrow.stack];
345+
}
346+
err('exiting due to exception: ' + toLog);
347+
}
364348
quit(status);
365349
};
366350
}

src/threadprofiler.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ var emscriptenThreadProfiler = {
2020
document.body.appendChild(div);
2121
this.threadProfilerDiv = document.getElementById('threadprofiler');
2222
}
23-
setInterval(function() { emscriptenThreadProfiler.updateUi() }, this.uiUpdateIntervalMsecs);
23+
var i = setInterval(function() { emscriptenThreadProfiler.updateUi() }, this.uiUpdateIntervalMsecs);
24+
addOnExit(() => clearInterval(i));
2425
},
2526

2627
initializeNode: function initializeNode() {
2728
addOnInit(() => {
2829
emscriptenThreadProfiler.dumpState();
29-
setInterval(function() { emscriptenThreadProfiler.dumpState() }, this.uiUpdateIntervalMsecs);
30+
var i = setInterval(function() { emscriptenThreadProfiler.dumpState() }, this.uiUpdateIntervalMsecs);
31+
addOnExit(() => clearInterval(i));
3032
});
3133
},
3234

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
26171
1+
26046
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
26135
1+
26010
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
30708
1+
30583
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
25850
1+
25725
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
30708
1+
30583
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
26171
1+
26046

0 commit comments

Comments
 (0)