Skip to content

Commit 2b1dc86

Browse files
authored
[pthreads] Fix memory leak in mailbox await loop during shutdown (#26692)
Introduce PThread.terminateRuntime which is called during runtime exit. This function not only terminates all workers but also clears the thread state of the main thread and sends a final notification to its mailbox. By clearing the thread state, _pthread_self() will now return 0, which causes the mailbox checking loop to terminate instead of re-registering a new Atomics.waitAsync promise. This allows the Wasm memory and module instance to be garbage collected. Fixes: #20920
1 parent 662cb06 commit 2b1dc86

File tree

7 files changed

+98
-32
lines changed

7 files changed

+98
-32
lines changed

src/lib/libpthread.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ var LibraryPThread = {
8686
'$spawnThread',
8787
'_emscripten_thread_free_data',
8888
'exit',
89+
'pthread_self',
90+
'__set_thread_state',
91+
'$waitAsyncPolyfilled',
8992
#if PTHREADS_DEBUG || ASSERTIONS
9093
'$ptrToString',
9194
#endif
@@ -190,6 +193,22 @@ var LibraryPThread = {
190193
PThread.runningWorkers = [];
191194
PThread.pthreads = {};
192195
},
196+
197+
terminateRuntime: () => {
198+
#if ASSERTIONS
199+
assert(!ENVIRONMENT_IS_PTHREAD, 'terminateRuntime() can only ever be called from main application thread!');
200+
#endif
201+
PThread.terminateAllThreads();
202+
var pthread_ptr = _pthread_self();
203+
___set_thread_state(0, 0, 0, 1);
204+
if (!waitAsyncPolyfilled) {
205+
// Break the waitAsync loop. Note that checkMailbox will not
206+
// re-register since the `___set_thread_state` above causes _pthread_self
207+
// to return 0.
208+
Atomics.notify(HEAP32, {{{ getHeapOffset('pthread_ptr', 'i32') }}});
209+
}
210+
},
211+
193212
returnWorkerToPool: (worker) => {
194213
// We don't want to run main thread queued calls here, since we are doing
195214
// some operations that leave the worker queue in an invalid state until

src/postamble_minimal.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
function exitRuntime(ret) {
1616
<<< ATEXITS >>>
1717
#if PTHREADS
18-
PThread.terminateAllThreads();
18+
PThread.terminateRuntime();
1919
#endif
2020

2121
#if ASSERTIONS

src/preamble.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ function exitRuntime() {
208208
#endif
209209
<<< ATEXITS >>>
210210
#if PTHREADS
211-
PThread.terminateAllThreads();
211+
PThread.terminateRuntime();
212212
#endif
213213
runtimeExited = true;
214214
}

test/codesize/test_codesize_minimal_pthreads.json

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7367,
3-
"a.out.js.gz": 3603,
4-
"a.out.nodebug.wasm": 19075,
5-
"a.out.nodebug.wasm.gz": 8818,
6-
"total": 26442,
7-
"total_gz": 12421,
3+
"a.out.js.gz": 3605,
4+
"a.out.nodebug.wasm": 19079,
5+
"a.out.nodebug.wasm.gz": 8824,
6+
"total": 26446,
7+
"total_gz": 12429,
88
"sent": [
99
"a (memory)",
1010
"b (exit)",
@@ -38,13 +38,14 @@
3838
"n (__pthread_create_js)"
3939
],
4040
"exports": [
41-
"A (_emscripten_thread_free_data)",
42-
"B (_emscripten_thread_exit)",
43-
"C (_emscripten_check_mailbox)",
44-
"D (emscripten_stack_set_limits)",
45-
"E (_emscripten_stack_restore)",
46-
"F (_emscripten_stack_alloc)",
47-
"G (emscripten_stack_get_current)",
41+
"A (_emscripten_run_js_on_main_thread)",
42+
"B (_emscripten_thread_free_data)",
43+
"C (_emscripten_thread_exit)",
44+
"D (_emscripten_check_mailbox)",
45+
"E (emscripten_stack_set_limits)",
46+
"F (_emscripten_stack_restore)",
47+
"G (_emscripten_stack_alloc)",
48+
"H (emscripten_stack_get_current)",
4849
"o (__wasm_call_ctors)",
4950
"p (add)",
5051
"q (main)",
@@ -54,9 +55,9 @@
5455
"u (pthread_self)",
5556
"v (_emscripten_proxy_main)",
5657
"w (_emscripten_thread_init)",
57-
"x (_emscripten_thread_crashed)",
58-
"y (_emscripten_run_js_on_main_thread_done)",
59-
"z (_emscripten_run_js_on_main_thread)"
58+
"x (__set_thread_state)",
59+
"y (_emscripten_thread_crashed)",
60+
"z (_emscripten_run_js_on_main_thread_done)"
6061
],
6162
"funcs": [
6263
"$__errno_location",

test/codesize/test_codesize_minimal_pthreads_memgrowth.json

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
22
"a.out.js": 7769,
3-
"a.out.js.gz": 3809,
4-
"a.out.nodebug.wasm": 19076,
5-
"a.out.nodebug.wasm.gz": 8819,
6-
"total": 26845,
7-
"total_gz": 12628,
3+
"a.out.js.gz": 3811,
4+
"a.out.nodebug.wasm": 19080,
5+
"a.out.nodebug.wasm.gz": 8825,
6+
"total": 26849,
7+
"total_gz": 12636,
88
"sent": [
99
"a (memory)",
1010
"b (exit)",
@@ -38,13 +38,14 @@
3838
"n (__pthread_create_js)"
3939
],
4040
"exports": [
41-
"A (_emscripten_thread_free_data)",
42-
"B (_emscripten_thread_exit)",
43-
"C (_emscripten_check_mailbox)",
44-
"D (emscripten_stack_set_limits)",
45-
"E (_emscripten_stack_restore)",
46-
"F (_emscripten_stack_alloc)",
47-
"G (emscripten_stack_get_current)",
41+
"A (_emscripten_run_js_on_main_thread)",
42+
"B (_emscripten_thread_free_data)",
43+
"C (_emscripten_thread_exit)",
44+
"D (_emscripten_check_mailbox)",
45+
"E (emscripten_stack_set_limits)",
46+
"F (_emscripten_stack_restore)",
47+
"G (_emscripten_stack_alloc)",
48+
"H (emscripten_stack_get_current)",
4849
"o (__wasm_call_ctors)",
4950
"p (add)",
5051
"q (main)",
@@ -54,9 +55,9 @@
5455
"u (pthread_self)",
5556
"v (_emscripten_proxy_main)",
5657
"w (_emscripten_thread_init)",
57-
"x (_emscripten_thread_crashed)",
58-
"y (_emscripten_run_js_on_main_thread_done)",
59-
"z (_emscripten_run_js_on_main_thread)"
58+
"x (__set_thread_state)",
59+
"y (_emscripten_thread_crashed)",
60+
"z (_emscripten_run_js_on_main_thread_done)"
6061
],
6162
"funcs": [
6263
"$__errno_location",
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
async function run() {
2+
console.log('START');
3+
let bufferCollected = false;
4+
const registry = new FinalizationRegistry(() => {
5+
bufferCollected = true;
6+
});
7+
8+
await (async () => {
9+
const instance = await Module();
10+
console.log('GOT INSTANCE');
11+
// Once this scope exits we expect the instance and its memory to be
12+
// collected.
13+
registry.register(instance.wasmMemory.buffer, "wasmMemory");
14+
})();
15+
16+
// Force GC
17+
const assert = require('node:assert/strict');
18+
assert(global.gc);
19+
for (let i = 0; i < 20; i++) {
20+
global.gc();
21+
await new Promise(r => setTimeout(r, 50));
22+
if (bufferCollected) break;
23+
}
24+
25+
if (bufferCollected) {
26+
console.log('SUCCESS: No leak detected');
27+
process.exit(0);
28+
} else {
29+
console.log('FAILURE: Leak detected');
30+
process.exit(1);
31+
}
32+
}
33+
34+
if (!isPthread)
35+
run();

test/test_other.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11753,6 +11753,16 @@ def test_pthread_unavailable(self):
1175311753
expected = ['got: hello world string, longer than 16 chars', 'pthread_create: environment does not support SharedArrayBuffer, pthreads are not available']
1175411754
self.do_runf('hello_world.c', expected, assert_all=True, assert_returncode=NON_ZERO, cflags=['--pre-js=pre.js'])
1175511755

11756+
@requires_node
11757+
@requires_pthreads
11758+
def test_pthread_mem_leak(self):
11759+
self.set_setting('MODULARIZE')
11760+
self.set_setting('EXIT_RUNTIME')
11761+
self.set_setting('EXPORTED_RUNTIME_METHODS', ['wasmMemory'])
11762+
self.node_args.append('--expose-gc')
11763+
self.cflags += ['--extern-post-js', test_file('pthread/test_pthread_mem_leak_post.js')]
11764+
self.do_runf('hello_world.c', 'SUCCESS: No leak detected', cflags=['-pthread'])
11765+
1175611766
def test_stdin_preprocess(self):
1175711767
create_file('temp.h', '#include <string>')
1175811768
outputStdin = self.run_process([EMCC, '-x', 'c++', '-dM', '-E', '-'], input="#include <string>", stdout=PIPE).stdout

0 commit comments

Comments
 (0)