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.laur...@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
