From 1c5b41f01c9f31f7bd787c6b194f7939904e239b Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Thu, 12 Jun 2014 21:08:21 +0200 Subject: Fix NEWT EDTUtil Deadlock on EDTUtil.start() DisplayImpl.runOnEDTIfAvail(..) issues EDTUtil.start() while holding it's object-lock - if the EDT is not running, then invokes the given task. EDTUtil.start() impl. holds it's own edt-lock while starting, then releases it's edt-lock while issuing a null-task. If another thread injects a blocking task right in-between which also acquires the display's object-lock it deadlocks. Simply remove issuing the null-task, so EDTUtil.start() can return immediatly (releasing edt-lock) and allowing DisplayImpl.runOnEDTIfAvail(..) also to release it's object-lock. The other threads task then can be executed, where the 'starting task' would come second - which is OK, even though a rare occasion. Above situation was triggered via AWT/NEWT reparenting w/ forced recreation via TestParenting01dAWT. +++ The null-task at EDTUtil.start() was remaining code to ensure that the EDT completed starting, which is redundant. --- src/newt/classes/com/jogamp/newt/util/EDTUtil.java | 4 ++-- src/newt/classes/jogamp/newt/DefaultEDTUtil.java | 6 ++++-- src/newt/classes/jogamp/newt/driver/awt/AWTEDTUtil.java | 11 ++++------- src/newt/classes/jogamp/newt/swt/SWTEDTUtil.java | 13 +++++-------- .../opengl/test/junit/newt/TestDisplayLifecycle01NEWT.java | 11 ++++++----- 5 files changed, 21 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/newt/classes/com/jogamp/newt/util/EDTUtil.java b/src/newt/classes/com/jogamp/newt/util/EDTUtil.java index 582dc3e1f..58aa67105 100644 --- a/src/newt/classes/com/jogamp/newt/util/EDTUtil.java +++ b/src/newt/classes/com/jogamp/newt/util/EDTUtil.java @@ -71,13 +71,13 @@ public interface EDTUtil { * and the caller should wait {@link #waitUntilStopped() until it's stopped}. *

* - * @return true if EDT has been successfully restarted, otherwise false * @throws IllegalStateException if EDT is running and not subject to be stopped, i.e. {@link #isRunning()} returns true + * @throws RuntimeException if EDT could not be started * * @see #invokeStop(boolean, java.lang.Runnable) * @see #waitUntilStopped() */ - public boolean start() throws IllegalStateException; + public void start() throws IllegalStateException; /** * Returns true if the current thread is the event dispatch thread (EDT). diff --git a/src/newt/classes/jogamp/newt/DefaultEDTUtil.java b/src/newt/classes/jogamp/newt/DefaultEDTUtil.java index d481ce8f9..7afa8d746 100644 --- a/src/newt/classes/jogamp/newt/DefaultEDTUtil.java +++ b/src/newt/classes/jogamp/newt/DefaultEDTUtil.java @@ -82,7 +82,7 @@ public class DefaultEDTUtil implements EDTUtil { } @Override - public final boolean start() throws IllegalStateException { + public final void start() throws IllegalStateException { synchronized(edtLock) { if( edt.isRunning() ) { throw new IllegalStateException("EDT still running and not subject to stop. Curr "+Thread.currentThread().getName()+", EDT "+edt.getName()+", isRunning "+edt.isRunning+", shouldStop "+edt.shouldStop); @@ -103,7 +103,9 @@ public class DefaultEDTUtil implements EDTUtil { } startImpl(); } - return invoke(true, nullTask); + if( !edt.isRunning() ) { + throw new RuntimeException("EDT could not be started: "+edt); + } } private final void startImpl() { diff --git a/src/newt/classes/jogamp/newt/driver/awt/AWTEDTUtil.java b/src/newt/classes/jogamp/newt/driver/awt/AWTEDTUtil.java index 4a7193306..407d3abf9 100644 --- a/src/newt/classes/jogamp/newt/driver/awt/AWTEDTUtil.java +++ b/src/newt/classes/jogamp/newt/driver/awt/AWTEDTUtil.java @@ -68,7 +68,7 @@ public class AWTEDTUtil implements EDTUtil { } @Override - public final boolean start() throws IllegalStateException { + public final void start() throws IllegalStateException { synchronized(edtLock) { if( nedt.isRunning() ) { throw new IllegalStateException("EDT still running and not subject to stop. Curr "+Thread.currentThread().getName()+", NEDT "+nedt.getName()+", isRunning "+nedt.isRunning+", shouldStop "+nedt.shouldStop+", on AWT-EDT "+EventQueue.isDispatchThread()); @@ -82,7 +82,9 @@ public class AWTEDTUtil implements EDTUtil { } startImpl(); } - return invoke(true, nullTask); + if( !nedt.isRunning() ) { + throw new RuntimeException("EDT could not be started: "+nedt); + } } private final void startImpl() { @@ -128,11 +130,6 @@ public class AWTEDTUtil implements EDTUtil { return invokeImpl(wait, task, false); } - private static Runnable nullTask = new Runnable() { - @Override - public void run() { } - }; - private final boolean invokeImpl(boolean wait, Runnable task, boolean stop) { Throwable throwable = null; RunnableTask rTask = null; diff --git a/src/newt/classes/jogamp/newt/swt/SWTEDTUtil.java b/src/newt/classes/jogamp/newt/swt/SWTEDTUtil.java index db89690f4..91c18f023 100644 --- a/src/newt/classes/jogamp/newt/swt/SWTEDTUtil.java +++ b/src/newt/classes/jogamp/newt/swt/SWTEDTUtil.java @@ -78,7 +78,7 @@ public class SWTEDTUtil implements EDTUtil { } @Override - public final boolean start() throws IllegalStateException { + public final void start() throws IllegalStateException { final boolean swtDisposed = swtDisplay.isDisposed(); synchronized(edtLock) { if( nedt.isRunning() ) { @@ -99,9 +99,11 @@ public class SWTEDTUtil implements EDTUtil { } } if( !swtDisposed ) { - return invoke(true, nullTask); + if( !nedt.isRunning() ) { + throw new RuntimeException("EDT could not be started: "+nedt); + } } else { - return false; + // FIXME: Throw exception ? } } @@ -149,11 +151,6 @@ public class SWTEDTUtil implements EDTUtil { return invokeImpl(wait, task, false); } - private static Runnable nullTask = new Runnable() { - @Override - public void run() { } - }; - private final boolean invokeImpl(boolean wait, Runnable task, boolean stop) { Throwable throwable = null; RunnableTask rTask = null; diff --git a/src/test/com/jogamp/opengl/test/junit/newt/TestDisplayLifecycle01NEWT.java b/src/test/com/jogamp/opengl/test/junit/newt/TestDisplayLifecycle01NEWT.java index b007f57f3..8989f3a89 100644 --- a/src/test/com/jogamp/opengl/test/junit/newt/TestDisplayLifecycle01NEWT.java +++ b/src/test/com/jogamp/opengl/test/junit/newt/TestDisplayLifecycle01NEWT.java @@ -3,14 +3,14 @@ * * Redistribution and use in source and binary forms, with or without modification, are * permitted provided that the following conditions are met: - * + * * 1. Redistributions of source code must retain the above copyright notice, this list of * conditions and the following disclaimer. - * + * * 2. Redistributions in binary form must reproduce the above copyright notice, this list * of conditions and the following disclaimer in the documentation and/or other materials * provided with the distribution. - * + * * THIS SOFTWARE IS PROVIDED BY JogAmp Community ``AS IS'' AND ANY EXPRESS OR IMPLIED * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL JogAmp Community OR @@ -20,7 +20,7 @@ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * + * * The views and conclusions contained in the software and documentation are those of the * authors and should not be interpreted as representing official policies, either expressed * or implied, of JogAmp Community. @@ -69,7 +69,7 @@ public class TestDisplayLifecycle01NEWT extends UITestCase { // // Create native windowing resources .. X11/Win/OSX - // + // GLWindow glWindow; if(null!=screen) { Window window = NewtFactory.createWindow(screen, caps); @@ -209,6 +209,7 @@ public class TestDisplayLifecycle01NEWT extends UITestCase { Assert.assertNotNull(edtUtil); Assert.assertEquals(false,edtUtil.isRunning()); edtUtil.start(); + Assert.assertEquals(true,edtUtil.isRunning()); edtUtil.invoke(true, null); Assert.assertEquals(true,edtUtil.isRunning()); edtUtil.invokeStop(true, null); -- cgit v1.2.3