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

[Anthony Petrov]

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
>>>>>
>>>