-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Return detached threads to the pool #8286
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
aac8627
Return detached threads to the pool
VirtualTim 7b8a67c
Check if a thread is detached before freeing its resources
VirtualTim 8438f0e
Fix typo in previous commit
VirtualTim e40b80c
Extract 'returning threads to the pool' into a function
VirtualTim 7a5d296
When workers are returned to the pool remove their disposed thread fr…
VirtualTim 86baf13
Add a test for detached threads
VirtualTim ee97e8a
Add test_pthread_join test.
VirtualTim 1dab304
Code Review Changes
VirtualTim 921628b
Update detached thread test to run much quicker
VirtualTim 461ddb4
Missed two function renames
VirtualTim b1cd5b3
Fix thread resources being double freed on join.
VirtualTim 70bd24c
Fix thread resources being double freed on join Part2
VirtualTim 4e29926
test_pthread_detach -> test_std_thread_detach
VirtualTim 2c48635
rename test to test_std_thread_detach
VirtualTim 773e863
Revert changes that caused a double free.
VirtualTim 6ee3d7e
Fix cleanup ordering. Moved setting thread exit status to after main …
VirtualTim 31a5da8
Merge from emscripten-core/incoming
VirtualTim bc6e5ad
See if this this allows join on the main thread.
VirtualTim f82d037
Don't try and clean up an cleaned up thread
VirtualTim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| // 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 <thread> | ||
| #include <math.h> | ||
| #include <emscripten.h> | ||
| #include <assert.h> | ||
|
|
||
| #ifndef REPORT_RESULT | ||
| #include <iostream> | ||
| #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<10; i++) //simulate work | ||
| d += (i%2 ? sqrt((int)(rand())) : (-1)*sqrt((int)(rand()))); | ||
| } ).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_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_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_workers); | ||
| #else | ||
| 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.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( | ||
| 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<threads_to_spawn; i++) { | ||
| setTimeout(() => { _spawn_a_thread(); }, i*100); | ||
| } | ||
|
|
||
| //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); | ||
| ); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it would be better to write this using just the pthreads API, and not look at
PThread.unusedWorkerPooland other internal details, since this might change in future refactorings. but if it's much easier to write it this way then it's fine for now I thinkThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm not sure this can be done from the pthreads API. I mean, you can't really use the API to test the API, right?
I realise that this will likely change once we get native WASM threads, but I think by then we'll need to redo a lot of the pthreads tests anyway.