Skip to content
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

New function: default_thread_pool_shutdown() #2382

Merged
merged 3 commits into from
Jun 12, 2023

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Nov 5, 2019

This cleanly terminates all worker threads of the default thread pool
by resizing the pool to 0 threads... but only if the pool has been
created in the first place. Note that the alternate idiom

default_thread_pool()->resize(0);

seems like it should accomplish the same thing, but in fact will start
by creating a pool and its threads, if it has not done so before, only
to immediately terminate them.

@fpsunflower
Copy link
Contributor

I'm not sure I follow the logic.

If the thread pool already exists, it will stop all the threads. But if the thread pool hasn't been created yet, it isn't going to prevent them from being created in the future.

So someone might call default_thread_pool_shutdown() only to see the default thread pool get created in the future.

Am I missing something?

@lgritz
Copy link
Collaborator Author

lgritz commented Nov 5, 2019

It's intended to be called at shutdown. Hence the name, default_thread_pool_shutdown().

The intent is to solve a problem that occurs for some apps on Windows, where apparently the app upon terminating can kill the threads before calling the static destructors (like that of the thread pool), and if one of those threads held a mutex, it can cause a deadlock.

I don't 100% understand the situation of what set of circumstances was a concern, but this was a request from a software vendor and it seemed reasonable. The problem is "solved" for them by calling default_thread_pool->resize(0), but they wanted a way to avoid that idiom's tendency to create a bunch of threads for the case where the pool had never been created in the first place.

@lgritz
Copy link
Collaborator Author

lgritz commented Nov 5, 2019

Pause this, I'm still haggling over the technically correct solution with the requestors...

This cleanly terminates all worker threads of the default thread pool
by resizing the pool to 0 threads... but only if the pool has been
created in the first place. Note that the alternate idiom

    default_thread_pool()->resize(0);

seems like it should accomplish the same thing, but in fact will start
by creating a pool and its threads, if it has not done so before, only
to immediately terminate them.
@lgritz
Copy link
Collaborator Author

lgritz commented Nov 23, 2019

Abandoning this, it seems not to be needed at this time.

@lgritz
Copy link
Collaborator Author

lgritz commented Jun 12, 2023

This look ok to you, @LazyDodo ?

@LazyDodo
Copy link
Contributor

LGTM!

@lgritz lgritz merged commit c9f0c93 into AcademySoftwareFoundation:master Jun 12, 2023
@lgritz lgritz deleted the lg-threadpool branch June 12, 2023 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants