Hi Mandy,
On 9/5/13 2:46 AM, Mandy Chung wrote:
On 9/4/2013 12:56 PM, Daniel Fuchs wrote:
Hi,
Please find below a changeset that will fix
8023168: Cleanup LogManager class initialization and
LogManager/LoggerContext relationship.
<http://cr.openjdk.java.net/~dfuchs/webrev_8023168/webrev.00/>
It is good to see the LogManager/Logger initialization untangled. This
helps in the long run.
LogManager.java L342: thanks to adding the assert here. L340-341 can
be removed.
This will be helpful in debugging (rather than swallowing the
exception). The initialization has been so fragile and swallowing the
exception makes the problem even worse. With this cleanup, I think
the initialization code will be more robust.
Will do.
The ensureLogManagerInitialized method does the rest of the
initialization for the global log manager - it reads the
configuration, instantiates and registers the root logger, and also
register the global logger. The comment L289-292 about deadlock
between two class initializers needs to be revise.
Will do.
I agree with David that it's worth exploring if lazy holder class
idiom would help here - perhaps a holder class with a static
rootLogger field.
Well, I am not too keen on using a lazy holder class idiom here.
Though it is true that ensureLogManagerInitialized will do nothing if
called before the 'manager' field
is set, or if called from additional instances of LogManager, I'd prefer
not to stick this code in yet
another static initializer. If that's acceptable - I'd prefer to use a
synchronized block within the
method and an additional volatile boolean to mark when initialization is
finished.
This should make it possible to avoid entering the synchronized block
once the LogManager
is initialized.
Since you touch these lines, it's good to switch to try-with-resource.
1205 final InputStream in = new FileInputStream(fname);
1206 final BufferedInputStream bin = new BufferedInputStream(in);
1207 try {
1208 readConfiguration(bin);
1209 } finally {
1210 in.close();
1211 }
OK.
Logger.java - getGlobal() method: would it help if the holder class
has static Logger global field which is set during its class
initialization. It should guarantee that global.manager is set
and the log manager initialization completes??
Can't do. Logger.global is a public final field.
-- daniel
I was wondering if this can be simplified.
thanks
Mandy