RFR: 8023168 - Cleanup LogManager class initialization and LogManager/LoggerContext relationship

[Mandy Chung]

On 9/5/2013 11:51 AM, Daniel Fuchs wrote:
>
> <http://cr.openjdk.java.net/~dfuchs/webrev_8023168/webrev.02/>

Logger.java - I like this and much cleaner fix.

  251         // because global.manager will become not null somewhen during
  252         // the initialization of LogManager.

s/somewhen/somewhere/

The comment for the setLogManager method needs updated since it's
called when a logger is registered to a log manager.

  347     // It is called from the LogManager.<clinit> to complete
  348     // initialization of the global Logger.

The checkPermission() method has this code to deal with null manager:
            if (manager == null) {
                 // Complete initialization of the global Logger.
                 manager = LogManager.getLogManager();
             }

I think it's still possible that checkPermission is called with
null manager, right?  e.g. calling Logger.global.setFilter

Should this simply call LogManager.getLogManager() unconditionally as
in getGlobal()?

LogManager.java - the initialization is really tricky and the discussion
on the ensureLogManagerInitialized is very useful.
  
  159     private volatile Logger rootLogger; // non final field - make it volatile to
  160             // make sure that other threads will see the new value once
  161             // ensureLogManagerInitialized() has finished executing.

suggest to move the comments above the declaration
  
Maybe renaming "initializedCalled" to "recursiveInit" to make the 2
variables easy to tell.  space missing around "=" in line 270, 271

  310                         // Read configuration. This was previously triggered
  311                         // by the new RootLogger() constructor - but no longer.

Previously readPrimordialConfiguration() wascalled by LogManager.getLogManager
that was triggered when instantiating the RootLogger - to be precise.
It seems that leaving out the history might cause less confusion
since it no longer calls it.

  760             final LogManager owner = getOwner();
  761             logger.setLogManager(owner);

Should this have an assert to ensure logger.manager == null or == owner?
We don't expect a Logger to change its owner, do we?  The behavior of multiple
LogManager instances is not specified anyway.

Reading the RootLogger - looks like it's potential for cleanup.
Logger.global uses a private constructor that doesn't call
LogManager.getLogManager to break the cyclic dependency.  Perhaps
RootLogger should do the same?  It's fine to leave it for future
if you prefer since you have well tested the bits you have.

Mandy