From 7fd5d996c168a6957fc52cfabeafe7f9e821282f Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Wed, 10 Nov 2021 11:28:43 -0800 Subject: [PATCH] Use compareExchange to set detach_state in pthread_join_js This is a little more robust in the face of racey/invalid code that does stuff like joining from muliple threads or detaching a thread while it is being joined. --- src/library_pthread.js | 47 +++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 26 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 1b01fdbb11f7a..f6250948e2c2c 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -125,26 +125,6 @@ var LibraryPThread = { }, #endif -#if PTHREADS_DEBUG - detachStateToString: function(state) { - if (state === {{{ cDefine('DT_EXITED') }}}) return 'DT_EXITED'; - if (state === {{{ cDefine('DT_EXITING') }}}) return 'DT_EXITING'; - if (state === {{{ cDefine('DT_JOINABLE') }}}) return 'DT_JOINABLE'; - if (state === {{{ cDefine('DT_DETACHED') }}}) return 'DT_DETACHED'; - assert(false); - }, -#endif - - setDetachState: function(thread, newstate) { -#if PTHREADS_DEBUG - var oldstate = Atomics.load(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detach_state }}}) >> 2); - var oldname = PThread.detachStateToString(oldstate); - var newname = PThread.detachStateToString(newstate); - console.log('thread 0x' + thread.toString(16) + ' state change: ' + oldname + ' -> ' + newname); -#endif - Atomics.store(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detach_state }}}) >> 2, newstate); - }, - terminateAllThreads: function() { #if ASSERTIONS assert(!ENVIRONMENT_IS_PTHREAD, 'Internal Error! terminateAllThreads() can only ever be called from main application thread!'); @@ -598,7 +578,7 @@ var LibraryPThread = { var tis = pthread.threadInfoStruct >> 2; // spawnThread is always called with a zero-initialized thread struct so // no need to set any valudes to zero here. - PThread.setDetachState(pthread.threadInfoStruct, threadParams.initialState); + Atomics.store(HEAPU32, tis + ({{{ C_STRUCTS.pthread.detach_state }}} >> 2), threadParams.initialState); Atomics.store(HEAPU32, tis + ({{{ C_STRUCTS.pthread.stack_size }}} >> 2), threadParams.stackSize); Atomics.store(HEAPU32, tis + ({{{ C_STRUCTS.pthread.stack }}} >> 2), stackHigh); @@ -892,24 +872,39 @@ var LibraryPThread = { #endif for (;;) { - var detach_state = Atomics.load(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detach_state }}} ) >> 2); - if (detach_state == {{{ cDefine('DT_EXITING') }}}) { // Exiting? + // The thread we are joining with must be either DT_JOINABLE or + // DT_EXITING. If its DT_EXITING then we move it to DT_EXITED and + // we are done. If its DT_JOINABLE we keep waiting. + var old_state = Atomics.compareExchange(HEAP32, + (thread + {{{ C_STRUCTS.pthread.detach_state }}}) >> 2, + {{{ cDefine('DT_EXITING') }}}, + {{{ cDefine('DT_EXITED') }}} + ); + if (old_state == {{{ cDefine('DT_EXITING') }}}) { +#if PTHREADS_DEBUG + err('thread 0x' + thread.toString(16) + ' successfully joined'); +#endif + // We successfully marked the tread as DT_EXITED if (status) { var result = Atomics.load(HEAPU32, (thread + {{{ C_STRUCTS.pthread.result }}} ) >> 2); {{{ makeSetValue('status', 0, 'result', 'i32') }}}; } - // Mark the thread as exited. - PThread.setDetachState(thread, {{{ cDefine('DT_EXITED') }}}); if (!ENVIRONMENT_IS_PTHREAD) cleanupThread(thread); else postMessage({ 'cmd': 'cleanupThread', 'thread': thread }); return 0; } +#if ASSERTIONS + assert(old_state === {{{ cDefine('DT_JOINABLE') }}}, 'pthread_join attempted on thread 0x' + thread.toString(16) + ', which is in an invalid state:' + old_state); +#else + if (old_state !== {{{ cDefine('DT_JOINABLE') }}}) return {{{ cDefine('EINVAL') }}}; +#endif + _pthread_testcancel(); // In main runtime thread (the thread that initialized the Emscripten C // runtime and launched main()), assist pthreads in performing operations // that they need to access the Emscripten main runtime for. if (!ENVIRONMENT_IS_PTHREAD) _emscripten_main_thread_process_queued_calls(); - _emscripten_futex_wait(thread + {{{ C_STRUCTS.pthread.detach_state }}}, detach_state, ENVIRONMENT_IS_PTHREAD ? 100 : 1); + _emscripten_futex_wait(thread + {{{ C_STRUCTS.pthread.detach_state }}}, old_state, ENVIRONMENT_IS_PTHREAD ? 100 : 1); } },