Feature #19440
openDeprecate ThreadGroup
Description
I looked at the ThreadGroup methods, and none of them seem really useful.
enclose
has super confusing semantics, in that it surprisingly allows creating new threads from a thread already in that group, but forbids #add
.
So the new thread is not the same as "adding" a thread, very confusing.
This weird restriction results for instance in Timeout
not being able to move the Thread it creates to the default ThreadGroup:
https://github.com/ruby/timeout/issues/24
And there is also no method to create a Thread in the given ThreadGroup.
add
and list
could just be simulated with an Array.
Maybe it's time we deprecate ThreadGroup or at least enclose
/enclosed?
since those seem to have unusable/useless semantics?
If not, how do we cleanly solve https://github.com/ruby/timeout/issues/24?
Updated by ioquatix (Samuel Williams) over 1 year ago
I don't think I have ever seen ThreadGroup
used in practice, nor do I personally know why I'd want to use it. Therefore, I'd be okay with deprecating it.
Updated by mame (Yusuke Endoh) over 1 year ago
- Related to Bug #19020: Unexpected timeout thread appears in ThreadGroup added
Updated by mame (Yusuke Endoh) over 1 year ago
Discussed at the dev meeting.
Many committers agree that ThreadGroup is not very useful, but @matz (Yukihiro Matsumoto) did not approve of deprecating it because it is actually used by many gems.
#19020 explained only the phenomenon, but didn't explain how the problem is troubling in a real program. Assuming that the problem is to pollute the ThreadGroup to which the calling Thread belongs, @nobu (Nobuyoshi Nakada) suggested timeout.rb to create a dedicated ThreadGroup and to make its timer thread belong there (as follows), instead of making the thread belong to ThreadGroup::DEFAULT.
-ThreadGroup::Default.add(watcher)
+ThreadGroup.new.add(watcher)
Updated by Eregon (Benoit Daloze) over 1 year ago
mame (Yusuke Endoh) wrote in #note-3:
#19020 explained only the phenomenon, but didn't explain how the problem is troubling in a real program. Assuming that the problem is to pollute the ThreadGroup to which the calling Thread belongs, @nobu (Nobuyoshi Nakada) suggested timeout.rb to create a dedicated ThreadGroup and to make its timer thread belong there (as follows), instead of making the thread belong to ThreadGroup::DEFAULT.
-ThreadGroup::Default.add(watcher) +ThreadGroup.new.add(watcher)
This would not work. Because ThreadGroup.new.add(watcher)
will fail due to the timeout Thread being created initially in the airbrake's enclosed ThreadGroup (see https://github.com/ruby/timeout/issues/24).
So currently there is no way to put the the Timeout thread in the correct group if there is an enclosed ThreadGroup.
Updated by nobu (Nobuyoshi Nakada) over 1 year ago
Even if ThreadGroup
were to be deprecated in 3.3, it can't help previous versions with the current timeout.rb.
Updated by Eregon (Benoit Daloze) over 1 year ago
Yeah I know.
timeout.rb already has this workaround: https://github.com/ruby/timeout/pull/25/files
If ThreadGroup#enclose
becomes deprecated then usages of enclose
will become increasingly rarer and so having that workaround matters much less.