<AWT Dev> OpenJdk11-28-EA JDialog hanging
Martin Balao
mbalao at redhat.com
Mon Oct 15 07:59:30 UTC 2018
Hi Laurent,
Thanks for your feedback and happy to hear that it worked!
In regards to pendingEvents sync, my understanding is that SequencedEvent
objects will be posted to one EventQueue only and, thus, be dispatched by
one EDT.
I had a quick look at "Exception in thread "AWT-EventQueue-1"
java.lang.IllegalArgumentException: null source" exception and could not
find any connection to the code modified in the context of this bug.
Apparently, "activeWindow" variable in "WindowEvent.WINDOW_LOST_FOCUS" case
is null (DefaultKeyboardFocusManager.dispatchEvent function). I thought
that this was caused by the fact that the test is injecting events that
modify the focus on both windows at a very high rate and, by the time
DefaultKeyboardFocusManager
dispatches the event, it could be too late. Just an hypothesis.
Look forward to your re-written test.
Kind regards,
Martin.-
On Fri, Oct 12, 2018 at 6:03 PM, Laurent Bourgès <bourges.laurent at gmail.com>
wrote:
> Martin,
>
> The reproducer test works now on OpenJDK12 + 8204142.webrev.01 patch !
> I also tested with 4 windows and it is OK.
>
> One minor bug that appears in my logs, it is certainly related to your fix
> as I never had that message on JDK8:
> Exception in thread "AWT-EventQueue-1" java.lang.IllegalArgumentException:
> null source
> at java.base/java.util.EventObject.<init>(EventObject.java:56)
> at java.desktop/java.awt.AWTEvent.<init>(AWTEvent.java:317)
> at java.desktop/java.awt.event.ComponentEvent.<init>(
> ComponentEvent.java:120)
> at java.desktop/java.awt.event.WindowEvent.<init>(
> WindowEvent.java:206)
> at java.desktop/java.awt.event.WindowEvent.<init>(
> WindowEvent.java:251)
> at java.desktop/java.awt.DefaultKeyboardFocusManager.dispatchEvent(
> DefaultKeyboardFocusManager.java:820)
> at java.desktop/java.awt.Component.dispatchEventImpl(
> Component.java:4889)
> at java.desktop/java.awt.Container.dispatchEventImpl(
> Container.java:2321)
> at java.desktop/java.awt.Window.dispatchEventImpl(Window.java:2762)
> at java.desktop/java.awt.Component.dispatchEvent(Component.java:4840)
> at java.desktop/java.awt.EventQueue.dispatchEventImpl(
> EventQueue.java:772)
> at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
> at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
> at java.base/java.security.AccessController.doPrivileged(Native
> Method)
> at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.
> doIntersectionPrivilege(ProtectionDomain.java:85)
> at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.
> doIntersectionPrivilege(ProtectionDomain.java:95)
> at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
> at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:743)
> at java.base/java.security.AccessController.doPrivileged(Native
> Method)
> at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.
> doIntersectionPrivilege(ProtectionDomain.java:85)
> at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
> at java.desktop/java.awt.SequencedEvent.dispatch(
> SequencedEvent.java:201)
> at java.desktop/java.awt.EventQueue.dispatchEventImpl(
> EventQueue.java:770)
> at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:721)
> at java.desktop/java.awt.EventQueue$4.run(EventQueue.java:715)
> at java.base/java.security.AccessController.doPrivileged(Native
> Method)
> at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.
> doIntersectionPrivilege(ProtectionDomain.java:85)
> at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.
> doIntersectionPrivilege(ProtectionDomain.java:95)
> at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:745)
> at java.desktop/java.awt.EventQueue$5.run(EventQueue.java:743)
> at java.base/java.security.AccessController.doPrivileged(Native
> Method)
> at java.base/java.security.ProtectionDomain$JavaSecurityAccessImpl.
> doIntersectionPrivilege(ProtectionDomain.java:85)
> at java.desktop/java.awt.EventQueue.dispatchEvent(EventQueue.java:742)
> at java.desktop/java.awt.EventDispatchThread.pumpOneEventForFilters(
> EventDispatchThread.java:203)
> at java.desktop/java.awt.EventDispatchThread.pumpEventsForFilter(
> EventDispatchThread.java:124)
> at java.desktop/java.awt.EventDispatchThread.pumpEventsForHierarchy(
> EventDispatchThread.java:113)
> at java.desktop/java.awt.EventDispatchThread.pumpEvents(
> EventDispatchThread.java:109)
> at java.desktop/java.awt.EventDispatchThread.pumpEvents(
> EventDispatchThread.java:101)
> at java.desktop/java.awt.EventDispatchThread.run(
> EventDispatchThread.java:90)
>
> Congratulations,
> Laurent
>
> Le ven. 12 oct. 2018 à 17:24, Laurent Bourgès <bourges.laurent at gmail.com>
> a écrit :
>
>> Hi Martin,
>>
>> What an amazing effort !
>> I am very pleased that my (crazy) test triggered a very complicated case;
>> I will try asap if it works with your new webrev.
>>
>> Thanks for your feedback and sharing your test.
>>>
>>> I've taken your suggestion of making the filter class final. In regards
>>> to having a single instance of the filter class, it would have been a good
>>> idea but now we have some additional requirements you'll see below.
>>>
>>> I could reproduce your deadlock. This deadlock scenario is a bit more
>>> interesting than the one fixed in Webrev00.
>>>
>>> At some point, we may reach the following state:
>>>
>>> SequenceEvent.list: EV0-AppContext1, EV0-AppContext0, EV1-AppContext0
>>> EventQueue0: EV0-AppContext0, EV1-AppContext0
>>>
>>> EDT0 now takes EV0-AppContext0 out of EventQueue0. When dispatching this
>>> event, it notices that it's not the first one in SequenceEvent.list and
>>> proceeds to dispatch a new event from EventQueue0 while waiting. It gets
>>> EV1-AppContext0 and again, it cannot continue because EV0-AppContext1 is
>>> still the first one in SequenceEvent.list. EDT0 now waits for either a
>>> SentEvent (sent by EDT1) or for a new SequencedEvent in EventQueue0.
>>>
>>> Let's say that EDT1 dispatches EV0-AppContext1 and sends a SentEvent to
>>> EDT0. EDT0 wakes up (enabled at Webrev00) and tries to dispatch
>>> EV1-AppContext0. However, EV1-AppContext0 is not the first one in
>>> SequenceEvent.list so it goes to sleep again (waiting for a SentEvent or a
>>> new SequencedEvent to come).
>>>
>>> However, EDT0 is the only one that can unlock this by dispatching
>>> EV0-AppContext0. This is not possible because EV0-AppContext0-dispatch call
>>> is down the stack. There is a stack frame on top of it: the one for
>>> EV1-AppContext0-dispatch. It's an inversion of the order.
>>>
>>> We don't want EDT0 to take a new SequencedEvent out of EventQueue0 (that
>>> is: creating a new dispatch frame on the call stack) if all events in
>>> SequenceEvent.list previous to the currently SequencedEvent event being
>>> dispatched are from a different AppContext.
>>>
>>> My first approach to this was: check SequenceEvent.list before going to
>>> sleep and see if we need to filter SequencedEvent events or not. However,
>>> this does not work for 2 reasons:
>>>
>>> 1) Events in SequencedEvent.list which are not being dispatched have a
>>> null AppContext. The only way of knowing if they belong to a specific
>>> AppContext is by iterating the EventeQueue queue and checking object
>>> identity. No APIs for this. On the other hand, assigning an AppContext when
>>> creating SequencedEvent events may break things (i.e.: creating events on a
>>> TG and posting them to a different one).
>>>
>>> 2) We cannot loose SequencedEvent events. Of course.
>>>
>>> What I propose to do is capturing out-of-order SequencedEvents events in
>>> the filter and releasing them once we dispatch the frame. In other words,
>>> try to dispatch only if the event that arrives is from a previous position
>>> in SequenceEvent.list (so we won't block).
>>>
>>> In addition to these changes, I propose to dispatch any
>>> non-SequencedEvent event while waiting. There is no need to block other
>>> events, and the GUI becomes unresponsive under a stress case such as the
>>> test otherwise.
>>>
>>> Removed an unnecessary "wakeup" on SequencedEvent.dispose function.
>>>
>>> Webrev 01:
>>>
>>> * http://cr.openjdk.java.net/~mbalao/webrevs/8204142/8204142.webrev.01/
>>> * http://cr.openjdk.java.net/~mbalao/webrevs/8204142/
>>> 8204142.webrev.01.zip
>>>
>>
>> I had a quick look to the patch.
>> I noticed that the list pendingEvents is not synchronized and I wonder
>> if multiple EDT threads may be working on the same SequencedEvent:
>> - currentSequencedEvent.pendingEvents.add(ev);
>> - for(AWTEvent e : pendingEvents) { SunToolkit.postEvent(appContext, e);
>> }
>>
>> I suppose it should never happen, so synchronization is useless !
>>
>> @Laurent: is it possible to tweak your test a bit to remove internal API
>>> usage and include an assertion? I don't know if you have tried that before,
>>> but it would be very nice so we can include your test in the patch. One
>>> additional comment regarding the test: the 10 ms lapse between events
>>> injection it is too fast for dispatching to keep up with -at least in my
>>> environment-; the queue tends to grow in the long run. 100 ms made it
>>> stable. I don't expect this test to run for too much though.
>>>
>>
>> Good idea, I will try rewriting the test.
>>
>> Regards,
>> Laurent
>>
>
>
> --
> --
> Laurent Bourgès
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20181015/2ea0b977/attachment.html>
More information about the awt-dev
mailing list