From aac86279eea5dd92f6665de63f6b5263f984706f Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Tue, 12 Mar 2019 13:15:27 +0800 Subject: [PATCH 01/18] Return detached threads to the pool Once detached threads are finished their execution they emit the 'exit' command. Instead of a noop they should rejoin the pool. I don't know if this change has any other consequences. --- src/library_pthread.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 03d752e9a00e0..0cf14a631944e 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -319,17 +319,14 @@ var LibraryPThread = { err('Thread ' + d.threadId + ': ' + d.text); } else if (d.cmd === 'alert') { alert('Thread ' + d.threadId + ': ' + d.text); - } else if (d.cmd === 'exit') { - // Thread is exiting, no-op here } else if (d.cmd === 'exitProcess') { // A pthread has requested to exit the whole application process (runtime). Module['noExitRuntime'] = false; exit(d.returnCode); - } else if (d.cmd === 'cancelDone') { + } else if (d.cmd === 'exit' || d.cmd === 'cancelDone') { PThread.freeThreadData(worker.pthread); worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. PThread.unusedWorkerPool.push(worker); - // TODO: Free if detached. PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. } else if (d.cmd === 'objectTransfer') { PThread.receiveObjectTransfer(e.data); From 7b8a67cba5cc9b19e5417a7bdc0d673b78bb4d3c Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Tue, 12 Mar 2019 17:39:06 +0800 Subject: [PATCH 02/18] Check if a thread is detached before freeing its resources --- src/library_pthread.js | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 0cf14a631944e..000ff78b7adae 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -185,7 +185,7 @@ var LibraryPThread = { delete Module['canvas']; #endif - postMessage({ cmd: 'exit' }); + postMessage({ cmd: 'exit', threadId: tb }); } } }, @@ -324,10 +324,13 @@ var LibraryPThread = { Module['noExitRuntime'] = false; exit(d.returnCode); } else if (d.cmd === 'exit' || d.cmd === 'cancelDone') { - PThread.freeThreadData(worker.pthread); - worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. - PThread.unusedWorkerPool.push(worker); - PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. + var detached = Atomics.load(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); + if (detached) { + PThread.freeThreadData(worker.pthread); + worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. + PThread.unusedWorkerPool.push(worker); + PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. + } } else if (d.cmd === 'objectTransfer') { PThread.receiveObjectTransfer(e.data); } else if (e.data.target === 'setimmediate') { From 8438f0e1bb6449666ddde1c01f2dbb8452b5cfdc Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Tue, 12 Mar 2019 17:40:47 +0800 Subject: [PATCH 03/18] Fix typo in previous commit --- src/library_pthread.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 000ff78b7adae..84c4d4d45eb60 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -324,7 +324,7 @@ var LibraryPThread = { Module['noExitRuntime'] = false; exit(d.returnCode); } else if (d.cmd === 'exit' || d.cmd === 'cancelDone') { - var detached = Atomics.load(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); + var detached = Atomics.load(HEAPU32, (d.threadId + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); if (detached) { PThread.freeThreadData(worker.pthread); worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. From e40b80cbfb848cc8d02d15a2d5f8fc2a059ebb0f Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Wed, 17 Apr 2019 12:01:19 +0800 Subject: [PATCH 04/18] Extract 'returning threads to the pool' into a function Plus some minor refactoring. --- src/library_pthread.js | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 84c4d4d45eb60..b10d2e3da37b0 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -239,7 +239,13 @@ var LibraryPThread = { pthread.stackBase = 0; if (pthread.worker) pthread.worker.pthread = null; }, - + returnThreadToPool: function(workerToFree) { + PThread.freeThreadData(workerToFree.pthread); + workerToFree.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. + PThread.unusedWorkerPool.push(workerToFree); + PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(workerToFree.pthread), 1); // Not a running Worker anymore. + }, + receiveObjectTransfer: function(data) { #if OFFSCREENCANVAS_SUPPORT if (typeof GL !== 'undefined') { @@ -319,18 +325,17 @@ var LibraryPThread = { err('Thread ' + d.threadId + ': ' + d.text); } else if (d.cmd === 'alert') { alert('Thread ' + d.threadId + ': ' + d.text); + } else if (d.cmd === 'exit') { + var detached = Atomics.load(HEAPU32, (d.threadId + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); + if (detached) { + PThread.returnThreadToPool(worker); + } } else if (d.cmd === 'exitProcess') { // A pthread has requested to exit the whole application process (runtime). Module['noExitRuntime'] = false; exit(d.returnCode); - } else if (d.cmd === 'exit' || d.cmd === 'cancelDone') { - var detached = Atomics.load(HEAPU32, (d.threadId + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); - if (detached) { - PThread.freeThreadData(worker.pthread); - worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. - PThread.unusedWorkerPool.push(worker); - PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. - } + } else if (d.cmd === 'cancelDone') { + PThread.returnThreadToPool(worker); } else if (d.cmd === 'objectTransfer') { PThread.receiveObjectTransfer(e.data); } else if (e.data.target === 'setimmediate') { @@ -409,10 +414,7 @@ var LibraryPThread = { {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; var pthread = PThread.pthreads[pthread_ptr]; var worker = pthread.worker; - PThread.freeThreadData(pthread); - worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. - PThread.unusedWorkerPool.push(worker); - PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. + PThread.returnThreadToPool(worker); }, _cancel_thread: function(pthread_ptr) { From 7a5d29637f98498873c3fc666d220bf0ca12b26c Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Wed, 24 Apr 2019 17:55:22 +0800 Subject: [PATCH 05/18] When workers are returned to the pool remove their disposed thread from the PThread.pthreads object --- src/library_pthread.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/library_pthread.js b/src/library_pthread.js index b10d2e3da37b0..38bd7064d759f 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -240,7 +240,9 @@ var LibraryPThread = { if (pthread.worker) pthread.worker.pthread = null; }, returnThreadToPool: function(workerToFree) { + delete PThread.pthreads[workerToFree.pthread.thread]; PThread.freeThreadData(workerToFree.pthread); + //Note: workerToFree is intentionally not terminated so the pool can dynamically grow. workerToFree.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. PThread.unusedWorkerPool.push(workerToFree); PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(workerToFree.pthread), 1); // Not a running Worker anymore. From 86baf130e125e3578b2a4e00943ef3bb761c3014 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Wed, 24 Apr 2019 18:02:34 +0800 Subject: [PATCH 06/18] Add a test for detached threads May need to adjust timings --- tests/pthread/test_pthread_detach.cpp | 57 +++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 tests/pthread/test_pthread_detach.cpp diff --git a/tests/pthread/test_pthread_detach.cpp b/tests/pthread/test_pthread_detach.cpp new file mode 100644 index 0000000000000..d7f7122915e15 --- /dev/null +++ b/tests/pthread/test_pthread_detach.cpp @@ -0,0 +1,57 @@ +// Copyright 2019 The Emscripten Authors. All rights reserved. +// Emscripten is available under two separate licenses, the MIT license and the +// University of Illinois/NCSA Open Source License. Both these licenses can be +// found in the LICENSE file. + +#include +#include +#include +#include + +#ifndef REPORT_RESULT +#include +#endif + +extern "C" { +//Create a thread that does some work +void EMSCRIPTEN_KEEPALIVE spawn_a_thread() { + std::thread( [] { + double d=0; + for (int i=0; i<10000000; i++) //simulate work + d += (i%2 ? sqrt((int)(rand())) : (-1)*sqrt((int)(rand()))); +#ifndef REPORT_RESULT + std::cout << d << std::endl; +#endif + } ).detach(); +} + + +//Check that the number of workers is less than the number of spawned threads. +void EMSCRIPTEN_KEEPALIVE count_threads(int num_threads_spawned) { + int num_threads = EM_ASM_INT({ + return PThread.runningWorkers.length + PThread.unusedWorkerPool.length; + }); + +#ifdef REPORT_RESULT + if (num_threads < num_threads_spawned) + REPORT_RESULT(0); + else + REPORT_RESULT(num_threads); +#else + assert(num_threads < num_threads_spawned); + std::cout << num_threads << std::endl; +#endif +} +} + +//Spawn a detached thread every 0.5s. After 10s Check that the number of workers are less than the number of spawned threads +int main(int argc, char** argv) { + EM_ASM( + const i_max = 20; + for (let i=0; i { _spawn_a_thread(); }, i*500); + } + + setTimeout(() => { _count_threads(i_max); }, i_max*500); + ); +} From ee97e8af2add4b91202c5ae34ee6f5c75edeb37b Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Wed, 24 Apr 2019 18:10:12 +0800 Subject: [PATCH 07/18] Add test_pthread_join test. --- tests/test_browser.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_browser.py b/tests/test_browser.py index 4523c04b88c1f..023b8a5430f47 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -3598,6 +3598,11 @@ def test_pthread_nested_spawns(self): def test_pthread_join(self): self.btest(path_from_root('tests', 'pthread', 'test_pthread_join.cpp'), expected='6765', args=['-O3', '-s', 'USE_PTHREADS=1', '-s', 'PTHREAD_POOL_SIZE=8']) + # Test that threads can rejoin the pool once detached and finished + @requires_threads + def test_pthread_detach(self): + self.btest(path_from_root('tests', 'pthread', 'test_pthread_detach.cpp'), expected='0', args=['-std=c++11', '-s', 'USE_PTHREADS=1']) + # Test pthread_cancel() operation @requires_threads def test_pthread_cancel(self): From 1dab304042cdc6763d7a71276951f0a88c9c07fa Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Tue, 7 May 2019 13:41:56 +0800 Subject: [PATCH 08/18] Code Review Changes 1. Don't pass around threadId 2. Rename returnThreadToPool to returnWorkerToPool --- src/library_pthread.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 38bd7064d759f..b781c58d36421 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -185,7 +185,7 @@ var LibraryPThread = { delete Module['canvas']; #endif - postMessage({ cmd: 'exit', threadId: tb }); + postMessage({ cmd: 'exit' }); } } }, @@ -239,13 +239,13 @@ var LibraryPThread = { pthread.stackBase = 0; if (pthread.worker) pthread.worker.pthread = null; }, - returnThreadToPool: function(workerToFree) { - delete PThread.pthreads[workerToFree.pthread.thread]; - PThread.freeThreadData(workerToFree.pthread); - //Note: workerToFree is intentionally not terminated so the pool can dynamically grow. - workerToFree.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. - PThread.unusedWorkerPool.push(workerToFree); - PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(workerToFree.pthread), 1); // Not a running Worker anymore. + returnWorkerToPool: function(worker) { + delete PThread.pthreads[worker.pthread.thread]; + PThread.freeThreadData(worker.pthread); + //Note: worker is intentionally not terminated so the pool can dynamically grow. + worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. + PThread.unusedWorkerPool.push(worker); + PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. }, receiveObjectTransfer: function(data) { @@ -328,9 +328,9 @@ var LibraryPThread = { } else if (d.cmd === 'alert') { alert('Thread ' + d.threadId + ': ' + d.text); } else if (d.cmd === 'exit') { - var detached = Atomics.load(HEAPU32, (d.threadId + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); + var detached = Atomics.load(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); if (detached) { - PThread.returnThreadToPool(worker); + PThread.returnWorkerToPool(worker); } } else if (d.cmd === 'exitProcess') { // A pthread has requested to exit the whole application process (runtime). From 921628ba1c7562d4d677c784414469b070ee8079 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Tue, 7 May 2019 15:15:03 +0800 Subject: [PATCH 09/18] Update detached thread test to run much quicker May need to adjust max_thread_check if there are any timing issues. But 5 checks seems very lenient. --- tests/pthread/test_pthread_detach.cpp | 49 +++++++++++++++++++-------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/tests/pthread/test_pthread_detach.cpp b/tests/pthread/test_pthread_detach.cpp index d7f7122915e15..934de368f30dc 100644 --- a/tests/pthread/test_pthread_detach.cpp +++ b/tests/pthread/test_pthread_detach.cpp @@ -17,41 +17,60 @@ extern "C" { void EMSCRIPTEN_KEEPALIVE spawn_a_thread() { std::thread( [] { double d=0; - for (int i=0; i<10000000; i++) //simulate work + for (int i=0; i<10; i++) //simulate work d += (i%2 ? sqrt((int)(rand())) : (-1)*sqrt((int)(rand()))); -#ifndef REPORT_RESULT - std::cout << d << std::endl; -#endif } ).detach(); } //Check that the number of workers is less than the number of spawned threads. -void EMSCRIPTEN_KEEPALIVE count_threads(int num_threads_spawned) { - int num_threads = EM_ASM_INT({ +void EMSCRIPTEN_KEEPALIVE count_threads(int num_threads_spawned, int num_threads_spawned_extra) { + num_threads_spawned += num_threads_spawned_extra; + int num_workers = EM_ASM_INT({ return PThread.runningWorkers.length + PThread.unusedWorkerPool.length; }); #ifdef REPORT_RESULT - if (num_threads < num_threads_spawned) + if (num_threads_spawned_extra == 0) //check extra thread spawned + REPORT_RESULT(-1); + if (num_workers < num_threads_spawned) //check worker returned to pool and was assigned another thread REPORT_RESULT(0); else - REPORT_RESULT(num_threads); + REPORT_RESULT(num_workers); #else - assert(num_threads < num_threads_spawned); - std::cout << num_threads << std::endl; + std::cout << + "Worker pool size: " << num_workers << + ", Number of threads spawned: " << num_threads_spawned + << "." << std::endl; + assert(num_threads_spawned_extra != 0); + assert(num_workers < num_threads_spawned); #endif } } -//Spawn a detached thread every 0.5s. After 10s Check that the number of workers are less than the number of spawned threads +//Spawn a detached thread every 0.1s. After 0.3s Check that the number of workers are less than the number of spawned threads int main(int argc, char** argv) { EM_ASM( - const i_max = 20; - for (let i=0; i { _spawn_a_thread(); }, i*500); + let thread_check = 0; + const max_thread_check = 5; //fail the test if the number of threads doesn't go down after checking this many times + const threads_to_spawn = 3; + let threads_to_spawn_extra = 0; + + //Spawn some detached threads + for (let i=0; i { _spawn_a_thread(); }, i*100); } - setTimeout(() => { _count_threads(i_max); }, i_max*500); + //Check if a worker is free every threads_to_spawn*100 ms, or until max_thread_check is exceeded + const SpawnMoreThreads = setInterval(() => { + if (PThread.unusedWorkerPool.length > 0) { //Spawn a thread if a worker is available + _spawn_a_thread(); + threads_to_spawn_extra++; + } + if (thread_check++ > max_thread_check || threads_to_spawn_extra > 0) { + clearInterval(SpawnMoreThreads); + _count_threads(threads_to_spawn, threads_to_spawn_extra); + } + }, threads_to_spawn*100); ); } From 461ddb472f5e5c720ccce1303343504fb93bc19c Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Tue, 7 May 2019 16:02:32 +0800 Subject: [PATCH 10/18] Missed two function renames --- src/library_pthread.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index b781c58d36421..4c9a54312d5b6 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -337,7 +337,7 @@ var LibraryPThread = { Module['noExitRuntime'] = false; exit(d.returnCode); } else if (d.cmd === 'cancelDone') { - PThread.returnThreadToPool(worker); + PThread.returnWorkerToPool(worker); } else if (d.cmd === 'objectTransfer') { PThread.receiveObjectTransfer(e.data); } else if (e.data.target === 'setimmediate') { @@ -416,7 +416,7 @@ var LibraryPThread = { {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; var pthread = PThread.pthreads[pthread_ptr]; var worker = pthread.worker; - PThread.returnThreadToPool(worker); + PThread.returnWorkerToPool(worker); }, _cancel_thread: function(pthread_ptr) { From b1cd5b333c201b804cd73f1357931b69d6cebbfc Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Thu, 9 May 2019 16:03:30 +0800 Subject: [PATCH 11/18] Fix thread resources being double freed on join. --- src/library_pthread.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 4c9a54312d5b6..ba3832d32a328 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -305,11 +305,11 @@ var LibraryPThread = { } else if (d.cmd === 'spawnThread') { __spawn_thread(e.data); } else if (d.cmd === 'cleanupThread') { - __cleanup_thread(d.thread); + __cleanup_thread(worker.pthread.thread); } else if (d.cmd === 'killThread') { - __kill_thread(d.thread); + __kill_thread(worker.pthread.thread); } else if (d.cmd === 'cancelThread') { - __cancel_thread(d.thread); + __cancel_thread(worker.pthread.thread); } else if (d.cmd === 'loaded') { worker.loaded = true; // If this Worker is already pending to start running a thread, launch the thread now @@ -760,7 +760,7 @@ var LibraryPThread = { Atomics.store(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2, 1); // Mark the thread as detached. if (!ENVIRONMENT_IS_PTHREAD) __cleanup_thread(thread); - else postMessage({ cmd: 'cleanupThread', thread: thread}); + else postMessage({ cmd: 'cleanupThread' }); return 0; } // TODO HACK! Replace the _js variant with just _pthread_testcancel: From 70bd24c8a9af145f6928eb31858aa7278ae02b69 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Thu, 9 May 2019 16:20:29 +0800 Subject: [PATCH 12/18] Fix thread resources being double freed on join Part2 Missed this change that was supposed to be part of the last changelist. --- src/library_pthread.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index ba3832d32a328..30993f7c32d6b 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -414,9 +414,6 @@ var LibraryPThread = { if (ENVIRONMENT_IS_PTHREAD) throw 'Internal Error! _cleanup_thread() can only ever be called from main application thread!'; if (!pthread_ptr) throw 'Internal Error! Null pthread_ptr in _cleanup_thread!'; {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; - var pthread = PThread.pthreads[pthread_ptr]; - var worker = pthread.worker; - PThread.returnWorkerToPool(worker); }, _cancel_thread: function(pthread_ptr) { From 4e2992674adf3838d9e698139ccc91c9ca890459 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Fri, 10 May 2019 10:40:13 +0800 Subject: [PATCH 13/18] test_pthread_detach -> test_std_thread_detach --- tests/test_browser.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_browser.py b/tests/test_browser.py index 023b8a5430f47..647c5b34903f1 100644 --- a/tests/test_browser.py +++ b/tests/test_browser.py @@ -3600,8 +3600,8 @@ def test_pthread_join(self): # Test that threads can rejoin the pool once detached and finished @requires_threads - def test_pthread_detach(self): - self.btest(path_from_root('tests', 'pthread', 'test_pthread_detach.cpp'), expected='0', args=['-std=c++11', '-s', 'USE_PTHREADS=1']) + def test_std_thread_detach(self): + self.btest(path_from_root('tests', 'pthread', 'test_std_thread_detach.cpp'), expected='0', args=['-std=c++11', '-s', 'USE_PTHREADS=1']) # Test pthread_cancel() operation @requires_threads From 2c486351881a13bd614456ecdf31c7006dfb2dd9 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Fri, 10 May 2019 10:41:00 +0800 Subject: [PATCH 14/18] rename test to test_std_thread_detach --- .../{test_pthread_detach.cpp => test_std_thread_detach.cpp} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/pthread/{test_pthread_detach.cpp => test_std_thread_detach.cpp} (100%) diff --git a/tests/pthread/test_pthread_detach.cpp b/tests/pthread/test_std_thread_detach.cpp similarity index 100% rename from tests/pthread/test_pthread_detach.cpp rename to tests/pthread/test_std_thread_detach.cpp From 773e86393627a224fbf600d916c857057418f4ee Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Fri, 10 May 2019 10:46:37 +0800 Subject: [PATCH 15/18] Revert changes that caused a double free. This wasn't introduced by this PR, and can be fixed by a subsequent one. Basically PThread.pthreads seems to not remove references to threads after that are exited. Once this PR is merged I'll open another one to address this issue. --- src/library_pthread.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 30993f7c32d6b..862bf7450fcd5 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -240,7 +240,8 @@ var LibraryPThread = { if (pthread.worker) pthread.worker.pthread = null; }, returnWorkerToPool: function(worker) { - delete PThread.pthreads[worker.pthread.thread]; + //thread is not removed from PThread.pthreads in some cases. Will need to investigate/fix. + //delete PThread.pthreads[worker.pthread.thread]; PThread.freeThreadData(worker.pthread); //Note: worker is intentionally not terminated so the pool can dynamically grow. worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. @@ -414,6 +415,9 @@ var LibraryPThread = { if (ENVIRONMENT_IS_PTHREAD) throw 'Internal Error! _cleanup_thread() can only ever be called from main application thread!'; if (!pthread_ptr) throw 'Internal Error! Null pthread_ptr in _cleanup_thread!'; {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; + var pthread = PThread.pthreads[pthread_ptr]; + var worker = pthread.worker; + PThread.returnWorkerToPool(worker); }, _cancel_thread: function(pthread_ptr) { From 6ee3d7ec37f8bc9d54a3ade1df60a08fe457f76b Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Mon, 13 May 2019 16:11:33 +0800 Subject: [PATCH 16/18] Fix cleanup ordering. Moved setting thread exit status to after main thread has exited worker. --- src/library_pthread.js | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 862bf7450fcd5..e8e7d4d578d2f 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -145,7 +145,6 @@ var LibraryPThread = { Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadExitCode }}} ) >> 2, exitCode); // When we publish this, the main thread is free to deallocate the thread object and we are done. // Therefore set threadInfoStruct = 0; above to 'release' the object in this worker thread. - Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadStatus }}} ) >> 2, 1); // Disable all cancellation so that executing the cleanup handlers won't trigger another JS // canceled exception to be thrown. @@ -240,15 +239,13 @@ var LibraryPThread = { if (pthread.worker) pthread.worker.pthread = null; }, returnWorkerToPool: function(worker) { - //thread is not removed from PThread.pthreads in some cases. Will need to investigate/fix. - //delete PThread.pthreads[worker.pthread.thread]; - PThread.freeThreadData(worker.pthread); + delete PThread.pthreads[worker.pthread.thread]; //Note: worker is intentionally not terminated so the pool can dynamically grow. - worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. PThread.unusedWorkerPool.push(worker); - PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore. + PThread.runningWorkers.splice(PThread.runningWorkers.indexOf(worker.pthread), 1); // Not a running Worker anymore + PThread.freeThreadData(worker.pthread); + worker.pthread = undefined; // Detach the worker from the pthread object, and return it to the worker pool as an unused worker. }, - receiveObjectTransfer: function(data) { #if OFFSCREENCANVAS_SUPPORT if (typeof GL !== 'undefined') { @@ -306,11 +303,11 @@ var LibraryPThread = { } else if (d.cmd === 'spawnThread') { __spawn_thread(e.data); } else if (d.cmd === 'cleanupThread') { - __cleanup_thread(worker.pthread.thread); + __cleanup_thread(d.thread); } else if (d.cmd === 'killThread') { - __kill_thread(worker.pthread.thread); + __kill_thread(d.thread); } else if (d.cmd === 'cancelThread') { - __cancel_thread(worker.pthread.thread); + __cancel_thread(d.thread); } else if (d.cmd === 'loaded') { worker.loaded = true; // If this Worker is already pending to start running a thread, launch the thread now @@ -329,9 +326,11 @@ var LibraryPThread = { } else if (d.cmd === 'alert') { alert('Thread ' + d.threadId + ': ' + d.text); } else if (d.cmd === 'exit') { - var detached = Atomics.load(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2); + var detached = Atomics.load(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.detached }}}) >> 2); if (detached) { PThread.returnWorkerToPool(worker); + } else { + Atomics.store(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.threadStatus }}}) >> 2, 1); } } else if (d.cmd === 'exitProcess') { // A pthread has requested to exit the whole application process (runtime). @@ -761,7 +760,7 @@ var LibraryPThread = { Atomics.store(HEAPU32, (thread + {{{ C_STRUCTS.pthread.detached }}} ) >> 2, 1); // Mark the thread as detached. if (!ENVIRONMENT_IS_PTHREAD) __cleanup_thread(thread); - else postMessage({ cmd: 'cleanupThread' }); + else postMessage({ cmd: 'cleanupThread', thread: thread }); return 0; } // TODO HACK! Replace the _js variant with just _pthread_testcancel: From bc6e5ad2d63e15a259c3cbd86a38acbb07dd41c4 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Fri, 17 May 2019 13:23:04 +0800 Subject: [PATCH 17/18] See if this this allows join on the main thread. --- src/library_pthread.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index 50fe87ae7b25f..a37272283ce3b 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -145,6 +145,7 @@ var LibraryPThread = { Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadExitCode }}} ) >> 2, exitCode); // When we publish this, the main thread is free to deallocate the thread object and we are done. // Therefore set threadInfoStruct = 0; above to 'release' the object in this worker thread. + Atomics.store(HEAPU32, (tb + {{{ C_STRUCTS.pthread.threadStatus }}} ) >> 2, 1); // Disable all cancellation so that executing the cleanup handlers won't trigger another JS // canceled exception to be thrown. @@ -300,11 +301,9 @@ var LibraryPThread = { } else if (d.cmd === 'alert') { alert('Thread ' + d.threadId + ': ' + d.text); } else if (d.cmd === 'exit') { - var detached = Atomics.load(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.detached }}}) >> 2); + var detached = worker.pthread && Atomics.load(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.detached }}}) >> 2); if (detached) { PThread.returnWorkerToPool(worker); - } else { - Atomics.store(HEAPU32, (worker.pthread.thread + {{{ C_STRUCTS.pthread.threadStatus }}}) >> 2, 1); } } else if (d.cmd === 'exitProcess') { // A pthread has requested to exit the whole application process (runtime). From f82d037d4cff19dd0a486cc31236ed99cd1ff022 Mon Sep 17 00:00:00 2001 From: Tim Lander Date: Fri, 17 May 2019 17:48:15 +0800 Subject: [PATCH 18/18] Don't try and clean up an cleaned up thread --- src/library_pthread.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/library_pthread.js b/src/library_pthread.js index a37272283ce3b..3d13eed20bfd5 100644 --- a/src/library_pthread.js +++ b/src/library_pthread.js @@ -387,8 +387,10 @@ var LibraryPThread = { if (!pthread_ptr) throw 'Internal Error! Null pthread_ptr in _cleanup_thread!'; {{{ makeSetValue('pthread_ptr', C_STRUCTS.pthread.self, 0, 'i32') }}}; var pthread = PThread.pthreads[pthread_ptr]; - var worker = pthread.worker; - PThread.returnWorkerToPool(worker); + if (pthread) { + var worker = pthread.worker; + PThread.returnWorkerToPool(worker); + } }, _cancel_thread: function(pthread_ptr) {