JBR-2146 improve InvokeOnToolkitHelper to cover more generic case

src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java

@@ -64,7 +64,7 @@ import com.apple.laf.ClientPropertyApplicator.Property;
 import sun.awt.AWTAccessor;
 import sun.awt.AWTAccessor.ComponentAccessor;
 import sun.awt.AWTAccessor.WindowAccessor;
-import sun.awt.InvokeOnToolkitHelper;
+import sun.awt.AWTThreading;
 import sun.java2d.SurfaceData;
 import sun.lwawt.LWLightweightFramePeer;
 import sun.lwawt.LWToolkit;
@@ -359,7 +359,7 @@ public class CPlatformWindow extends CFRetainedResource implements PlatformWindo
         long nativeWindowPtr = java.security.AccessController.doPrivileged(
                 (PrivilegedAction<Long>) () -> {
                     try {
-                        return InvokeOnToolkitHelper.invokeAndBlock(() -> {
+                        return AWTThreading.executeWaitToolkit(() -> {
                             AtomicLong ref = new AtomicLong();
                             contentView.execute(viewPtr -> {
                                 boolean hasOwnerPtr = false;

src/java.desktop/macosx/classes/sun/lwawt/macosx/LWCToolkit.java

@@ -496,9 +496,9 @@ public final class LWCToolkit extends LWToolkit {
     public Insets getScreenInsets(final GraphicsConfiguration gc) {
         GraphicsDevice gd = gc.getDevice();
         if (!(gd instanceof CGraphicsDevice)) {
-            return InvokeOnToolkitHelper.invokeAndBlock(() -> super.getScreenInsets(gc));
+            return AWTThreading.executeWaitToolkit(() -> super.getScreenInsets(gc));
         }
-        return InvokeOnToolkitHelper.invokeAndBlock(() -> ((CGraphicsDevice)gd).getScreenInsets());
+        return AWTThreading.executeWaitToolkit(() -> ((CGraphicsDevice)gd).getScreenInsets());
     }
 
     @Override
@@ -752,7 +752,7 @@ public final class LWCToolkit extends LWToolkit {
         final long mediator = createAWTRunLoopMediator();
 
         InvocationEvent invocationEvent =
-                new InvocationEvent(component,
+                AWTThreading.createAndTrackInvocationEvent(component,
                         runnable,
                         () -> {
                             if (mediator != 0) {
@@ -767,7 +767,8 @@ public final class LWCToolkit extends LWToolkit {
 
             // 3746956 - flush events from PostEventQueue to prevent them from getting stuck and causing a deadlock
             SunToolkit.flushPendingEvents(appContext);
-        } else {
+        }
+        else {
             // This should be the equivalent to EventQueue.invokeAndWait
             ((LWCToolkit)Toolkit.getDefaultToolkit()).getSystemEventQueueForInvokeAndWait().postEvent(invocationEvent);
         }

src/java.desktop/share/classes/sun/awt/AWTThreading.java

@@ -0,0 +1,209 @@
+package sun.awt;
+
+import sun.font.FontUtilities;
+
+import java.awt.*;
+import java.awt.event.InvocationEvent;
+import java.lang.ref.SoftReference;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.util.Map;
+import java.util.Stack;
+import java.util.concurrent.*;
+
+/**
+ * Used to perform a cross threads (EventDispatch, Toolkit) execution so that the execution does not cause a deadlock.
+ */
+public class AWTThreading {
+    private ExecutorService executor;
+    // every invokeAndWait() pushes a queue of invocations
+    private final Stack<TrackingQueue> invocations = new Stack<>();
+
+    private int level; // re-entrance level
+
+    // invocations should be dispatched on proper EDT (per AppContext)
+    private static final Map<Thread, AWTThreading> EDT_TO_INSTANCE_MAP = new ConcurrentHashMap<>();
+
+    @SuppressWarnings("serial")
+    private static class TrackingQueue extends LinkedBlockingQueue<InvocationEvent> {}
+
+    private AWTThreading() {}
+
+    /**
+     * Executes a callable from EventDispatch thread (EDT). It's assumed the callable either performs a blocking execution on Toolkit
+     * or waits a notification from Toolkit. The method is re-entrant.
+     * <p>
+     * Currently only macOS is supported. The callable can wrap a native obj-c selector. The selector should be executed via
+     * [JNFRunLoop performOnMainThreadWaiting:YES ...] so that doAWTRunLoop on AppKit (which is run in [JNFRunLoop javaRunLoopMode]) accepts it.
+     * The callable should not call any Java code that would normally be called on EDT.
+     * <p>
+     * A deadlock can happen when the callable triggers any blocking invocation from Toolkit to EDT, or when Toolkit already waits in
+     * such blocking invocation. To avoid that:
+     * <ul>
+     * <li>The callback execution is delegated to a dedicated pool thread.
+     * <li>All invocation events, initiated by Toolkit via invokeAndWait(), are tracked via a dedicated queue.
+     * <li>All the tracked invocation events are dispatched on EDT out of order (EventQueue) during the callback execution.
+     * <li>In case of a re-entrant method call, all the tracked invocation events coming after the call are dispatched first.
+     * </ul><p>
+     * When called on non-EDT, or on non-macOS, the method executes the callable just in place.
+     */
+    public static <T> T executeWaitToolkit(Callable<T> callable) {
+        return executeWaitToolkit(callable, -1, null);
+    }
+
+    /**
+     * Same as {@link #executeWaitToolkit(Callable)} except that the method waits no longer than the specified timeout.
+     */
+    public static <T> T executeWaitToolkit(Callable<T> callable, long timeout, TimeUnit unit) {
+        if (callable == null) return null;
+
+        if (FontUtilities.isMacOSX && EventQueue.isDispatchThread()) {
+            AWTThreading instance = getInstance(Thread.currentThread());
+            if (instance != null) {
+                return instance.execute(callable, timeout, unit);
+            }
+        }
+        // fallback to default
+        try {
+            return callable.call();
+        } catch (Exception e) {
+            e.printStackTrace();
+        }
+        return null;
+    }
+
+    @SuppressWarnings("removal")
+    private <T> T execute(Callable<T> callable, long timeout, TimeUnit unit) {
+        assert EventQueue.isDispatchThread();
+        if (executor == null) {
+            // init on EDT
+            AccessController.doPrivileged((PrivilegedAction<?>)() ->
+                    executor = new ThreadPoolExecutor(1, Integer.MAX_VALUE,
+                            60L, TimeUnit.SECONDS,
+                            new SynchronousQueue<>(),
+                            new ThreadFactory() {
+                                private final ThreadFactory factory = Executors.privilegedThreadFactory();
+                                @Override
+                                public Thread newThread(Runnable r) {
+                                    Thread t = factory.newThread(r);
+                                    t.setDaemon(true);
+                                    t.setName("AWT-" + AWTThreading.class.getSimpleName() + " " + t.getName());
+                                    return t;
+                                }
+                            })
+            );
+        }
+        level++;
+        try {
+            TrackingQueue currentQueue;
+            synchronized (invocations) {
+                if (level == 1 && invocations.size() == 1) {
+                    currentQueue = invocations.peek();
+                } else {
+                    invocations.push(currentQueue = new TrackingQueue());
+                }
+            }
+
+            FutureTask<T> task = new FutureTask<>(callable) {
+                @Override
+                protected void done() {
+                    synchronized (invocations) {
+                        invocations.remove(currentQueue);
+                        // add dummy event to wake up the queue
+                        currentQueue.add(new InvocationEvent(new Object(), ()  -> {}));
+                    }
+                }
+            };
+            executor.execute(task);
+
+            try {
+                while (!task.isDone() || !currentQueue.isEmpty()) {
+                    InvocationEvent event;
+                    if (timeout >= 0 && unit != null) {
+                        event = currentQueue.poll(timeout, unit);
+                    } else {
+                        event = currentQueue.take();
+                    }
+                    if (event == null) {
+                        task.cancel(false);
+                        synchronized (invocations) {
+                            invocations.remove(currentQueue);
+                        }
+                        new RuntimeException("Waiting for the invocation event timed out").printStackTrace();
+                        break;
+                    }
+                    event.dispatch();
+                }
+                return task.isCancelled() ? null : task.get();
+
+            } catch (InterruptedException | ExecutionException e) {
+                e.printStackTrace();
+            }
+        } finally {
+            level--;
+        }
+        return null;
+    }
+
+    /**
+     * Used by implementation.
+     * Creates an invocation event and adds it to the tracking queue.
+     * It's assumed the event is then posted to EventQueue.
+     * The following is provided:
+     * <ul>
+     * <li>If the event is first dispatched from EventQueue - it gets removed from the tracking queue.
+     * <li>If the event is first dispatched from the tracking queue - its dispatching on EventQueue will be noop.
+     * <ul>
+     */
+    public static InvocationEvent createAndTrackInvocationEvent(Object source, Runnable runnable, Runnable listener, boolean catchThrowables) {
+        AWTThreading instance = getInstance(source);
+        if (instance != null) {
+            synchronized (instance.invocations) {
+                if (instance.invocations.isEmpty()) {
+                    instance.invocations.push(new TrackingQueue());
+                }
+                final TrackingQueue queue = instance.invocations.peek();
+                InvocationEvent event = new InvocationEvent(source, runnable, listener, catchThrowables) {
+                    final SoftReference<TrackingQueue> queueRef = new SoftReference<>(queue);
+
+                    @Override
+                    public void dispatch() {
+                        if (!isDispatched()) {
+                            super.dispatch();
+
+                            TrackingQueue queue = queueRef.get();
+                            if (queue != null) {
+                                queue.remove(this);
+                                queueRef.clear();
+                            }
+                        }
+                    }
+                };
+                queue.add(event);
+                return event;
+            }
+        }
+        return new InvocationEvent(source, runnable, listener, catchThrowables);
+    }
+
+    private static AWTThreading getInstance(Object obj) {
+        if (obj == null) return null;
+
+        AppContext appContext = SunToolkit.targetToAppContext(obj);
+        if (appContext == null) return null;
+
+        return getInstance((EventQueue)appContext.get(AppContext.EVENT_QUEUE_KEY));
+    }
+
+    private static AWTThreading getInstance(EventQueue eq) {
+        if (eq == null) return null;
+
+        return getInstance(AWTAccessor.getEventQueueAccessor().getDispatchThread(eq));
+    }
+
+    private static AWTThreading getInstance(Thread edt) {
+        if (edt == null) return null;
+
+        return EDT_TO_INSTANCE_MAP.computeIfAbsent(edt, key -> new AWTThreading());
+    }
+}

src/java.desktop/share/classes/sun/awt/InvokeOnToolkitHelper.java

@@ -1,159 +0,0 @@
-package sun.awt;
-
-import sun.font.FontUtilities;
-
-import java.awt.*;
-import java.awt.event.InvocationEvent;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
-import java.util.Map;
-import java.util.Stack;
-import java.util.concurrent.*;
-
-/**
- * Used to perform a blocking invocation on Toolkit thread from EDT, preventing a deadlock.
- * The deadlock can happen when EDT and Toolkit perform blocking invocations at the same time.
- * The following is performed to resolve it:
- * 1) The invoker spins a nested event loop on EDT while waiting for the invocation to complete.
- * 2) A separate pool thread is used to perform the invocation.
- */
-public class InvokeOnToolkitHelper {
-    private ExecutorService executor;
-    // every invokeAndWait() pushes a queue of invocations
-    private final Stack<LinkedBlockingQueue<InvocationEvent>> invocations = new Stack<>();
-
-    // invocations should be dispatched on proper EDT (per AppContext)
-    private static final Map<Thread, InvokeOnToolkitHelper> edt2invokerMap = new ConcurrentHashMap<>();
-
-    private static final int WAIT_LIMIT_SECONDS = 5;
-
-    private InvokeOnToolkitHelper() {}
-
-    /**
-     * Invokes the callable on Toolkit thread and blocks until the completion. The method is re-entrant.
-     *
-     * On macOS it is assumed the callable wraps a native selector. The selector should be executed via [JNFRunLoop performOnMainThreadWaiting:YES ...]
-     * so that the doAWTRunLoop on AppKit (which is run in [JNFRunLoop javaRunLoopMode]) accepts it. The callable wrapper should not call any Java code
-     * which would normally be called on EDT.
-     * <p>
-     * If Toolkit posts invocation events caused by the callable, those events are intercepted and dispatched on EDT out of order.
-     * <p>
-     * When called on non-EDT, or on non-macOS, the method invokes the callable in place.
-     */
-    public static <T> T invokeAndBlock(Callable<T> callable) {
-        if (callable == null) return null;
-
-        if (FontUtilities.isMacOSX && EventQueue.isDispatchThread()) {
-            InvokeOnToolkitHelper invoker = getInstance(Thread.currentThread());
-            if (invoker != null) {
-                return invoker.invoke(callable);
-            }
-        }
-        // fallback to default
-        try {
-            return callable.call();
-        } catch (Exception e) {
-            e.printStackTrace();
-        }
-        return null;
-    }
-    @SuppressWarnings("removal")
-    private <T> T invoke(Callable<T> callable) {
-        assert EventQueue.isDispatchThread();
-        if (executor == null) {
-            // init on EDT
-            AccessController.doPrivileged((PrivilegedAction<?>)() ->
-                    executor = new ThreadPoolExecutor(1, Integer.MAX_VALUE,
-                            60L, TimeUnit.SECONDS,
-                            new SynchronousQueue<>(),
-                            new ThreadFactory() {
-                                private final ThreadFactory factory = Executors.privilegedThreadFactory();
-                                @Override
-                                public Thread newThread(Runnable r) {
-                                    Thread t = factory.newThread(r);
-                                    t.setDaemon(true);
-                                    t.setName("AWT-InvokeOnToolkitHelper " + t.getName());
-                                    return t;
-                                }
-                            })
-            );
-        }
-        LinkedBlockingQueue<InvocationEvent> currentQueue;
-        synchronized (invocations) {
-            invocations.push(currentQueue = new LinkedBlockingQueue<>());
-        }
-
-        FutureTask<T> task = new FutureTask<T>(callable) {
-            @Override
-            protected void done() {
-                synchronized (invocations) {
-                    // Done with the current queue, wake it up.
-                    invocations.pop().add(new InvocationEvent(executor, () -> {}));
-                }
-            }
-        };
-        executor.execute(task);
-
-        try {
-            while (!task.isDone() || !currentQueue.isEmpty()) {
-                InvocationEvent event = currentQueue.poll(WAIT_LIMIT_SECONDS, TimeUnit.SECONDS);
-                if (event == null) {
-                    new RuntimeException("Waiting for the invocation event timed out").printStackTrace();
-                    break;
-                }
-                event.dispatch();
-            }
-            return task.isDone() ? task.get() : null;
-
-        } catch (InterruptedException | ExecutionException e) {
-            e.printStackTrace();
-        }
-        return null;
-    }
-
-    /**
-     * Warning: the method is used by the implementation and should not be used by a client.
-     *
-     * Checks if there's an active InvokeOnToolkitHelper corresponding to the invocation's AppContext,
-     * adds the invocation to the InvokeOnToolkitHelper's queue and returns true.
-     * Otherwise does nothing and returns false.
-     */
-    public static boolean offer(InvocationEvent invocation) {
-        Object source = invocation.getSource();
-
-        InvokeOnToolkitHelper invoker = (source instanceof Component) ?
-                getInstance((Component)source) :
-                getInstance(Toolkit.getDefaultToolkit().getSystemEventQueue());
-
-        if (invoker == null) return false;
-
-        synchronized (invoker.invocations) {
-            if (!invoker.invocations.isEmpty()) {
-                invoker.invocations.peek().add(invocation);
-                return true;
-            }
-        }
-        return false;
-    }
-
-    private static InvokeOnToolkitHelper getInstance(Component comp) {
-        if (comp == null) return null;
-
-        AppContext appContext = SunToolkit.targetToAppContext(comp);
-        if (appContext == null) return null;
-
-        return getInstance((EventQueue)appContext.get(AppContext.EVENT_QUEUE_KEY));
-    }
-
-    private static InvokeOnToolkitHelper getInstance(EventQueue eq) {
-        if (eq == null) return null;
-
-        return getInstance(AWTAccessor.getEventQueueAccessor().getDispatchThread(eq));
-    }
-
-    private static InvokeOnToolkitHelper getInstance(Thread edt) {
-        if (edt == null) return null;
-
-        return edt2invokerMap.computeIfAbsent(edt, key -> new InvokeOnToolkitHelper());
-    }
-}

src/java.desktop/share/classes/sun/java2d/opengl/OGLRenderQueue.java

@@ -25,7 +25,7 @@
 
 package sun.java2d.opengl;
 
-import sun.awt.InvokeOnToolkitHelper;
+import sun.awt.AWTThreading;
 import sun.awt.util.ThreadGroupUtils;
 import sun.java2d.pipe.RenderBuffer;
 import sun.java2d.pipe.RenderQueue;
@@ -33,6 +33,7 @@ import sun.java2d.pipe.RenderQueue;
 import static sun.java2d.pipe.BufferedOpCodes.*;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+import java.util.concurrent.TimeUnit;
 
 /**
  * OGL-specific implementation of RenderQueue.  This class provides a
@@ -192,7 +193,7 @@ public class OGLRenderQueue extends RenderQueue {
             }
             if (needsFlush) {
                 // if we still wait for flush then avoid potential deadlock
-                err = InvokeOnToolkitHelper.invokeAndBlock(() -> {
+                err = AWTThreading.executeWaitToolkit(() -> {
                     synchronized (QueueFlusher.this) {
                         while (needsFlush) {
                             try {
@@ -202,7 +203,7 @@ public class OGLRenderQueue extends RenderQueue {
                         }
                         return error;
                     }
-                });
+                }, 5, TimeUnit.SECONDS);
             }
             // re-throw any error that may have occurred during the flush
             if (err != null) {

test/jdk/jb/java/awt/Toolkit/AWTThreadingTest.java

@@ -0,0 +1,140 @@
+import java.awt.*;
+import java.awt.event.WindowAdapter;
+import java.awt.event.WindowEvent;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Condition;
+import java.util.concurrent.locks.ReentrantLock;
+
+import sun.awt.InvokeOnToolkitHelper;
+import sun.lwawt.macosx.CThreading;
+import sun.lwawt.macosx.LWCToolkit;
+
+import javax.swing.*;
+
+/*
+ * @test
+ * @summary tests that AWTThreading can manage a stream of cross EDT/AppKit invocation requests
+ * @requires (os.family == "mac")
+ * @compile --add-exports=java.desktop/sun.lwawt.macosx=ALL-UNNAMED --add-exports=java.desktop/sun.awt=ALL-UNNAMED AWTThreadingTest.java
+ * @run main/othervm --add-exports=java.desktop/sun.lwawt.macosx=ALL-UNNAMED --add-exports=java.desktop/sun.awt=ALL-UNNAMED AWTThreadingTest
+ * @author Anton Tarasov
+ */
+public class AWTThreadingTest {
+    static final ReentrantLock LOCK = new ReentrantLock();
+    static final Condition COND = LOCK.newCondition();
+    static final CountDownLatch LATCH = new CountDownLatch(1);
+
+    static JFrame frame;
+    static Thread thread;
+
+    final static AtomicBoolean passed = new AtomicBoolean(true);
+    final static AtomicInteger counter = new AtomicInteger(0);
+
+    public static void main(String[] args) throws InterruptedException {
+        EventQueue.invokeLater(AWTThreadingTest::runGui);
+
+        LATCH.await(5, TimeUnit.SECONDS);
+
+        frame.dispose();
+        thread.interrupt();
+
+        if (!passed.get()) {
+            throw new RuntimeException("Test FAILED!");
+        }
+        System.out.println("Test PASSED");
+    }
+
+    static void runGui() {
+        frame = new JFrame("frame");
+        frame.setLocationRelativeTo(null);
+        frame.setSize(200, 200);
+        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
+        frame.addWindowListener(new WindowAdapter() {
+            @Override
+            public void windowOpened(WindowEvent e) {
+                startThread();
+            }
+        });
+        frame.setVisible(true);
+    }
+
+    static void startThread() {
+        thread = new Thread(() -> {
+            while (true) {
+                try {
+                    Thread.sleep(100);
+                } catch (InterruptedException e) {
+                    break;
+                }
+
+                //
+                // 1. Execute invokeAndWait() from AppKit to EDT
+                //
+                CThreading.executeOnAppKit(() -> {
+                    try {
+                        LWCToolkit.invokeAndWait(counter::incrementAndGet, Window.getWindows()[0]);
+                    } catch (Exception e) {
+                        fail(e);
+                    }
+                });
+
+                //
+                // 2. Execute invokeAndBlock() from EDT to AppKit
+                //
+                EventQueue.invokeLater(() -> {
+                    passed.set(false);
+
+                    Boolean success = InvokeOnToolkitHelper.invokeAndBlock(() -> {
+                        try {
+                            return CThreading.executeOnAppKit(() -> Boolean.TRUE);
+                        } catch (Throwable e) {
+                            fail(e);
+                        }
+                        return null;
+                    });
+                    System.out.println("Success: " + counter.get() + ": " + success);
+
+                    passed.set(Boolean.TRUE.equals(success));
+
+                    if (passed.get()) {
+                        lock(COND::signal);
+                    }
+                    else {
+                        fail(null);
+                    }
+                });
+
+                lock(COND::await);
+            }
+        });
+        thread.setDaemon(true);
+        thread.start();
+    }
+
+    static void lock(MyRunnable runnable) {
+        LOCK.lock();
+        try {
+            try {
+                runnable.run();
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+        } finally {
+            LOCK.unlock();
+        }
+    }
+
+    interface MyRunnable {
+        void run() throws Exception;
+    }
+
+    static void fail(Throwable e) {
+        if (e != null) e.printStackTrace();
+        passed.set(false);
+        LATCH.countDown();
+    }
+}