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

Oleg Pekhovskiy oleg.pekhovskiy at oracle.com
Thu Jul 12 16:28:19 PDT 2012


No probs :)

Thank you,
Oleg.

13.07.2012 3:25, David Holmes wrote:
> On 13/07/2012 8:13 AM, Oleg Pekhovskiy wrote:
>> Hi David,
>>
>> the condition you mentioned:
>>
>> if (!forceDetach && (peekEvent() != null) ||
>> !SunToolkit.isPostEventQueueEmpty()) {
>> return false;
>> }
>>
>> could lead to understanding problem (braces are the best way to 
>> clarify).
>>
>> && has higher priority than ||, thus we could rewrite that condition
>> like this:
>>
>> ( !forceDetach && (peekEvent() != null) ) ||
>> !SunToolkit.isPostEventQueueEmpty()
>
> Sorry - need to clean my glasses. I was seeing !f && ( X || Y)
>
> :(
>
>> That means SunToolkit.isPostEventQueueEmpty() is called when:
>> forceDetach == true
>> OR
>> peekEvent() == null
>> OR
>> both above.
>>
>> Anthony has pushed the fix that changes (and simplifies) that place:
>> http://cr.openjdk.java.net/~anthony/7u6-16-missingAWTThread-7162144.0/
>> So maybe we should discuss the new code - the new behavior.
>
> Yes I saw that patch too. All clear now.
>
> Thanks,
> David
>
>
>> Thanks,
>> Oleg
>>
>> 7/13/2012 1:10 AM, David Holmes wrote:
>>> On 12/07/2012 10:18 PM, Anthony Petrov wrote:
>>>> On 07/12/12 05:41, David Holmes wrote:
>>>
>>> <snip>
>>>
>>>>> It also seems that with this fix the interrupted EDT will not detach
>>>>> due
>>>>> to the flush being in progress. The EDT will resume the pumpEvents 
>>>>> loop
>>>>> in its run() method. If the interrupt state of the EDT has not been
>>>>> maintained then the fact it was interrupted (and should detach?) 
>>>>> is now
>>>>> lost. If it is maintained then presumably we will try to
>>>>> detachDispatchThread again, in which case the EDT will enter a busy
>>>>> polling loop trying, but failing, to detach, until the flush() has
>>>>> actually completed.
>>>>
>>>> When interrupt() is called on EDT, the shutdown flag is set, which is
>>>> subsequently passed as the forceDetach parameter to
>>>> detachDispatchingThread(). If the parameter is true, the detaching
>>>> happens unconditionally. So this shouldn't be an issue.
>>>
>>> I don't quite follow this part. The logic is:
>>>
>>> if (!forceDetach && (peekEvent() != null) ||
>>> !SunToolkit.isPostEventQueueEmpty()) {
>>> return false;
>>> }
>>>
>>> If forceDetach is true then we won't execute peekEvent() and
>>> isPostEventQueueEmpty(), which means we would not have hit the
>>> original deadlock.
>>>
>>> Oleg's response seems to indicate that we will indeed keep looping but
>>> because of the values of various flags we won't do anything "useful".
>>> But that is the kind of "busy wait" that I was referring to.
>>>
>>> Cheers,
>>> David
>>>
>>>
>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>>
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>>
>>>>>> 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
>>>>>>
>>




More information about the awt-dev mailing list