On Wed, 11 Mar 2026 11:08:14 GMT, Daniel Fuchs <[email protected]> wrote:
> The `ParentLoggerTest` is supposed to verify that when a logger is created > programatically, any parent logger for which a level or handler is defined in > the configuration will be created along if not already created. > > In the `LogManager` class, the method `addLocalLogger` calls the > `processParentHandlers` method to process the parent hierarchy and potentialy > create any loggers from that hierarchy that is defined in the configuration > but that has not been created yet, adding them to the logger tree. However, > these loggers newly created by `processParentHandlers` are only weakly > referenced by the tree, until `addLocalLogger` later sets-up the `parent` > field in their children (a strong reference to the parent logger). There is > therefore a short time window where loggers defined in the configuration can > be lazily created by `processParentHandlers` but immediately garbage > collected before the strong reference that should keep them alive when their > child is strongly referenced has been set. > > The `java/util/logging/ParentLoggersTest.java` has been observed failing > intermitently due to that, first in the loom repo, and now again in the > valhalla repo. > > I was able to reproduce the failure 100% of the time by adding a call to > `System.gc()` in the `addLocalLogger` method, calling it in a loop, waiting > until a weak reference was cleared, right after calling > `processParentHandlers` but before starting setting up the child/parent > relationship. > > The fix proposed here makes uses of the `visited` predicate that can be > passed to the `processParentHandlers` method. > `addLocalLogger` now passes a special `visited` predicate that will save the > newly created loggers in a local array list, instead of passing a predicate > that only always returns false as it was before. > That array list is then cleared after the child/parent relationships have > been updated. > This ensures that parent loggers are kept alive until they are strongly > referenced by their child. > > With the fix, and using the modified `addLocalLogger` that calls `System.gc`, > the issue no longer reproduces. > > Testing this reliably requires a modification of the `LogManager` that we > can't keep in the product code, so I believe we will unfortunately have to > rely on the existing `ParentLoggersTest.java` to continue verifying this > issue is fixed. The changes look good to me. ------------- Marked as reviewed by jpai (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/30185#pullrequestreview-3931201309
