[7u10] Review request for 7188708 - REGRESSION: closed/java/awt/EventQueue/PostEventOrderingTest.java fails

[Artem Ananiev]

On 9/7/2012 4:39 PM, Oleg Pekhovskiy wrote:
> Hi David,
>
> I thought about that variant first, then something stopped me.
> Anyway, it resolves the regression.
>
> Please review the changes here:
> http://cr.openjdk.java.net/~bagiras/7u10/7188708.2/

This version looks fine.

Thanks,

Artem

> Thanks,
> Oleg
>
> 9/7/2012 3:36 PM, David Holmes wrote:
>> Hi Oleg,
>>
>> It seems to me that the original code has the
>>
>>  isFlushingPendingEvents = false;
>>
>> in the wrong place: it should only ever be cleared by an invocation
>> that set it. So a simple reorganization of the code would achieve that:
>>
>>  553     public static void flushPendingEvents()  {
>>  554         flushLock.lock();
>>  555         try {
>>  556             // Don't call flushPendingEvents() recursively
>>  557             if (!isFlushingPendingEvents) {
>>  558                 isFlushingPendingEvents = true;
>> +                     try {
>>     559                 AppContext appContext =
>> AppContext.getAppContext();
>>     560                 PostEventQueue postEventQueue =
>>     561 (PostEventQueue)appContext.get(POST_EVENT_QUEUE_KEY);
>>     562                 if (postEventQueue != null) {
>>     563                     postEventQueue.flush();
>>     564                 }
>> +                    } finally {
>> +                       isFlushingPendingEvents = false;
>> +                    }
>>  565             }
>>  566         } finally {
>> -567
>>  568             flushLock.unlock();
>>  569         }
>>  570     }
>>
>> David
>> -----
>>
>> On 7/09/2012 11:57 AM, Oleg Pekhovskiy wrote:
>>> Hi!
>>>
>>> Please review the fix for CR:
>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7188708
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~bagiras/7u10/7188708.1/
>>>
>>> The reason is that isFlushingPendingEvents in
>>> SunToolkit.flushPendingEvents() is reset
>>> after the first recursive call of flushPendingEvents(). Thus, if there
>>> are several pending events in PostEventQueue,
>>> recursion would be rejected only for the first event, allowing nested
>>> call of PostEventQueue.flush() after.
>>>
>>> To resolve the problem I added the counter "flushNestingLevel" instead
>>> of the flag "isFlushingPendingEvents".
>>> It's increased each time entering SunToolkit.flushPendingEvents() and
>>> decreased on exit.
>>> So, PostEventQueue.flush() method is called only when we enter
>>> SunToolkit.flushPendingEvents() for the first time
>>> within one thread.
>>>
>>> That fix was prepared ONLY for 7u10.
>>> For JDK 8 the fix for "CR7186109 - Simplify lock machinery for
>>> PostEventQueue & EventQueue" should cover this case.
>>>
>>> Thanks,
>>> Oleg
>