On 04/30/2015 07:42 AM, Daniel Fuchs wrote:
http://cr.openjdk.java.net/~dfuchs/webrev_8077846/webrev.00/
http://cr.openjdk.java.net/~plevart/misc/LogManager.synchronization/webrev.02/
I reviewed both of the above webrevs.
TestConfigurationLock.java
Copyright year should be 2015
TestConfigurationLock.properties
It would be good to delete all commented lines except the lines
relevant to the setting to make it obvious what the configuration is.
LogManager.java
I actually like having one state variable to describe the global handler
(re-)initialization that to me is a simplification. I couldn't
immediately match the constant names in following the code right away.
I read these states as:
STATE_UNINITIALIZED - request reinitialization or invalidate the
RootLogger
(called from readConfiguration and after reset)
STATE_INITIALIZED - reinitialization complete or error during
readConfiguration
STATE_INITIALIZING - global handlers are being initialized
Perhaps renaming the constant names to GLOBAL_HANDLERS_INVALIDATED etc
might help.
I'm less sure about shutdown and the global handlers can skip
reinitialization during shutdown and keeping it as separate variable
makes it more explicit. It may eliminate the configurationLock.lock()
in the Cleaner.run() method.
A minor point - I wonder if a private reset method taking a boolean
argument to indicate if the state change to STATE_INITIALIZED is
required would make it little easier to follow.
Mandy
