Skip to content

Commit 34ce2af

Browse files
authored
Fix for failure to reset runtimeKeepaliveCounter (#21526)
Failure to reset this counter means that we can leak workers under the following circumstances: 1. Thread A calls exit_with_live_runtime causing to to have its keelalive counter set. 2. Thread A is cancelled via pthead_cancel, returning the worker to the pool. 3. Thread B inherits the worker from the pool, with the keelalive counter still in a non-zero state. 4. Thread B exits normally, but is kept alive unnecessarily (it also cannot be join()'d). Each time steps 1-4 is repeated we effectively leak the worker since it can never be re-used.
1 parent cf90417 commit 34ce2af

File tree

5 files changed

+78
-7
lines changed

5 files changed

+78
-7
lines changed

src/library.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3332,11 +3332,7 @@ addToLibrary({
33323332
}
33333333
}
33343334
#endif
3335-
#if MINIMAL_RUNTIME
3336-
throw e;
3337-
#else
33383335
quit_(1, e);
3339-
#endif
33403336
},
33413337

33423338
$runtimeKeepaliveCounter__internal: true,

src/library_pthread.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1101,16 +1101,14 @@ var LibraryPThread = {
11011101
'_emscripten_thread_exit',
11021102
#if !MINIMAL_RUNTIME
11031103
'$keepRuntimeAlive',
1104-
#endif
1105-
#if EXIT_RUNTIME && !MINIMAL_RUNTIME
11061104
'$runtimeKeepaliveCounter',
11071105
#endif
11081106
],
11091107
$invokeEntryPoint: (ptr, arg) => {
11101108
#if PTHREADS_DEBUG
11111109
dbg(`invokeEntryPoint: ${ptrToString(ptr)}`);
11121110
#endif
1113-
#if EXIT_RUNTIME && !MINIMAL_RUNTIME
1111+
#if !MINIMAL_RUNTIME
11141112
// An old thread on this worker may have been canceled without returning the
11151113
// `runtimeKeepaliveCounter` to zero. Reset it now so the new thread won't
11161114
// be affected.

test/other/test_pthread_reuse.c

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#include <emscripten/emscripten.h>
2+
#include <emscripten/eventloop.h>
3+
#include <emscripten/console.h>
4+
5+
#include <assert.h>
6+
#include <pthread.h>
7+
#include <stdio.h>
8+
9+
_Atomic int ready;
10+
11+
void markReady(void* arg) {
12+
ready = 1;
13+
}
14+
15+
void* thread_main(void* arg) {
16+
int keepalive = (intptr_t)arg;
17+
emscripten_outf("in thread_main: %d", keepalive);
18+
if (keepalive) {
19+
// Exit with live runtime. Once we are in the event loop call markReady.
20+
emscripten_async_call(markReady, NULL, 0);
21+
emscripten_exit_with_live_runtime();
22+
}
23+
return NULL;
24+
}
25+
26+
void checkThreadPool() {
27+
int running = EM_ASM_INT(return PThread.runningWorkers.length);
28+
int unused = EM_ASM_INT(return PThread.unusedWorkers.length);
29+
printf("running=%d unused=%d\n", running, unused);
30+
assert(running == 0);
31+
assert(unused == 1);
32+
}
33+
34+
int main() {
35+
printf("in main\n");
36+
checkThreadPool();
37+
pthread_t t, t2;
38+
39+
for (int i = 0; i < 3; i++) {
40+
ready = 0;
41+
42+
// Create a thread that exit's with a non-zero keepalive counter
43+
pthread_create(&t, NULL, thread_main, (void*)1);
44+
while (!ready) {}
45+
pthread_cancel(t);
46+
pthread_join(t, NULL);
47+
checkThreadPool();
48+
49+
// Create a second thread that should re-use the same worker.
50+
// This thread should reset its keepalive counter on startup and
51+
// be able to exit normally.
52+
pthread_create(&t2, NULL, thread_main, NULL);
53+
pthread_join(t2, NULL);
54+
checkThreadPool();
55+
}
56+
printf("done\n");
57+
}

test/other/test_pthread_reuse.out

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
in main
2+
running=0 unused=1
3+
in thread_main: 1
4+
running=0 unused=1
5+
in thread_main: 0
6+
running=0 unused=1
7+
in thread_main: 1
8+
running=0 unused=1
9+
in thread_main: 0
10+
running=0 unused=1
11+
in thread_main: 1
12+
running=0 unused=1
13+
in thread_main: 0
14+
running=0 unused=1
15+
done

test/test_other.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11803,6 +11803,11 @@ def test_pthread_asyncify(self):
1180311803
self.set_setting('EXIT_RUNTIME')
1180411804
self.do_run_in_out_file_test('other/test_pthread_asyncify.c')
1180511805

11806+
@node_pthreads
11807+
def test_pthread_reuse(self):
11808+
self.set_setting('PTHREAD_POOL_SIZE', 1)
11809+
self.do_run_in_out_file_test('other/test_pthread_reuse.c')
11810+
1180611811
def test_stdin_preprocess(self):
1180711812
create_file('temp.h', '#include <string>')
1180811813
outputStdin = self.run_process([EMCC, '-x', 'c++', '-dM', '-E', '-'], input="#include <string>", stdout=PIPE).stdout

0 commit comments

Comments
 (0)