RFR: 8077846: improve locking strategy for readConfiguration(), reset(), and initializeGlobalHandlers()

[Peter Levart]

On 04/28/2015 05:46 PM, Peter Levart wrote:
>
>
> On 04/28/2015 04:57 PM, Daniel Fuchs wrote:
>>> Here's my attempt at simplifying this:
>>>
>>> http://cr.openjdk.java.net/~plevart/misc/LogManager.synchronization/webrev.01/ 
>>>
>>
>> LogManager can be subclassed, and subclasses may override reset() for
>> different purposes.
>> So I'm afraid the Cleaner thread still needs to call te public
>> reset() method. The same unfortunately applies to
>> readConfiguration().
>>
>> best regards,
>>
>> -- daniel 
>
> Um, yes of course. This can be fixed:
>
> http://cr.openjdk.java.net/~plevart/misc/LogManager.synchronization/webrev.02/ 
>
>
> Regards, Peter
>

Hi Daniel,

Just some observations...

I noticed you changed LogManager.Cleaner superclass from 
ManagedLocalsThread -> Thread. Is that intentional? The Cleaner is an 
unstarted Thread that gets registered as a shutdown hook in LogManager 
constructor. So at LogManager construction time it gets hold on 
inheritable thread locals and keeps them for the entire JVM lifetime. Do 
we have control on what thread performs LogManager construction and that 
no inheritable thread-local leaks are possible?

The other thing to note is that Cleaner class is not static, so it has 
an implicit reference to LogManager instance. So each LogManager 
instance constructed during lifetime of JVM is actually retained for the 
entire JVM lifetime.

The use of shutdown hook to call reset() directly is questionable. Why? 
Because other shutdown hooks might want to log their shutdown processing 
and they will race with LogManager shutdown hook executing reset(), 
shutting down all log handlers and effectively preventing shutdown 
process form being logged. Why is it so important to reset() 
LogManager(s) before exiting VM?

Regards, Peter