Skip to content

Commit

Permalink
Avoid requiring EXIT_RUNTIME when running threaded programs under node
Browse files Browse the repository at this point in the history
Prior to this change the thread calling `exit()` would itself keep
the runtime alive.

This change makes it easy to run arbitrary tests under
`PROXY_TO_PTHREAD` without also needing to remember to add
`EXIT_RUNTIME`
  • Loading branch information
sbc100 committed Aug 23, 2024
1 parent b5750f5 commit 1507300
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 58 deletions.
16 changes: 14 additions & 2 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ addToLibrary({
#endif
#if PTHREADS
'$exitOnMainThread',
'_emscripten_thread_cleanup',
#endif
#if PTHREADS_DEBUG
'$runtimeKeepaliveCounter',
Expand All @@ -111,6 +112,17 @@ addToLibrary({
// The pthread may have decided not to exit its own runtime, for example
// because it runs a main loop, but that doesn't affect the main thread.
exitOnMainThread(status);
#if !EXIT_RUNTIME
// When EXIT_RUNTIME is enabled the main thread will take care of shutting
// down the runtime, and killing all the workers/threads.
// However when EXIT_RUNTIME is no enabled we don't want the existance
// of this running thread to prevent node from exiting, do we use
// `__emscripten_thread_cleanup` to take outselves out of the list of
// running threads, but without making outselves joinable.
if (!keepRuntimeAlive()) {
__emscripten_thread_cleanup(_pthread_self())
}
#endif
throw 'unwind';
}
#if PTHREADS_DEBUG
Expand All @@ -127,7 +139,7 @@ addToLibrary({
#if ASSERTIONS
// if exit() was called explicitly, warn the user if the runtime isn't actually being shut down
if (keepRuntimeAlive() && !implicit) {
var msg = `program exited (with status: ${status}), but keepRuntimeAlive() is set (counter=${runtimeKeepaliveCounter}) due to an async operation, so halting execution but not exiting the runtime or preventing further async execution (you can use emscripten_force_exit, if you want to force a true shutdown)`;
var msg = `program exited (with status: ${status}), but keepRuntimeAlive() is set (counter=${runtimeKeepaliveCounter}), so halting execution but not exiting the runtime or preventing further async execution (you can use emscripten_force_exit, if you want to force a true shutdown)`;
#if MODULARIZE
readyPromiseReject(msg);
#endif // MODULARIZE
Expand Down Expand Up @@ -2207,7 +2219,7 @@ addToLibrary({
}
#endif
#if RUNTIME_DEBUG
dbg(`maybeExit: user callback done: runtimeKeepaliveCounter=${runtimeKeepaliveCounter}`);
dbg(`maybeExit: runtimeKeepaliveCounter=${runtimeKeepaliveCounter}`);
#endif
if (!keepRuntimeAlive()) {
#if RUNTIME_DEBUG
Expand Down
8 changes: 3 additions & 5 deletions src/library_pthread.js
Original file line number Diff line number Diff line change
Expand Up @@ -582,6 +582,7 @@ var LibraryPThread = {
#if PTHREADS_DEBUG
dbg(`cleanupThread: ${ptrToString(pthread_ptr)}`)
#endif
if (!PThread.pthreads[pthread_ptr]) return;
#if ASSERTIONS
assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! cleanupThread() can only ever be called from main application thread!');
assert(pthread_ptr, 'Internal Error! Null pthread_ptr in cleanupThread!');
Expand Down Expand Up @@ -1138,11 +1139,8 @@ var LibraryPThread = {
// emscripten_unwind_to_js_event_loop() in the pthread body.
__emscripten_thread_exit(result);
#else
if (keepRuntimeAlive()) {
EXITSTATUS = result;
} else {
__emscripten_thread_exit(result);
}
EXITSTATUS = result;
maybeExit();
#endif
}
#if ASYNCIFY == 2
Expand Down
2 changes: 1 addition & 1 deletion src/preamble.js
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ function makeAbortWrapper(original) {
|| e === Infinity // EXCEPTION_STACK_TRACES=0 will throw Infinity
#endif // EXCEPTION_STACK_TRACES
#endif
|| e === 'unwind'
|| (e instanceof ExitStatus || e == 'unwind')
) {
throw e;
}
Expand Down
2 changes: 1 addition & 1 deletion src/runtime_pthread.js
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ if (ENVIRONMENT_IS_PTHREAD) {
throw ex;
}
#if RUNTIME_DEBUG
dbg(`worker: Pthread 0x${_pthread_self().toString(16)} completed its main entry point with an 'unwind', keeping the worker alive for asynchronous operation.`);
dbg(`worker: pthread completed its main entry point with an 'unwind'`);
#endif
}
} else if (cmd === 'cancel') { // Main thread is asking for a pthread_cancel() on this thread.
Expand Down
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_minimal_pthreads.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
5103
5118
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_minimal_pthreads.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
11028
11064
2 changes: 1 addition & 1 deletion test/other/test_unoptimized_code_size.js.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
54682
54656
2 changes: 1 addition & 1 deletion test/other/test_unoptimized_code_size_strict.js.size
Original file line number Diff line number Diff line change
@@ -1 +1 @@
53633
53607
36 changes: 13 additions & 23 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ def test_hello_argc(self):
@node_pthreads
def test_hello_argc_pthreads(self):
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.do_core_test('test_hello_argc.c', args=['hello', 'world'])

@only_wasm2js('test shifts etc. on 64-bit integers')
Expand Down Expand Up @@ -1300,7 +1299,8 @@ def test_exceptions_uncaught_2(self):
def test_exceptions_typed(self):
# Depends on static destructors running
self.set_setting('EXIT_RUNTIME')
self.clear_setting('SAFE_HEAP') # Throwing null will cause an ignorable null pointer access.
if self.get_setting('SAFE_HEAP'):
self.skipTest('Throwing null will cause an ignorable null pointer access')
self.do_core_test('test_exceptions_typed.cpp')

@with_all_eh_sjlj
Expand All @@ -1318,7 +1318,8 @@ def test_exceptions_multi(self):

@with_all_eh_sjlj
def test_exceptions_std(self):
self.clear_setting('SAFE_HEAP')
if self.get_setting('SAFE_HEAP'):
self.skipTest('Fails under SAFE_HEAP')
self.do_core_test('test_exceptions_std.cpp')

@with_all_eh_sjlj
Expand Down Expand Up @@ -1871,7 +1872,7 @@ def test_em_asm_2(self):
# they will also get tested in MAIN_THREAD_EM_ASM form.
@parameterized({
'': ([],),
'pthread': (['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],),
'pthread': (['-pthread', '-sPROXY_TO_PTHREAD'],),
})
def test_main_thread_em_asm(self, args):
if args:
Expand Down Expand Up @@ -1933,8 +1934,8 @@ def test_em_asm_side_module(self):

@parameterized({
'': ([], False),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'], False),
'pthreads_dylink': (['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME', '-sMAIN_MODULE=2', '-Wno-experimental'], False),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD'], False),
'pthreads_dylink': (['-pthread', '-sPROXY_TO_PTHREAD', '-sMAIN_MODULE=2', '-Wno-experimental'], False),
'c': ([], True),
'dylink': (['-sMAIN_MODULE=2'], False),
'dylink_c': (['-sMAIN_MODULE=2'], True),
Expand Down Expand Up @@ -2500,7 +2501,6 @@ def test_pthread_proxying_canceled_work(self):
@node_pthreads
@flaky('https://github.com/emscripten-core/emscripten/issues/19795')
def test_pthread_proxying_refcount(self):
self.set_setting('EXIT_RUNTIME')
self.set_setting('PTHREAD_POOL_SIZE=1')
self.set_setting('ASSERTIONS=0')
self.do_run_in_out_file_test('pthread/test_pthread_proxying_refcount.c')
Expand All @@ -2524,7 +2524,6 @@ def test_pthread_nested_work_queue(self):
@node_pthreads
def test_pthread_thread_local_storage(self):
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
if not self.has_changed_setting('INITIAL_MEMORY'):
self.set_setting('INITIAL_MEMORY', '300mb')
self.do_run_in_out_file_test('pthread/test_pthread_thread_local_storage.cpp')
Expand Down Expand Up @@ -2574,7 +2573,6 @@ def test_pthread_abort_interrupt(self):
def test_pthread_emmalloc(self):
self.emcc_args += ['-fno-builtin']
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.set_setting('ASSERTIONS', 2)
self.set_setting('MALLOC', 'emmalloc')
self.do_core_test('test_emmalloc.c')
Expand All @@ -2589,7 +2587,6 @@ def test_pthread_stdout_after_main(self):
@node_pthreads
def test_pthread_proxy_to_pthread(self):
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.do_run_in_out_file_test('pthread/test_pthread_proxy_to_pthread.c')

@node_pthreads
Expand All @@ -2606,7 +2603,6 @@ def test_pthread_run_script(self):
# Run the test again with PROXY_TO_PTHREAD
self.setup_node_pthreads()
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.do_runf('pthread/test_pthread_run_script.c')

@node_pthreads
Expand Down Expand Up @@ -2903,7 +2899,7 @@ def test_dlfcn_missing(self):
@needs_dylink
@parameterized({
'': ([],),
'pthreads': (['-pthread', '-sEXIT_RUNTIME', '-sPROXY_TO_PTHREAD', '-Wno-experimental'],),
'pthreads': (['-pthread', '-sPROXY_TO_PTHREAD', '-Wno-experimental'],),
})
def test_dlfcn_basic(self, args):
if args:
Expand Down Expand Up @@ -6470,7 +6466,6 @@ def test_mmap_anon_pthreads(self):
# Same test with threading enabled so give is some basic sanity
# checks of the locking on the internal data structures.
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
if not self.has_changed_setting('INITIAL_MEMORY'):
self.set_setting('INITIAL_MEMORY', '64mb')
self.do_core_test('test_mmap_anon.c')
Expand Down Expand Up @@ -7363,7 +7358,7 @@ def test_embind_2(self):
self.emcc_args += [
'-lembind', '--post-js', 'post.js',
# for extra coverage, test using pthreads
'-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'
'-pthread', '-sPROXY_TO_PTHREAD'
]
create_file('post.js', '''
function printLerp() {
Expand Down Expand Up @@ -8313,7 +8308,6 @@ def test_pthread_join_and_asyncify(self):
self.require_jspi()
self.do_runf('core/test_pthread_join_and_asyncify.c', 'joining thread!\njoined thread!',
emcc_args=['-sJSPI_EXPORTS=run_thread',
'-sEXIT_RUNTIME=1',
'-pthread', '-sPROXY_TO_PTHREAD'])

@no_asan('asyncify stack operations confuse asan')
Expand Down Expand Up @@ -9118,7 +9112,7 @@ def test_pthread_create(self):
@parameterized({
'': ([],),
'pooled': (['-sPTHREAD_POOL_SIZE=1'],),
'proxied': (['-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'],),
'proxied': (['-sPROXY_TO_PTHREAD'],),
})
def test_pthread_c11_threads(self, args):
self.emcc_args += args
Expand Down Expand Up @@ -9164,7 +9158,6 @@ def test_pthread_create_pool(self):
def test_pthread_create_proxy(self):
# with PROXY_TO_PTHREAD, we can synchronously depend on workers being available
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.emcc_args += ['-DALLOW_SYNC']
self.do_run_in_out_file_test('core/pthread/create.c')

Expand Down Expand Up @@ -9215,7 +9208,6 @@ def test_pthread_unhandledrejection(self):
@no_wasm2js('wasm2js does not support PROXY_TO_PTHREAD (custom section support)')
def test_pthread_offset_converter(self):
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.set_setting('USE_OFFSET_CONVERTER')
if '-g' in self.emcc_args:
self.emcc_args += ['-DDEBUG']
Expand All @@ -9225,7 +9217,6 @@ def test_pthread_offset_converter(self):
@no_wasm2js('wasm2js does not support PROXY_TO_PTHREAD (custom section support)')
def test_pthread_offset_converter_modularize(self):
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.set_setting('USE_OFFSET_CONVERTER')
self.set_setting('MODULARIZE')
self.set_setting('EXPORT_NAME', 'foo')
Expand Down Expand Up @@ -9259,7 +9250,6 @@ def test_stdio_locking(self):
def test_pthread_dylink_basics(self):
self.emcc_args.append('-Wno-experimental')
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.do_basic_dylink_test()

@needs_dylink
Expand Down Expand Up @@ -9302,6 +9292,7 @@ def test_pthread_dlopen(self):

self.emcc_args += ['--embed-file', '[email protected]']
self.prep_dlfcn_main()
# Test depends on atexit()
self.set_setting('EXIT_RUNTIME')
self.set_setting('PROXY_TO_PTHREAD')
self.do_runf('core/pthread/test_pthread_dlopen.c',
Expand All @@ -9321,6 +9312,7 @@ def test_pthread_dlopen_many(self):
shutil.copyfile('liblib.so', f'liblib{i}.so')

self.prep_dlfcn_main()
# Test depends on atexit()
self.set_setting('EXIT_RUNTIME')
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('DEFAULT_LIBRARY_FUNCS_TO_INCLUDE', 'jslib_func')
Expand All @@ -9343,7 +9335,6 @@ def test_pthread_dlsym(self):
self.build_dlfcn_lib(test_file('core/pthread/test_pthread_dlsym_side.c'))

self.prep_dlfcn_main()
self.set_setting('EXIT_RUNTIME')
self.set_setting('PROXY_TO_PTHREAD')
self.do_runf('core/pthread/test_pthread_dlsym.c')

Expand All @@ -9370,7 +9361,7 @@ def test_pthread_dylink_main_module_1(self):

@parameterized({
'': ([],),
'pthreads': (['-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME', '-pthread', '-Wno-experimental'],)
'pthreads': (['-sPROXY_TO_PTHREAD', '-pthread', '-Wno-experimental'],)
})
@with_dylink_reversed
def test_Module_dynamicLibraries(self, args):
Expand Down Expand Up @@ -9532,7 +9523,6 @@ def test_abort_on_exceptions_main(self):
def test_abort_on_exceptions_pthreads(self):
self.set_setting('ABORT_ON_WASM_EXCEPTIONS')
self.set_setting('PROXY_TO_PTHREAD')
self.set_setting('EXIT_RUNTIME')
self.do_core_test('test_hello_world.c')

@needs_dylink
Expand Down
Loading

0 comments on commit 1507300

Please sign in to comment.