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

[David Holmes]

On 26/07/2012 11:59 PM, Oleg Pekhovskiy wrote:
> it's thread-safe indirectly. SunToolkit.flushPendingEvents() &
> SunToolkitSubclass.flushPendingEvents() are the only methods that call
> PostEventQueue.flush().
> They both have flushLock synchronization object that guarantees
> synchronized call of PostEventQueue.flush().

Yes but it is that undocumented indirection that concerns me. If someone 
decides to remove those outer locks they may not realize there is a 
lurking race here.

David

> Thanks,
> Oleg
>
> 7/26/2012 4:08 PM, David Holmes wrote:
>> This fix still makes me uncomfortable because flush() is not
>> thread-safe any more. If two threads can flush() then the first can
>> clear the isFlushing state (in the finally block) that has been set by
>> the second (in the synchronized block).
>>
>> David
>>
>> On 24/07/2012 9:42 PM, Peter Levart wrote:
>>> Hi Oleg,
>>>
>>> This is just cosmetics, but:
>>>
>>> SunToolkit:
>>> public synchronized boolean noEvents() {
>>> return queueHead == null && !isFlushing;
>>> }
>>>
>>> ... a thread calling noEvents could see occasional "spikes" of false
>>> return even though there is no flushing being performed (when other
>>> thread is calling flush on an empty PostEventQueue).
>>>
>>> Improved flush method would look like this:
>>>
>>> public void flush() {
>>> EventQueueItem tempQueue;
>>> synchronized (this) {
>>> tempQueue = queueHead;
>>> queueHead = queueTail = null;
>>> isFlushing =*/(tempQueue != null)/*;
>>>
>>> }
>>> try {
>>> while (tempQueue != null) {
>>> eventQueue.postEvent(tempQueue.event);
>>> tempQueue = tempQueue.next;
>>> }
>>> }
>>> finally {
>>> isFlushing = false;
>>> }
>>> }
>>>
>>>
>>> Regards, Peter
>>>
>>> 2012/7/23 Oleg Pekhovskiy <oleg.pekhovskiy at oracle.com
>>> <mailto:oleg.pekhovskiy at oracle.com>>
>>>
>>> Hi!
>>>
>>> Please review this back-port being already pushed to jdk8 but
>>> deferred for 7u6.
>>>
>>> Bug:
>>> http://bugs.sun.com/view_bug.__do?bug_id=7177040
>>> <http://bugs.sun.com/view_bug.do?bug_id=7177040>
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~__bagiras/7u8/7177040.1
>>> <http://cr.openjdk.java.net/~bagiras/7u8/7177040.1>
>>>
>>> Review thread for 7u6:
>>> http://mail.openjdk.java.net/__pipermail/awt-dev/2012-July/__003106.html
>>> <http://mail.openjdk.java.net/pipermail/awt-dev/2012-July/003106.html>
>>>
>>> Reviewers 7u6 & 8:
>>> Anthony Petrov, Anton Tarasov
>>>
>>> Thanks,
>>> Oleg
>>>
>>>
>