Hi Mandy,
Thanks for looking into this.
On 22/01/2018 23:52, mandy chung wrote:
So the cause of JDK-8191033 is because cx::addLocalLogger returns false
when addLogger is called the first time to add the root and global
logger and it causes that ".handlers" is not read and loaded for the
root logger.
Yes. "global" suffers the same fate.
It took me some time to understand what it's happening.
When addLocalLogger is called to add a logger, it calls
ensureAllDefaultLoggers to ensure the root and global loggers are added
first. Do you think the handling of ".handlers" should be done in
LoggerContext::ensureDefaultLogger? Would that avoid the check to call
loadLoggerHandlers if it's called from ensureLogManagerInitialized?
The code is getting quite complicated and if moving it closer to the
place it adds the root logger would help.
I did give it some consideration. I think this would be a mistake.
The whole point of this fix is that loadLoggerHandlers *must* be
called *within* addLogger - so that a subclass of LogManager that
overrides addLogger can take over.
The fact that loadLoggerHandlers was called outside of
addLogger was what caused the issue with ClassLoaderLogMaanger.
In 10 we see that the root logger is added to the context outside
of the call to addLogger, so doing what you suggest would - I
think - trigger the issue again.
In addition the lazy code that ensure that the root and global
loggers are added first to the contexts is complex enough - I
would be very reluctant to fiddle too much with it.
The condition in the / else if / can be true only once
when addLogger is called after reading the primordial
configuration - so I believe the fix is sound.
The other possibility would be to manage a state for the creation
of handlers (from name + ".handlers") for both root & global, but
I believe this would be more complicated.
best regards,
-- daniel
On 1/22/18 8:47 AM, Daniel Fuchs wrote:
http://cr.openjdk.java.net/~dfuchs/webrev_8195096/webrev.01/
