<AWT Dev>  Review request for 8031694: [macosx] TwentyThousandTest test intermittently hangs
artem.ananiev at oracle.com
Wed Feb 12 04:58:30 PST 2014
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
webrev .2 looks fine to me.
> What tests did you run with your fix?
> best regards,
> 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
>> 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.
>> 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,
>>> On 2/7/2014 4:31 AM, Oleg Pekhovskiy wrote:
>>>> Hi all,
>>>> please review the next version of fix:
>>>> 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.
>>>> On 01/28/2014 05:32 AM, Oleg Pekhovskiy wrote:
>>>>> Hi all,
>>>>> please review the fix
>>>>> 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
More information about the awt-dev