Use of ThreadGroup in WildFly and WildFly Core
by Brian Stansberry
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
9 months, 3 weeks
New Proposal: Move BOMs to the Server's source repository
by Eduardo Martins
Hello everyone, I would like to share a new proposal, which in short is about moving the sources of Wildfly BOMs to the WildFly repository.
The main reason for this move is to stop the recurrent out of sync issues between both repos, where changes merged on WildFly require changes on BOMs too, leading to the need of merging WildFly SNAPSHOT dependencies. The following are recent e3examples of such issues:
https://issues.redhat.com/browse/WFLY-18043
https://issues.redhat.com/browse/WFLY-18155
https://issues.redhat.com/browse/WFLY-17879
After the move WildFly will fail to build if a submitted PR changes the WildFly dependency management, but don’t include any required changes in BOMs too.
Another major reasons are the simplified maintenance, release process, and of course, one less repository in the dependency tree of apps.
You may find the WildFly Proposal document at https://github.com/wildfly/wildfly-proposals/pull/539.
Please note that there are still two major TODOs in the document, which I expect to resolve as a team during this week:
We need to identify all platform dependencies on WildFly BOMs, to specify any changes needed on the dependents <https://github.com/wildfly/wildfly-proposals/pull/539/files#diff-21f7c387...>
We need to identify all community documentation references to the WildFly BOMs source repository, to replace those with references to the new sources location <https://github.com/wildfly/wildfly-proposals/pull/539/files#diff-21f7c387...>
You may also find already the PR implementing this proposal at https://github.com/wildfly/wildfly/pull/17201.
Please let us know your thoughts on this proposal, any feedback is welcome. The plan is to move forward with this in one week, if no objections.
—E
10 months, 1 week
