Skip to content

Commit e20a4dd

Browse files
committed
[pthread] Don't let a stale "finished" message mark a reused thread in dlopen sync
With dynamic linking, a joinable thread posts CMD_MARK_AS_FINISHED to the main thread as it exits, so that dlopen()'s sync skips it. A join cleans the thread up as soon as it returns, possibly before that message is handled, and a new thread created in the meantime can get the same pthread_t. The late message then marked the new, running thread as finished, so every later dlopen() skipped it and its calls into newly loaded modules trapped. Only mark a thread if it is still known and has actually exited. Fixes: #27841
1 parent f9ce3a1 commit e20a4dd

6 files changed

Lines changed: 96 additions & 1 deletion

File tree

‎src/lib/libpthread.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ var LibraryPThread = {
8686
'$addOnPreRun',
8787
#if MAIN_MODULE
8888
'$markAsFinished',
89+
'_emscripten_thread_has_exited',
8990
#endif
9091
#if !MINIMAL_RUNTIME && PTHREAD_POOL_SIZE && !PTHREAD_POOL_DELAY_LOAD
9192
'$addRunDependency',
@@ -323,7 +324,12 @@ var LibraryPThread = {
323324
break;
324325
#if MAIN_MODULE
325326
case {{{ CMD_MARK_AS_FINISHED }}}:
326-
markAsFinished(d.thread);
327+
// By the time this arrives the thread may have been joined and
328+
// cleaned up, and its pthread_t reused by a new thread that is
329+
// still running, which must not be marked.
330+
if (PThread.pthreads[d.thread] && __emscripten_thread_has_exited(d.thread)) {
331+
markAsFinished(d.thread);
332+
}
327333
break;
328334
#endif
329335
case {{{ CMD_LOADED }}}:

‎system/lib/pthread/library_pthread.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,11 @@ int _emscripten_thread_is_valid(pthread_t thread) {
104104
return thread->self == thread;
105105
}
106106

107+
int _emscripten_thread_has_exited(pthread_t thread) {
108+
int state = thread->detach_state;
109+
return state == DT_EXITING || state == DT_EXITED;
110+
}
111+
107112
static void *dummy_tsd[1] = { 0 };
108113
weak_alias(dummy_tsd, __pthread_tsd_main);
109114

‎system/lib/pthread/threading_internal.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,9 @@ void _emscripten_thread_set_strongref(pthread_t thread);
7979
// we can pass more of the posixtest suite than vanilla musl.
8080
int _emscripten_thread_is_valid(pthread_t thread);
8181

82+
// Whether a thread that hasn't been cleaned up yet has run to its end.
83+
int _emscripten_thread_has_exited(pthread_t thread);
84+
8285
void _emscripten_thread_exit_joinable(pthread_t thread);
8386
void _emscripten_thread_exit(void* result);
8487
void _emscripten_process_dlopen_queue(void);
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
// A joinable thread's "finished" message only reaches the main thread once it
2+
// returns to the event loop. By then the main thread has joined that thread
3+
// and created a new one that reuses its pthread_t. The late message must not
4+
// stop the new, still running thread from taking part in dlopen()'s sync.
5+
6+
#include <assert.h>
7+
#include <dlfcn.h>
8+
#include <emscripten/eventloop.h>
9+
#include <emscripten/threading.h>
10+
#include <limits.h>
11+
#include <math.h>
12+
#include <pthread.h>
13+
#include <stdio.h>
14+
#include <stdlib.h>
15+
16+
typedef int (*func_t)();
17+
18+
static pthread_t waiter;
19+
static _Atomic int go;
20+
static func_t (*side_func_address)();
21+
static int result;
22+
23+
static void* noop(void* arg) { return NULL; }
24+
25+
static void* wait_then_call(void* arg) {
26+
while (!go) {
27+
emscripten_futex_wait(&go, 0, INFINITY);
28+
}
29+
if (side_func_address) {
30+
result = side_func_address()();
31+
}
32+
return NULL;
33+
}
34+
35+
static void release_waiter() {
36+
go = 1;
37+
emscripten_futex_wake(&go, INT_MAX);
38+
pthread_join(waiter, NULL);
39+
go = 0;
40+
}
41+
42+
static void load_and_call(void* arg) {
43+
void* handle = dlopen("libside.so", RTLD_NOW);
44+
assert(handle);
45+
side_func_address = dlsym(handle, "side_func_address");
46+
assert(side_func_address);
47+
// The waiter traps here if dlopen() skipped it.
48+
release_waiter();
49+
assert(result == 43);
50+
printf("done\n");
51+
exit(0);
52+
}
53+
54+
int main() {
55+
// The new thread almost always gets the joined thread's block back, but
56+
// retry rather than depend on it.
57+
for (int i = 0; i < 10; i++) {
58+
pthread_t joined;
59+
pthread_create(&joined, NULL, noop, NULL);
60+
pthread_join(joined, NULL);
61+
pthread_create(&waiter, NULL, wait_then_call, NULL);
62+
if (pthread_equal(waiter, joined)) {
63+
emscripten_set_timeout(load_and_call, 100, NULL);
64+
return 0;
65+
}
66+
release_waiter();
67+
}
68+
printf("no thread reused a joined thread's pthread_t\n");
69+
return 1;
70+
}

‎test/test_core.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9403,6 +9403,16 @@ def test_pthread_dlopen(self, args):
94039403
['side module ctor', 'done\n', 'side module atexit'],
94049404
assert_all=True, cflags=args)
94059405

9406+
@needs_dylink
9407+
@requires_pthreads
9408+
def test_pthread_dlsync_reused(self):
9409+
self.cflags += ['-Wno-experimental', '-pthread']
9410+
self.build_dlfcn_lib(test_file('core/pthread/test_pthread_dlopen_side.c'))
9411+
self.cflags += ['--embed-file', 'libside.so@libside.so']
9412+
self.prep_dlfcn_main()
9413+
self.set_setting('EXIT_RUNTIME')
9414+
self.do_runf('core/pthread/test_pthread_dlsync_reused.c', 'done\n')
9415+
94069416
@needs_dylink
94079417
@requires_pthreads
94089418
@flaky('https://github.com/emscripten-core/emscripten/issues/18887')

‎tools/native_sigs.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,7 @@
540540
'_emscripten_stack_restore': '_p',
541541
'_emscripten_thread_exit': '_p',
542542
'_emscripten_thread_free_data': '_p',
543+
'_emscripten_thread_has_exited': '_p',
543544
'_emscripten_thread_init': '_p_____',
544545
'_emscripten_thread_is_valid': '_p',
545546
'_emscripten_thread_mailbox_init': '_p',

0 commit comments

Comments
 (0)