`test_pthread_exit_process` suspicious log message on Node

[kleisauke]

It prints this message:

emscripten/tests/core/pthread/test_pthread_exit_runtime.c

Line 21 in ad6e356

printf("done join -- should never get here\n");

Which probably shouldn't happen, since the exit(42); call in the thread is expected to bring down the whole process.

emscripten/tests/core/pthread/test_pthread_exit_runtime.c

Line 10 in ad6e356

exit(42);

Reproducer:

$ ./tests/runner wasm2.test_pthread_exit_process -v 2>&1 | grep -B3 -A2 "should never get here"
-- begin program output --
main
calling exit
done join -- should never get here
onExit status: 42
-- end program output --

Although, on the browser it seems to work fine. Tested with:

$ ./tests/runner browser.test_pthread_exit_process --browser="/usr/bin/google-chrome --enable-logging=stderr --v=1" 2>&1 | grep -B3 -A2 "should never get here"

[kleisauke]

After further testing, it looks like PR #14481 will resolve this.

[sbc100]

This does indeed look wrong. I think its because we send the exitProcess message back to the main thread but we then go ahead and exit the calling thread using throw new ExitStatus(status); which ends up marking the current thread as join-able, which we probably should not do.

[sbc100]

I'm not sure how #14481 will help because it doesn't (or I don't see how it will) stop the calling thread from marking itself as join-able in _emscripten_thread_exit.

[kleisauke]

It looks like some kind of race, from what I've seen. It can be fixed (or circumvented?) with moving this PThread.terminateAllThreads() call:

emscripten/src/postamble.js

Line 444 in 0321203

PThread.terminateAllThreads();

after the exitRuntime function, for example:

diff --git a/src/postamble.js b/src/postamble.js
index 1111111..2222222 100644
--- a/src/postamble.js
+++ b/src/postamble.js
@@ -440,10 +440,10 @@ function exit(status, implicit) {
     }
 #endif // ASSERTIONS
   } else {
+    exitRuntime();
 #if USE_PTHREADS
     PThread.terminateAllThreads();
 #endif
-    exitRuntime();
   }
 
   procExit(status);

(a similar thing is done within PR #14481)

I haven't debugged it further yet, but it looks like the PThread.terminateAllThreads() call somehow causes the thread to mark itself as joinable/exited, and therefore causes this block to be executed:

emscripten/src/library_pthread.js

Lines 828 to 838 in 0321203

	if (threadStatus == 1) { // Exited?
	if (status) {
	var result = Atomics.load(HEAPU32, (thread + {{{ C_STRUCTS.pthread.result }}} ) >> 2);
	{{{ makeSetValue('status', 0, 'result', 'i32') }}};
	}
	// Mark the thread as detached.
	Atomics.store(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2, 1);
	if (!ENVIRONMENT_IS_PTHREAD) cleanupThread(thread);
	else postMessage({ 'cmd': 'cleanupThread', 'thread': thread });
	return 0;
	}

Note that the musl upgrade might also fix this, as I didn't observe this behavior when I reverted commit d3142ae in PR #13007 (perhaps moving pthread_join to native code is the proper way to resolve this).