Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/lib/libcore.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ addToLibrary({
#if PTHREADS
'$exitOnMainThread',
#endif
#if PROXY_TO_PTHREAD
'$proxiedMainDone',
#endif
#if PTHREADS_DEBUG || ASSERTIONS
'$runtimeKeepaliveCounter',
#endif
Expand All @@ -132,6 +135,10 @@ addToLibrary({
#endif
#if PTHREADS_DEBUG
dbg(`Pthread ${ptrToString(_pthread_self())} called exit(${status}), posting exitOnMainThread.`);
#endif
#if PROXY_TO_PTHREAD
// Forget a waiting main return.
proxiedMainDone = false;
#endif
// When running in a pthread we propagate the exit back to the main thread
// where it can decide if the whole process should be shut down or not.
Expand Down Expand Up @@ -2169,6 +2176,10 @@ addToLibrary({
#if PTHREADS
'_emscripten_thread_exit',
#endif
#if PROXY_TO_PTHREAD
'$proxiedMainDone',
'$exitOnMainThread',
#endif
#if RUNTIME_DEBUG >= 2
'$runtimeKeepaliveCounter',
#endif
Expand All @@ -2192,6 +2203,14 @@ addToLibrary({
// exit the current thread, but only if there is one active.
// TODO(https://github.com/emscripten-core/emscripten/issues/25076):
// Unify this check with the runtimeExited check above
#if PROXY_TO_PTHREAD && EXIT_RUNTIME
// Run a waiting main return once.
if (proxiedMainDone) {
proxiedMainDone = false;
exitOnMainThread(EXITSTATUS);
return;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this return needed? i.e. can we just rely on the return on line 2216?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so; falling through calls __emscripten_thread_exit(), which tries to post a message to the main thread right as the worker is being terminated, causing a received "6" command from terminated worker error in assertion builds.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we replace proxiedMainDone with isProxiedMainThread? This could be simple boolean what is true only on the proxied main thread.

This could then just be if (proxiedMainDone) exitOnMainThread(EXITSTATUS);

I don't think we need to track whether or not the main is done because if we get to this locaiton in the code its is by definition done.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, because if this I think we can maybe remove the exit(rtn) call from _main_thread, and just always rely on this maybeExit path to handle calling exit?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better still, maybe we can replace the if (ENVIRONMENT_IS_PTHREAD) { just above with if (ENVIRONMENT_IS_PTHREAD && !isProxiedMainThread) {`` .. then the existing (_exit`) below should work.

}
#endif
if (_pthread_self()) __emscripten_thread_exit(EXITSTATUS);
return;
}
Expand Down
18 changes: 18 additions & 0 deletions src/lib/libpthread.js
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,17 @@ var LibraryPThread = {
_exit(returnCode);
},

#if PROXY_TO_PTHREAD
// Main's return, saved for maybeExit.
$proxiedMainDone__internal: true,
$proxiedMainDone: false,

__emscripten_proxied_main_done__deps: ['$proxiedMainDone'],
__emscripten_proxied_main_done: () => {
proxiedMainDone = true;
},
#endif

#if MEMORY64
// Calls proxyToMainThread but returns a bigint rather than a number
$proxyToMainThreadPtr__deps: ['$proxyToMainThread'],
Expand Down Expand Up @@ -1144,6 +1155,9 @@ var LibraryPThread = {
#if !MINIMAL_RUNTIME
'$keepRuntimeAlive',
'$runtimeKeepaliveCounter',
#endif
#if PROXY_TO_PTHREAD
'$proxiedMainDone',
#endif
],
$invokeEntryPoint: {{{ asyncIf(ASYNCIFY == 2) }}}(ptr, arg) => {
Expand All @@ -1166,6 +1180,10 @@ var LibraryPThread = {
noExitRuntime = 0;
#endif
#endif
#if PROXY_TO_PTHREAD
// No main return waiting yet.
proxiedMainDone = false;
#endif

#if MAIN_MODULE
// Before we call the thread entry point, make sure any shared libraries
Expand Down
4 changes: 3 additions & 1 deletion system/lib/libc/crt1_proxy_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ static void* _main_thread(void* param) {
if (!emscripten_runtime_keepalive_check()) {
exit(rtn);
}
return NULL;
// Wait for keepalives, then exit with main's status.
__emscripten_proxied_main_done();
return (void*)(intptr_t)rtn;
}

EMSCRIPTEN_KEEPALIVE int _emscripten_proxy_main(int argc, char** argv) {
Expand Down
2 changes: 2 additions & 0 deletions system/lib/pthread/threading_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ void _emscripten_init_main_thread_js(void* tb);
void _emscripten_thread_profiler_enable();
void _emscripten_thread_cleanup(pthread_t thread);

void __emscripten_proxied_main_done(void);

hidden void* _emscripten_tls_init(void);
hidden void _emscripten_tls_free(void);

Expand Down
47 changes: 47 additions & 0 deletions test/other/test_proxied_main_keepalive_exit.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Proxied main must exit with its status after keepalives run.
#include <emscripten.h>
#include <emscripten/eventloop.h>
#include <stdio.h>
#include <stdlib.h>

void at_exit(void) { printf("done\n"); }

#ifdef MODE_CLEARED
int long_id;

void never(void* arg) {
printf("cleared callback ran\n");
abort();
}
#endif

void fired(void* arg) {
printf("fired\n");
#ifdef MODE_CLEARED
emscripten_clear_timeout(long_id);
#elif defined(MODE_FORCE_EXIT)
// exit() beats a waiting return.
emscripten_force_exit(7);
#elif defined(MODE_EXIT)
exit(7);
#endif
}

int main(void) {
#ifdef MODE_NEGATIVE
// Check the value, not just the code.
MAIN_THREAD_EM_ASM({ Module['onExit'] = (c) => { out('exited:' + c); }; });
#else
MAIN_THREAD_EM_ASM({ Module['onExit'] = () => { out('exited'); }; });
#endif
atexit(at_exit);
#ifdef MODE_CLEARED
long_id = emscripten_set_timeout(never, 10000, NULL);
#endif
emscripten_set_timeout(fired, 10, NULL);
#ifdef MODE_NEGATIVE
return -1;
#else
return 3;
#endif
}
15 changes: 15 additions & 0 deletions test/test_other.py
Original file line number Diff line number Diff line change
Expand Up @@ -11603,6 +11603,21 @@ def test_proxy_to_pthread_stack(self):
'-sSTACK_SIZE=128kb', '-sEXIT_RUNTIME',
'--profiling-funcs'])

@requires_pthreads
@parameterized({
'': ([], 3, 'fired\ndone\nexited\n'),
'cleared': (['-DMODE_CLEARED'], 3, 'fired\ndone\nexited\n'),
'force_exit': (['-DMODE_FORCE_EXIT'], 7, 'fired\ndone\nexited\n'),
'exit': (['-DMODE_EXIT'], 7, 'fired\ndone\nexited\n'),
'negative': (['-DMODE_NEGATIVE'], NON_ZERO, 'fired\ndone\nexited:-1\n'),
})
def test_proxied_main_keepalive_exit(self, cflags, returncode, expected):
# See https://github.com/emscripten-core/emscripten/issues/27721
# Proxied main that returns with a keepalive must exit with its status.
self.do_runf('other/test_proxied_main_keepalive_exit.c', expected,
cflags=['-pthread', '-sPROXY_TO_PTHREAD', '-sEXIT_RUNTIME'] + cflags,
assert_returncode=returncode)

@crossplatform
@no_windows('ptys and select are not available on windows')
def test_color_diagnostics(self):
Expand Down