On Wed, 11 Mar 2026 17:15:23 GMT, Jaikiran Pai <[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. Thanks @jaikiran ! ------------- PR Comment: https://git.openjdk.org/jdk/pull/30185#issuecomment-4040877011
