RFR: 8065991: LogManager unecessarily calls JavaAWTAccess from within a critical section

[Mandy Chung]

On 12/2/14 12:26 PM, Daniel Fuchs wrote:
> Hi Mandy,
>
> On 02/12/14 20:24, Mandy Chung wrote:
>> Hi Daniel,
>>
>> On 11/26/14 9:11 AM, Daniel Fuchs wrote:
>>> Hi,
>>>
>>> Please find below a patch for:
>>>
>>> 8065991: LogManager unecessarily calls JavaAWTAccess from within
>>>          a critical section
>>> https://bugs.openjdk.java.net/browse/JDK-8065991
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8065991/webrev.00/
>>>
>> line 465 can be moved together with line 461.
>
> Good point. It should not matter much however.
>

For non-applet case, it will also be null (just a fast path).

>> For the logger created by EventQueue in non-applet env, do we
>> expect JavaAWTAccess.getAppletContext to return null (as it
>> should only find the main AppContext)?
>
> The mainAppContext is equivalent to 'null' for us.
>
>> As the deadlock reported from JDK-8065709, it hits the race when
>> main AppContext object has been created and the numAppContexts counter
>> has been incremented but mainAppContext is not set in which case
>> even with this patch, it will still call AppContext.getAppContext()
>> and hit that deadlockon sun.awt.AppContext$GetAppContextLock.
>
> I'm not sure I follow. I don't think it will. Only when the
> LogManager has not yet been initialized - because getUserContext()
> ends up being called inside the lock held by 
> ensureLogManagerInitialized().
>

I was referring to the original deadlock.

> This patch doesn't pretend to solve JDK-8065709. This patch simply
> removes one unnecessary lock around getAppContext().

I understand.
>
>> Should JavaAWTAccess.getAppletContext simply return null (not calling
>> getAppContext) if it determines the main AppContext is being 
>> initialized?
>
> I'd prefer not to touch this code unless it's proven wrong.
>

That's fair.  The real fix is to fix the synchronization issues with 
AppContext.
> To solve the actual deadlock - several options have been proposed - and
> I think they should be preferred - or at least experimented.
>

It's fine with me.  Your patch looks good to go.
Mandy