8000 Use std::thread for coro::thread_pool by jbaldwin · Pull Request #221 · jbaldwin/libcoro · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use std::thread for coro::thread_pool #221

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Dec 1, 2023
Merged

Use std::thread for coro::thread_pool #221

merged 2 commits into from
Dec 1, 2023

Conversation

jbaldwin
Copy link
Owner
  • This change switches from std::jthread to std::thread for the coro::thread_pool
  • This change is in support of emscripten support

Closes #220

@jbaldwin
Use std::thread for coro::thread_pool
9cdbb8a 
* This change switches from std::jthread to std::thread for the
  coro::thread_pool
* This change is in support of emscripten support

Closes #220
@jbaldwin jbaldwin self-assigned this Nov 29, 2023
jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
@jbaldwin
Copy link
Owner Author

@bruno-j-nicoletti I think I found it! Its the notify_all() in the shutdown, its got some kind of a race condition and by placing the lock around it before calling it guarantees each worker thread in the coro::thread_pool will properly catch it. Clearly the std::jthread stop_token handled this properly 😬 . Please take a look, I did find one relaxed instance that was probably wrong so that got fixed as well.

jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
jbaldwin
jbaldwin commented Nov 29, 2023
View reviewed changes
@jbaldwin jbaldwin mentioned this pull request Dec 1, 2023
Merged
@jbaldwin jbaldwin merged commit d1be506 into main Dec 1, 2023
@jbaldwin jbaldwin deleted the std-thread-for-thread-pool branch December 1, 2023 15:59
@jbaldwin jbaldwin mentioned this pull request Dec 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bruno-j-nicoletti bruno-j-nicoletti bruno-j-nicoletti approved these changes

Assignees

@jbaldwin jbaldwin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

coro:thread_pool use std::thread instead of std::jthread
2 participants
@jbaldwin @bruno-j-nicoletti
0