<AWT Dev> [9] Review request for 8031694: [macosx] TwentyThousandTest test intermittently hangs

[Artem Ananiev]

On 2/10/2014 9:17 PM, Anthony Petrov wrote:
> Thanks for the clarifications. Note that given that we re-create the EDT
> if there are more events in the queue, I'm still unsure whether we
> regress or not. I recall there was a patch submitted on this mailing
> list a few years ago that made the EDT die unconditionally and never be
> resurrected if it's requested to die. So I'm afraid the code that relies
> on this behavior will stop working correctly after your fix because you
> will re-create the EDT for all remaining events.

I can barely imagine such a code that kills EDT and expect it to have 
died forever :) This is a violation of AWT contract, when event 
dispatching is started as soon as a new event is posted. If applications 
needs another behavior, it has to make sure no events are posted to AWT 
event queue.

Oleg,

webrev .2 looks fine to me.

Thanks,

Artem

> What tests did you run with your fix?
>
> --
> best regards,
> Anthony
>
> On 2/7/2014 8:18 PM, Oleg Pekhovskiy wrote:
>> Hi Anthony,
>>
>> there are two points for choosing this solution:
>> 1. If something makes EDT to die, there is a serious reason to do so.
>> It's a forced action.
>> So it should be done ASAP. Dying EDT usage for pumping followed events
>> looks strange because we expect him to die.
>> Moreover it could happen that events are posted quite frequently to keep
>> dying EDT alive for some period of time.
>>
>> 2. Synchronization object for posting events from different threads is
>> EventQueue.pushPopLock.
>> it is used in EventQueue. postEventPrivate(), EventQueue.getNextEvent()
>> and EventQueue. detachDispatchThread().
>> When dying EDT returns from EventDispatchThread.pumpEventsForFilter() to
>> EventDispatchThread.run() and then goes to
>> getEventQueue().detachDispatchThread(), EventQueue.pushPopLock is not
>> used, so any other thread could post events.
>> So if we don't call peekEvent() to recreate a new EDT, we'll just loose
>> these events as it currently happens.
>>
>> So the main idea is to make EDT life cycle phases obvious.
>>
>> Thanks,
>> Oleg
>>
>> On 02/07/2014 06:48 PM, Anthony Petrov wrote:
>>> Hi Oleg,
>>>
>>> This code is very tricky. I like it that we process any events that
>>> might be posted to the queue after the current EDT dies. However,
>>> could you please clarify how initializing a new EDT is any different
>>> from not letting the old one die? I.e. could we just not kill the old
>>> EDT if we see there are more events in the queue? If not, what exact
>>> difference does you solution bring?
>>>
>>> It's not that I'm against your fix, it looks good actually. I'd just
>>> like to understand the difference. Please elaborate.
>>> Also, I recall we've fixed a number of bugs in this area. Are we sure
>>> we don't regress after this fix?
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>> On 2/7/2014 4:31 AM, Oleg Pekhovskiy wrote:
>>>> Hi all,
>>>>
>>>> please review the next version of fix:
>>>> http://cr.openjdk.java.net/~bagiras/8031694.2/
>>>>
>>>> We with Artem Ananiev had off-line discussion and he offered let the
>>>> dying EDT to die
>>>> and process unhandled events by forcing another EDT start.
>>>>
>>>> Thanks,
>>>> Oleg
>>>>
>>>> On 01/28/2014 05:32 AM, Oleg Pekhovskiy wrote:
>>>>> Hi all,
>>>>>
>>>>> please review the fix
>>>>> http://cr.openjdk.java.net/~bagiras/8031694.1/
>>>>> for
>>>>> https://bugs.openjdk.java.net/browse/JDK-8031694
>>>>>
>>>>> During forward-port of JDK-7189350 EDT.doDispatch was not taken into
>>>>> account when calling EventQueue.detachDispatchThread().
>>>>> As a result harmful optimization of this method occurred.
>>>>> So when doDispatch became false, no more events in QventQueue were
>>>>> handled before EDT shutdown.
>>>>> I kept the optimization but added the check to
>>>>> EDT.pumpEventsForFilter() that EventQueue is not empty to keep
>>>>> pumping.
>>>>>
>>>>> Thanks,
>>>>> Oleg
>>>>
>>