Skip to content

Commit 20dd010

Browse files
authored
Sequentialize uncaught exceptions in Node.js Workers (#25041)
Sequentialize uncaught exceptions in Node.js Workers into the same postMessage() stream that other messages go through, to ensure they are received by the main thread in the order they were generated. Fixes #15014. See nodejs/node#59617.
1 parent 3d49dd5 commit 20dd010

File tree

5 files changed

+41
-11
lines changed

5 files changed

+41
-11
lines changed

src/lib/libpthread.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,9 @@ var LibraryPThread = {
241241
worker.onmessage = (e) => {
242242
var d = e['data'];
243243
var cmd = d.cmd;
244+
#if PTHREADS_DEBUG
245+
dbg(`main thread: received message '${cmd}' from worker. ${d}`);
246+
#endif
244247

245248
// If this message is intended to a recipient that is not the main
246249
// thread, forward it to the target thread.
@@ -283,6 +286,13 @@ var LibraryPThread = {
283286
// Worker wants to postMessage() to itself to implement setImmediate()
284287
// emulation.
285288
worker.postMessage(d);
289+
#if ENVIRONMENT_MAY_BE_NODE
290+
} else if (cmd === 'uncaughtException') {
291+
// Message handler for Node.js specific out-of-order behavior:
292+
// https://github.com/nodejs/node/issues/59617
293+
// A pthread sent an uncaught exception event. Re-raise it on the main thread.
294+
worker.onerror(d.error);
295+
#endif
286296
} else if (cmd === 'callHandler') {
287297
Module[d.handler](...d.args);
288298
} else if (cmd) {
@@ -308,6 +318,13 @@ var LibraryPThread = {
308318
if (ENVIRONMENT_IS_NODE) {
309319
worker.on('message', (data) => worker.onmessage({ data: data }));
310320
worker.on('error', (e) => worker.onerror(e));
321+
322+
#if PTHREADS_DEBUG
323+
worker.on('exit', (code) => {
324+
if (worker.pthread_ptr) dbg(`Worker hosting pthread ${ptrToString(worker.pthread_ptr)} has terminated with code ${code}.`);
325+
else dbg(`Worker has terminated with code ${code}.`);
326+
});
327+
#endif
311328
}
312329
#endif
313330

src/runtime_common.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,21 @@ if (ENVIRONMENT_IS_NODE && {{{ ENVIRONMENT_IS_WORKER_THREAD() }}}) {
4040
self: global,
4141
postMessage: (msg) => parentPort['postMessage'](msg),
4242
});
43+
// Node.js Workers do not pass postMessage()s and uncaught exception events to the parent
44+
// thread necessarily in the same order where they were generated in sequential program order.
45+
// See https://github.com/nodejs/node/issues/59617
46+
// To remedy this, capture all uncaughtExceptions in the Worker, and sequentialize those over
47+
// to the same postMessage pipe that other messages use.
48+
process.on("uncaughtException", (err) => {
49+
#if PTHREADS_DEBUG
50+
dbg(`uncaughtException on worker thread: ${err.message}`);
51+
#endif
52+
postMessage({ cmd: 'uncaughtException', error: err });
53+
// Also shut down the Worker to match the same semantics as if this uncaughtException
54+
// handler was not registered.
55+
// (n.b. this will not shut down the whole Node.js app process, but just the Worker)
56+
process.exit(1);
57+
});
4358
}
4459
#endif // (PTHREADS || WASM_WORKERS) && (ENVIRONMENT_MAY_BE_NODE && !WASM_ESM_INTEGRATION)
4560

test/code_size/test_codesize_minimal_pthreads.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"a.out.js": 7499,
3-
"a.out.js.gz": 3721,
2+
"a.out.js": 7649,
3+
"a.out.js.gz": 3768,
44
"a.out.nodebug.wasm": 19588,
55
"a.out.nodebug.wasm.gz": 9025,
6-
"total": 27087,
7-
"total_gz": 12746,
6+
"total": 27237,
7+
"total_gz": 12793,
88
"sent": [
99
"a (memory)",
1010
"b (emscripten_get_now)",

test/code_size/test_codesize_minimal_pthreads_memgrowth.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"a.out.js": 7926,
3-
"a.out.js.gz": 3924,
2+
"a.out.js": 8076,
3+
"a.out.js.gz": 3974,
44
"a.out.nodebug.wasm": 19589,
55
"a.out.nodebug.wasm.gz": 9025,
6-
"total": 27515,
7-
"total_gz": 12949,
6+
"total": 27665,
7+
"total_gz": 12999,
88
"sent": [
99
"a (memory)",
1010
"b (emscripten_get_now)",

test/test_core.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
from tools import shared, building, config, utils, webassembly
2525
import common
2626
from common import RunnerCore, path_from_root, requires_native_clang, test_file, create_file
27-
from common import skip_if, no_windows, no_mac, is_slow_test, parameterized, parameterize
27+
from common import skip_if, no_windows, is_slow_test, parameterized, parameterize
2828
from common import env_modify, with_env_modify, disabled, flaky, node_pthreads, also_without_bigint
2929
from common import read_file, read_binary, requires_v8, requires_node, requires_dev_dependency, requires_wasm2js, requires_node_canary
3030
from common import compiler_for, crossplatform, no_4gb, no_2gb, also_with_minimal_runtime, also_with_modularize
@@ -2669,8 +2669,6 @@ def test_pthread_attr_getstack(self):
26692669
self.do_run_in_out_file_test('pthread/test_pthread_attr_getstack.c')
26702670

26712671
@node_pthreads
2672-
@no_mac('https://github.com/emscripten-core/emscripten/issues/15014')
2673-
@flaky('https://github.com/emscripten-core/emscripten/issues/15014')
26742672
def test_pthread_abort(self):
26752673
self.set_setting('PROXY_TO_PTHREAD')
26762674
# Add the onAbort handler at runtime during preRun. This means that onAbort

0 commit comments

Comments
 (0)