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

Remove ThreadManager #6205

Open
dktapps opened this issue Dec 20, 2023 · 0 comments
Open

Remove ThreadManager #6205

dktapps opened this issue Dec 20, 2023 · 0 comments
Labels
Category: Core Related to internal functionality Type: Cleanup Removes or deprecates API methods or behaviour

Comments

@dktapps
Copy link
Member

dktapps commented Dec 20, 2023

Description

The only remaining uses for ThreadManager are:

  • Providing thread count - inaccurate if threads start more threads, can be better provided by ext-pmmpthread directly
  • Stopping threads which haven't stopped themselves - mostly pointless
    • threads will join themselves when __destruct()'d under normal circumstances, which ThreadManager actually interferes with by keeping strong references
    • on-shutdown thread stopping can be implemented by overriding join() which will be called directly by ext-pmmpthread during __destruct()
  • Simulating detach() and/or leaking threads - not a particularly useful case, and can also be simulated more intentionally by implementing detach() in ext-pmmpthread directly
  • Delaying automatic attempts to stop threads to allow ServerKiller functionality to work - not sure how to replace or remove this

In addition:

  • Even if it did track threads started by other threads, we wouldn't actually want ThreadManager to try and stop them, because the parent of a thread can't be join()'d before its children are also join()'d. At best we would want to stop them in reverse order, which it's never done.

TL;DR: ThreadManager no longer serves a useful purpose, and is actually probably detrimental in some cases.

@dktapps dktapps added Category: Core Related to internal functionality Type: Cleanup Removes or deprecates API methods or behaviour labels May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Core Related to internal functionality Type: Cleanup Removes or deprecates API methods or behaviour
Projects
None yet
Development

No branches or pull requests

1 participant