<AWT Dev> [7u6] Review request for 7177040: Deadlock between PostEventQueue.noEvents, EventQueue.isDispatchThread and SwingUtilities.invokeLater

[Anthony Petrov]

Thanks for addressing the issues. The fix looks fine now.

--
best regards,
Anthony

On 7/11/2012 8:08 PM, Oleg Pekhovskiy wrote:
> Thank you for the review, Anthony,
> 
> please review the next version of fix:
> http://cr.openjdk.java.net/~bagiras/7u6/7177040.2
> 
> I followed your suggestions there.
> I also added 'volatile' for isFlushing because it could be accessed from 
> different threads.
> 
> Thanks,
> Oleg
> 
> 7/11/2012 7:35 PM, Anthony Petrov wrote:
>> Hi Oleg,
>>
>> 1. I suggest to rename isPending to isFlushing, as it reflects the 
>> current state of the PostEventQueue more precisely.
>>
>> 2. I suggest to use the try{}finally{} pattern to set/reset the new 
>> flag in the flush() method to ensure the flag is reset even if the 
>> while() loop throws an exception.
>>
>> Otherwise the fix looks good.
>>
>> -- 
>> best regards,
>> Anthony
>>
>> On 7/11/2012 7:24 PM, Oleg Pekhovskiy wrote:
>>> Hi!
>>>
>>> Please review the fix for CR:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7177040
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~bagiras/7u6/7177040.1
>>>
>>> The idea of the fix is that there are two concurrent threads that try 
>>> to get two synchronization objects EventQueue.pushPopLock and 
>>> PostEventQueue itself.
>>> For NetBeans these threads are (EDT & WarmUp) or (EDT & TimerQueue).
>>> Problem happens when EDT is interrupted and goes to 
>>> EventQueue.detachDispatchThread() where EventQueue.pushPopLock is 
>>> owned and
>>> EDT is waiting to own PostEventQueue when calling 
>>> SunToolkit.isPostEventQueueEmpty() -> PostEventQueue.noEvents().
>>> At the same time another thread calls EventQueue.postEvent() that 
>>> calls EventQueue.flushPendingEvents() -> PostEventQueue.flush() where 
>>> PostEventQueue is owned
>>> and another thread is waiting to own EventQueue.pushPopLock when 
>>> calling EventQueue.postEvent() -> EventQueue.postEventPrivate().
>>>
>>> To avoid potential deadlock I moved synchronization out of 
>>> postEvent()'s cycle in PostEventQueue.flush(),
>>> but to be clear about the existence of Events that are not posted to 
>>> EventQueue yet I added PostEventQueue.isPending flag that is true 
>>> until the end of the cycle.
>>>
>>> There are only two classes that utilize PostEventQueue.flush() 
>>> method: SunToolkit.flushPendingEvents() and 
>>> SunToolkitSubclass.flushPendingEvents().
>>> They are both synchronized by static SunToolkit.flushLock. That 
>>> eliminates the situation when we have overlapped calls of 
>>> PostEventQueue.flush().
>>>
>>> Thanks,
>>> Oleg
>