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

[Oleg Pekhovskiy]

Thank you, guys!

Regards,
Oleg

On 12.02.2014 21:52, Anthony Petrov wrote:
> Oleg, Artem, thank you for the clarifications.
>
> I'm fine with the webrev version .2, too.
>
> --
> best regards,
> Anthony
>
> On 2/12/2014 4:58 PM, Artem Ananiev wrote:
>>
>> 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
>>>>>>
>>>>