Hi,
Please find below a candidate fix for:
8033661: readConfiguration does not cleanly reinitialize the
logging system
https://bugs.openjdk.java.net/browse/JDK-8033661
http://cr.openjdk.java.net/~dfuchs/webrev_8033661/webrev.00/
LogManager.readConfiguration() presents a number of flaws that may not
be fixable without endangering backward compatibility, or stability.
Some of them have been quite aptly summed up in JDK-8033661.
One of the main issue with readConfiguration() is that it calls reset().
This means that there's no way we can look at the previous
configuration and next configuration and close only those handlers
that the new configuration does not keep, leaving the others
unchanged.
I have tried experimenting with adding a per-logger
'handlersInitialized' flag, so that handlers could be created on-demand
when a logger first accesses them - as the root logger does, but rapidly
abandoned this idea due to the complexity of avoiding race conditions
and potential deadlocks.
There are also more complex cases that the current
readConfiguration() doen't take into account: when a new logger
is created by user/library code, the configuration is looked up and
any intermediate logger for which there is a configuration (level,
handler, useParentHandler) is created: this is done by
processParentHandler()).
However, readConfiguration() does not ensure that this also
happens for existing loggers: if a configuration (e.g a level,
or a handler) appears in the new configuration, for the parent of
an existing logger, but that parent does not exist yet - then this
piece of configuration will be ignored until the parent is created
explicitly by application/library code, which might be never.
This leaves the logger tree in an inconsistent state with regard to
the configuration loaded.
It seems that to fix that - we should call processParentHandler() on
all existing loggers.
I have the feeling that trying to fix all of this in
readConfiguration() could be dangerous WRT to the assumptions
that already existing code that calls readConfiguration()
may have on what readConfiguration() actually does.
If we wanted to fix readConfiguration() we therefore might need to
introduce yet another opt-in/opt-out flag for the new behavior.
All this leads to propose a more prudent approach.
I'm proposing to leave readConfiguration() unchanged, and instead
introduce a new updateConfiguration() method that would make it
possible to give more control to the caller on how the old and
new configuration should be merged.
The new updateConfiguration() method would take a
remapper function, allowing the caller to control how the old and
new configuration are going to be merged into a resulting configuration.
The method would ensure that loggers are properly initialized, given
the difference between their old & resulting configuration, calling
processParentHandlers when necessary.
The new method has the advantage that it would not call reset(), as
readConfiguration() did.
So if a logger has an identical configuration in the old & new conf,
it doesn't need to be changed at all. Its handlers don't need to be
closed and reopened - they can just be left alone.
The problems exhibited by JDK-8033661 could then be solved by
calling updateConfiguration() with a remapper function that,
for a given property 'p' with old value 'o' and new value 'n'
always return the new value 'n':
manager.updateConfiguration((p) -> ((o,n) -> n));
comments welcomed,
-- daniel
