Hi Martin,
Sorry to tell you that your patch does not fix the reproducer test I made
(see at the end).
It is expected that 2 windows have their corresponding buttons "TEST <num>"
animated: <num> is incrementing, showing events are properly propagated
among windows (2 AppContexts).
I added few logs in the SequencedEventFilter (accept ids) and reject, and
it seems SequencedEvent are still blocking the EDT queues (sort of
deadlock):
SequencedEventsFilter.acceptEvent: reject id = 1200
SequencedEventsFilter.acceptEvent: id = 1006
SequencedEventsFilter.acceptEvent: id = 1006
SequencedEventsFilter.acceptEvent: id = 1006
SequencedEventsFilter.acceptEvent: id = 1007
SequencedEventsFilter.acceptEvent: reject id = 1200
SequencedEventsFilter.acceptEvent: id = 1007
SequencedEventsFilter.acceptEvent: id = 1007
SequencedEventsFilter.acceptEvent: id = 1007
SequencedEventsFilter.acceptEvent: reject id = 800
SequencedEventsFilter.acceptEvent: id = 1007
SequencedEventsFilter.acceptEvent: reject id = 800
SequencedEventsFilter.acceptEvent: reject id = 1200
SequencedEventsFilter.acceptEvent: id = 1006
...
My modified patch:
--- a/src/java.desktop/share/classes/java/awt/SequencedEvent.java Wed
Oct 10 16:20:52 2018 +0530
+++ b/src/java.desktop/share/classes/java/awt/SequencedEvent.java Wed
Oct 10 21:15:26 2018 +0200
@@ -81,6 +81,20 @@
});
}
+ private static final class SequencedEventsFilter implements
EventFilter {
+ private static final SequencedEventsFilter INSTANCE = new
SequencedEventsFilter();
+ @Override
+ public FilterAction acceptEvent(AWTEvent ev) {
+ final int eventID = ev.getID();
+ if (eventID == ID || eventID == SentEvent.ID) {
+System.out.println("SequencedEventsFilter.acceptEvent: id = "+eventID);
+ return FilterAction.ACCEPT;
+ }
+System.out.println("SequencedEventsFilter.acceptEvent: reject id =
"+eventID);
+ return FilterAction.REJECT;
+ }
+ }
+
/**
* Constructs a new SequencedEvent which will dispatch the specified
* nested event.
@@ -135,7 +149,8 @@
if (Thread.currentThread() instanceof
EventDispatchThread) {
EventDispatchThread edt = (EventDispatchThread)
Thread.currentThread();
- edt.pumpEvents(ID, () ->
!SequencedEvent.this.isFirstOrDisposed());
+ edt.pumpEventsForFilter(() ->
!SequencedEvent.this.isFirstOrDisposed(),
+ SequencedEventsFilter.INSTANCE);
} else {
if (fxAppThreadIsDispatchThread) {
fxCheckSequenceThread.start();
Kind Regards,
Laurent
PS: the reproducer TestWinEvent class:
import java.awt.AWTEvent;
import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.WindowEvent;
import java.lang.reflect.Constructor;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.SwingUtilities;
import javax.swing.Timer;
import sun.awt.AppContext;
import sun.awt.SunToolkit;
/*
* Running this code causes the AWT Event Queues to be blocked on OpenJDK11
* @author Laurent Bourges
*/
public class TestWinEvent extends JFrame implements ActionListener {
private static final long serialVersionUID = 1L;
private int counter = 0;
private JButton btn;
public static void main(String[] args) {
createWin(1);
createWin(2);
}
private static void createWin(int tgNum) {
ThreadGroup tg = new ThreadGroup("TG " + tgNum);
Thread t = new Thread(tg, new Runnable() {
public void run() {
AppContext context = SunToolkit.createNewAppContext();
SwingUtilities.invokeLater(new Runnable() {
public void run() {
final TestWinEvent window = new TestWinEvent(tgNum);
window.setVisible(true);
new Timer(10, window).start();
}
});
}
});
t.start();
}
public TestWinEvent(final int num) {
super("Test Window + " + num);
setMinimumSize(new Dimension(300, 200));
setLocation(100 + 400 * (num - 1), 100);
setLayout(new BorderLayout());
JLabel textBlock = new JLabel("Lorem ipsum dolor sit amet...");
add(textBlock);
btn = new JButton("TEST");
btn.addActionListener(new ActionListener() {
@Override
public void actionPerformed(ActionEvent e) {
System.out.println("Button#" + num + " clicked: " +
counter);
}
});
add(btn, BorderLayout.SOUTH);
setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
pack();
}
@Override
public void actionPerformed(ActionEvent e) {
AWTEvent eventOne =
getSequencedEvent(WindowEvent.WINDOW_GAINED_FOCUS);
AWTEvent eventTwo =
getSequencedEvent(WindowEvent.WINDOW_LOST_FOCUS);
btn.setText("TEST " + (counter++));
Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent(eventOne);
Toolkit.getDefaultToolkit().getSystemEventQueue().postEvent(eventTwo);
}
private AWTEvent getSequencedEvent(int id) {
AWTEvent wrapMe = new WindowEvent(this, id);
try {
@SuppressWarnings("unchecked")
Class<? extends AWTEvent> seqClass = (Class<? extends
AWTEvent>) Class.forName("java.awt.SequencedEvent");
Constructor<? extends AWTEvent> seqConst =
seqClass.getConstructor(AWTEvent.class);
seqConst.setAccessible(true);
AWTEvent instance = seqConst.newInstance(wrapMe);
return instance;
} catch (Throwable err) {
throw new Error("Unable to instantiate SequencedEvent", err);
}
}
}
Le mer. 10 oct. 2018 à 20:57, Laurent Bourgès <bourges.laur...@gmail.com> a
écrit :
> Hi Martin,
>
> I am not a reviewer, but I will test your fix with the reproducer test I
> wrote for this bug.
> Did you try it ? I will do asap.
>
> You should use a single instance for your SequencedEventFilter and make
> this class final:
> private static final class SequencedEventsFilter implements
> EventFilter {
> + private static final SequencedEventsFilter INSTANCE = new
> SequencedEventsFilter();
> Then
> edt.pumpEventsForFilter(() ->
> !SequencedEvent.this.isFirstOrDisposed(),
> + new SequencedEventsFilter());
> => SequencedEventsFilter.INSTANCE);
>
> I noticed your patch do not provide any test... yet ?
> I think it is needed here.
>
> Good job for your detailed analysis and the webrev.
>
> Regards,
> Laurent
>
> Le mer. 10 oct. 2018 à 18:19, Martin Balao <mba...@redhat.com> a écrit :
>
>> Hi,
>>
>> Can I have a review for JDK-8204142 [1] Webrev 00?
>>
>> * http://cr.openjdk.java.net/~mbalao/webrevs/8204142/8204142.webrev.00/
>> *
>> http://cr.openjdk.java.net/~mbalao/webrevs/8204142/8204142.webrev.00.zip
>>
>> As a result of [2], I propose Event Dispatch Threads to be aware of
>> SentEvent AWT events when sleeping.
>>
>> In the tests I did, the SequencedEvent.list is successfully flushed with
>> this patch.
>>
>> Thanks,
>> Martin.-
>>
>> --
>> [1] - https://bugs.openjdk.java.net/browse/JDK-8204142
>> [2] -
>> http://mail.openjdk.java.net/pipermail/awt-dev/2018-October/014426.html
>>
>
>
> --
> --
> Laurent Bourgès
>
--
--
Laurent Bourgès
