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

[David Holmes]

That looks okay to me - it correctly prevents recursive calls.

David

On 7/09/2012 10: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/
>
> 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
>