<AWT Dev> OpenJdk11-28-EA JDialog hanging

[Martin Balao]

Hi Laurent,

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

Can I have a review for it?

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

Kind regards,
Martin.-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20181012/2df11ce8/attachment.html>