diff options
author | Sven Gothel <[email protected]> | 2014-09-22 23:17:28 +0200 |
---|---|---|
committer | Sven Gothel <[email protected]> | 2014-09-22 23:17:28 +0200 |
commit | cef7ba607ad7e8eb1ff2a438d77710a29aa0bda6 (patch) | |
tree | 3023dbb624f261047d99ac6bc034eec781a89d7a /src/jogl/classes/com | |
parent | f9acc8f7b9072b8c12cdc16dee657349180e6bf0 (diff) |
Fix synchronization issues in GLDrawableHelper.flushGLRunnables(), fixes rare deadlock with animator-exception and invoke(wait=true, ..)
Fix synchronization issues in GLDrawableHelper.flushGLRunnables():
- Querying 'glRunnables.size()' is not synchronized, only its reference is volatile,
not the instance's own states.
- 'flushGLRunnable()' must operates while acquired the 'glRunnable' lock.
- 'glRunnables' are no more volatile
- introduced volatile 'glRunnableCount', allowing 'display(..)' method
to pre-query whether blocking 'execGLRunnables(..)' must be called.
This is risk (deadlock) free.
Also fixes rare deadlock in animator display-exception / GLAD.invoke(wait=true, ..) case:
- 'GLDrawableHelper.invoke(.., GLRunnable)' acquires the 'glRunnable' lock.
- Then it queries animator state, which is blocking.
- Hence animator's 'flushGLRunnable()' call must happen outside the animator lock
Diffstat (limited to 'src/jogl/classes/com')
3 files changed, 60 insertions, 40 deletions
diff --git a/src/jogl/classes/com/jogamp/opengl/util/Animator.java b/src/jogl/classes/com/jogamp/opengl/util/Animator.java index c7a03eddb..4686e1745 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/Animator.java +++ b/src/jogl/classes/com/jogamp/opengl/util/Animator.java @@ -165,7 +165,6 @@ public class Animator extends AnimatorBase { } catch (final UncaughtAnimatorException dre) { displayCaught = dre; stopIssued = true; - isAnimating = false; break; // end pause loop } } @@ -199,7 +198,6 @@ public class Animator extends AnimatorBase { } catch (final UncaughtAnimatorException dre) { displayCaught = dre; stopIssued = true; - isAnimating = false; break; // end animation loop } if ( !runAsFastAsPossible ) { @@ -226,24 +224,32 @@ public class Animator extends AnimatorBase { } } } - synchronized (Animator.this) { - if(DEBUG) { - System.err.println("Animator stop on " + animThread.getName() + ": " + toString()); - if( null != displayCaught ) { - System.err.println("Animator caught: "+displayCaught.getMessage()); - displayCaught.printStackTrace(); + boolean flushGLRunnables = false; + try { + synchronized (Animator.this) { + if(DEBUG) { + System.err.println("Animator stop on " + animThread.getName() + ": " + toString()); + if( null != displayCaught ) { + System.err.println("Animator caught: "+displayCaught.getMessage()); + displayCaught.printStackTrace(); + } } - } - stopIssued = false; - pauseIssued = false; - isAnimating = false; - try { - if( null != displayCaught ) { - handleUncaughtException(displayCaught); // may throw exception if null handler + stopIssued = false; + pauseIssued = false; + isAnimating = false; + try { + if( null != displayCaught ) { + flushGLRunnables = true; + handleUncaughtException(displayCaught); // may throw exception if null handler + } + } finally { + animThread = null; + Animator.this.notifyAll(); } - } finally { - animThread = null; - Animator.this.notifyAll(); + } + } finally { + if( flushGLRunnables ) { + flushGLRunnables(); } } } diff --git a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java index 539d81beb..212a36ab4 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java +++ b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java @@ -524,23 +524,24 @@ public abstract class AnimatorBase implements GLAnimatorControl { /** * Should be called in case of an uncaught exception - * from within the animator thread to flush all animator + * from within the animator thread, throws given exception if no handler has been installed. */ protected final synchronized void handleUncaughtException(final UncaughtAnimatorException ue) { if( null != uncaughtExceptionHandler ) { try { uncaughtExceptionHandler.uncaughtException(this, ue.getGLAutoDrawable(), ue.getCause()); } catch (final Throwable t) { /* ignore intentionally */ } - flushGLRunnables(); } else { - flushGLRunnables(); throw ue; } } /** * Should be called in case of an uncaught exception - * from within the animator thread to flush all animator + * from within the animator thread to flush all animator. + * <p> + * The animator instance shall not be locked when calling this method! + * </p> */ protected final synchronized void flushGLRunnables() { for (int i=0; i<drawables.size(); i++) { diff --git a/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java b/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java index dc4a9a896..d8d746430 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java +++ b/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java @@ -177,7 +177,6 @@ public class FPSAnimator extends AnimatorBase { } catch (final UncaughtAnimatorException dre) { displayCaught = dre; stopIssued = true; - isAnimating = false; } } else if( pauseIssued && !stopIssued ) { // PAUSE if(DEBUG) { @@ -194,7 +193,6 @@ public class FPSAnimator extends AnimatorBase { } catch (final UncaughtAnimatorException dre) { displayCaught = dre; stopIssued = true; - isAnimating = false; } } if( null == displayCaught ) { @@ -207,7 +205,8 @@ public class FPSAnimator extends AnimatorBase { } } } - } else if( stopIssued ) { // STOP + } + if( stopIssued ) { // STOP incl. immediate exception handling of 'displayCaught' if(DEBUG) { System.err.println("FPSAnimator stopping: "+alreadyStopped+", "+ Thread.currentThread() + ": " + toString()); } @@ -220,25 +219,39 @@ public class FPSAnimator extends AnimatorBase { try { display(); // propagate exclusive context -> off! } catch (final UncaughtAnimatorException dre) { - displayCaught = dre; + if(DEBUG) { + System.err.println("AnimatorBase.setExclusiveContextThread: caught: "+dre.getMessage()); + dre.printStackTrace(); + } + if( null == displayCaught ) { + displayCaught = dre; + } } } - synchronized (FPSAnimator.this) { - if(DEBUG) { - System.err.println("FPSAnimator stop " + Thread.currentThread() + ": " + toString()); - if( null != displayCaught ) { - System.err.println("AnimatorBase.setExclusiveContextThread: caught: "+displayCaught.getMessage()); - displayCaught.printStackTrace(); + boolean flushGLRunnables = false; + try { + synchronized (FPSAnimator.this) { + if(DEBUG) { + System.err.println("FPSAnimator stop " + Thread.currentThread() + ": " + toString()); + if( null != displayCaught ) { + System.err.println("Animator caught: "+displayCaught.getMessage()); + displayCaught.printStackTrace(); + } } - } - isAnimating = false; - try { - if( null != displayCaught ) { - handleUncaughtException(displayCaught); // may throw exception if null handler + isAnimating = false; + try { + if( null != displayCaught ) { + flushGLRunnables = true; + handleUncaughtException(displayCaught); // may throw exception if null handler + } + } finally { + animThread = null; + FPSAnimator.this.notifyAll(); } - } finally { - animThread = null; - FPSAnimator.this.notifyAll(); + } + } finally { + if( flushGLRunnables ) { + flushGLRunnables(); } } } |