tl;dr; Should we be calling ThreadGroup.setMaxPriority in our ThreadGroups? Any other thoughts on consistency in $subject?

Tomas Hofman has been doing some work cleaning up memory leaks that happen when reload is called on a server, and one small one is ThreadGroup objects leak. This is a small leak, as a ThreadGroup holds very little state if all its threads have terminated. But it's good to clean things up, which has led to a couple PRs:

https://github.com/wildfly/wildfly/pull/17192
https://github.com/wildfly/wildfly/pull/17193

Thinking about those got me thinking about how we use ThreadGroup in WF and WF Core in general, and whether we should be more standard about it. Rather than just discussing this on those PRs I figured I'd post here.

The first PR stores the TGs in constants. This works, but an alternative is calling TG.setDaemon(true) on the thread group. Note that calling that doesn't set a default for Thread.isDaemon() for the group's threads; all it does is allow the TG to be terminated once all its threads terminate.

Which of these seems more appropriate depends on the intended lifecycle for the threads; i.e. whether the expected thread count is at least one for the lifetime of the group. Tomas' static field approach is more robust about this.  Calling setDaemon(true) on a TG stored in a constant is almost certainly wrong.

A bit more interesting is Matej's approach in #17193. Here the TG is being discarded. The effect of this is that the TG for the threads created by the factory will be the TG of the thread that called the JBossThreadFactory constructor. So, in this case, the TG of the MSC thread pool threads.

The basic downside of this approach is the code creating the JBossThreadFactory has no idea what the settings of the current thread's TG are and whether they are appropriate. It just assumes it's ok. However, Matej's change doesn't actually change anything in this regard, since if you create a new ThreadGroup and don't call setMaxPriority, your new TG inherits the max priority from the current thread's TG.

It seems like if we create a new TG, we should call setMaxPriority on it, unless we know what it has is ok.

Best regards,
Brian