[7u60] Request for review & approval for 8025512: NPE with logging while launching webstart on jre7u40 if logging is disabled

[Daniel Fuchs]

On 12/10/13 10:41 AM, Jaroslav Bachorik wrote:
> Hi Daniel,
>
> On 9.12.2013 14:46, Daniel Fuchs wrote:
>> Hi,
>>
>> This is a request for review & approval for
>> 8025512: NPE with logging while launching webstart on jre7u40
>>           if logging is disabled
>>
>> webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8025512/webrev.00/
>> bugid: https://bugs.openjdk.java.net/browse/JDK-8025512
>>
>> In some circumstances, an NPE can occur in logging initialization.
>>
>> This is because there is one place (in Logger.setParent) where access to
>> 'manager' is not guarded by a if (manager == null) - as is done
>> every where else.
>>
>> The fix is to copy over the code we already have for this method in
>> JDK 8, and do:
>>
>> if (manager == null) {
>>     manager = LogManager.getLogManager();
>> }
>
> Shouldn't be the null value sanitized directly in the constructor and
> the setter? There is already the same check in "checkPermissions()"

Hi Jaroslav,

checkPermission() also has a check for anonymous logger, and this is why
it cannot be called there.

As for the constructor - I believe the issue with manager == null
only happens for the global logger, which is created before the
LogManager is initialized - at a time when LogManager.getLogManager()
returns null.

When setParent() is called however the LogManager will be initialized
and LogManager.getLogManager() will no longer be null.
In JDK 8 I have completely revisited the initialization of Logger and
LogManager - which should make it more robust.
This is an area which is very sensitive and even in 8 I haven't dared
remove the calls to LogManager.getLogManager() or the guards against
manager == null.

best regards,

-- daniel


>
> -JB-
>
>>
>> The risk should be very limited. Not guarding for manager == null
>> in this place was an oversight.
>>
>> The test case will fail without the fix and pass with it.
>>
>>
>> best regards,
>>
>> -- daniel
>>
>