From 8e9407ab74f672c2a0d1e196a3ba2e7d8743debf Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Tue, 23 Sep 2014 03:56:04 +0200 Subject: Fix synchronization issues in Animator* Exception case Refines commit cef7ba607ad7e8eb1ff2a438d77710a29aa0bda6 - The animator monitor-lock was still hold in the post finally block issuing flushGLRunnables(), due to intrinsic monitor release (in finally): - - - Further: AnimatorBase.flushGLRunnables() acquired the lock itself (duh!) This commit removes the requirement for finally altogether by simply return a boolean from handleUncaughtException(caughtException), where false denotes the caller to propagate the exception itself (no handler). Post synchronized block then issues flushGLRunnables() and exceptation propagation as required. AnimatorBase.flushGLRunnables() 'synchronized' modifier is removed. Further, ThreadDeath is being propagated if caught. Here the finally block is also removed - redundant. --- .../classes/com/jogamp/opengl/util/Animator.java | 85 +++++++++++----------- .../com/jogamp/opengl/util/AnimatorBase.java | 9 ++- .../com/jogamp/opengl/util/FPSAnimator.java | 57 +++++++-------- 3 files changed, 77 insertions(+), 74 deletions(-) diff --git a/src/jogl/classes/com/jogamp/opengl/util/Animator.java b/src/jogl/classes/com/jogamp/opengl/util/Animator.java index 4686e1745..03c566d7d 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/Animator.java +++ b/src/jogl/classes/com/jogamp/opengl/util/Animator.java @@ -132,7 +132,8 @@ public class Animator extends AnimatorBase { @Override public void run() { - UncaughtAnimatorException displayCaught = null; + ThreadDeath caughtThreadDeath = null; + UncaughtAnimatorException caughtException = null; try { synchronized (Animator.this) { @@ -163,7 +164,7 @@ public class Animator extends AnimatorBase { try { display(); // propagate exclusive context -> off! } catch (final UncaughtAnimatorException dre) { - displayCaught = dre; + caughtException = dre; stopIssued = true; break; // end pause loop } @@ -196,7 +197,7 @@ public class Animator extends AnimatorBase { try { display(); } catch (final UncaughtAnimatorException dre) { - displayCaught = dre; + caughtException = dre; stopIssued = true; break; // end animation loop } @@ -206,52 +207,54 @@ public class Animator extends AnimatorBase { } } } - } catch( final ThreadDeath td) { + } catch(final ThreadDeath td) { if(DEBUG) { System.err.println("Animator caught: "+td.getClass().getName()+": "+td.getMessage()); td.printStackTrace(); } - } finally { - if( exclusiveContext && !drawablesEmpty ) { - setDrawablesExclCtxState(false); - try { - display(); // propagate exclusive context -> off! - } catch (final UncaughtAnimatorException dre) { - if( null == displayCaught ) { - displayCaught = dre; - } else { - dre.printStackTrace(); - } - } - } - boolean flushGLRunnables = false; + caughtThreadDeath = td; + } + if( exclusiveContext && !drawablesEmpty ) { + setDrawablesExclCtxState(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 ) { - flushGLRunnables = true; - handleUncaughtException(displayCaught); // may throw exception if null handler - } - } finally { - animThread = null; - Animator.this.notifyAll(); - } + display(); // propagate exclusive context -> off! + } catch (final UncaughtAnimatorException dre) { + if( null == caughtException ) { + caughtException = dre; + } else { + System.err.println("Animator.setExclusiveContextThread: caught: "+dre.getMessage()); + dre.printStackTrace(); } - } finally { - if( flushGLRunnables ) { - flushGLRunnables(); + } + } + boolean flushGLRunnables = false; + boolean throwCaughtException = false; + synchronized (Animator.this) { + if(DEBUG) { + System.err.println("Animator stop on " + animThread.getName() + ": " + toString()); + if( null != caughtException ) { + System.err.println("Animator caught: "+caughtException.getMessage()); + caughtException.printStackTrace(); } } + stopIssued = false; + pauseIssued = false; + isAnimating = false; + if( null != caughtException ) { + flushGLRunnables = true; + throwCaughtException = !handleUncaughtException(caughtException); + } + animThread = null; + Animator.this.notifyAll(); + } + if( flushGLRunnables ) { + flushGLRunnables(); + } + if( throwCaughtException ) { + throw caughtException; + } + if( null != caughtThreadDeath ) { + throw caughtThreadDeath; } } } diff --git a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java index 212a36ab4..bc159ef5c 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java +++ b/src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java @@ -525,14 +525,17 @@ public abstract class AnimatorBase implements GLAnimatorControl { /** * Should be called in case of an uncaught exception * from within the animator thread, throws given exception if no handler has been installed. + * @return {@code true} if handled, otherwise {@code false}. In case of {@code false}, + * caller needs to propagate the exception. */ - protected final synchronized void handleUncaughtException(final UncaughtAnimatorException ue) { + protected final synchronized boolean handleUncaughtException(final UncaughtAnimatorException ue) { if( null != uncaughtExceptionHandler ) { try { uncaughtExceptionHandler.uncaughtException(this, ue.getGLAutoDrawable(), ue.getCause()); } catch (final Throwable t) { /* ignore intentionally */ } + return true; } else { - throw ue; + return false; } } @@ -543,7 +546,7 @@ public abstract class AnimatorBase implements GLAnimatorControl { * The animator instance shall not be locked when calling this method! *

*/ - protected final synchronized void flushGLRunnables() { + protected final void flushGLRunnables() { for (int i=0; i off! } catch (final UncaughtAnimatorException dre) { - displayCaught = dre; + caughtException = dre; stopIssued = true; } } - if( null == displayCaught ) { + if( null == caughtException ) { synchronized (FPSAnimator.this) { if(DEBUG) { System.err.println("FPSAnimator pause " + Thread.currentThread() + ": " + toString()); @@ -219,40 +219,37 @@ public class FPSAnimator extends AnimatorBase { try { display(); // propagate exclusive context -> off! } catch (final UncaughtAnimatorException dre) { - if(DEBUG) { - System.err.println("AnimatorBase.setExclusiveContextThread: caught: "+dre.getMessage()); + if( null == caughtException ) { + caughtException = dre; + } else { + System.err.println("FPSAnimator.setExclusiveContextThread: caught: "+dre.getMessage()); dre.printStackTrace(); } - if( null == displayCaught ) { - displayCaught = dre; - } } } 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 ) { - flushGLRunnables = true; - handleUncaughtException(displayCaught); // may throw exception if null handler - } - } finally { - animThread = null; - FPSAnimator.this.notifyAll(); + boolean throwCaughtException = false; + synchronized (FPSAnimator.this) { + if(DEBUG) { + System.err.println("FPSAnimator stop " + Thread.currentThread() + ": " + toString()); + if( null != caughtException ) { + System.err.println("Animator caught: "+caughtException.getMessage()); + caughtException.printStackTrace(); } } - } finally { - if( flushGLRunnables ) { - flushGLRunnables(); + isAnimating = false; + if( null != caughtException ) { + flushGLRunnables = true; + throwCaughtException = !handleUncaughtException(caughtException); } + animThread = null; + FPSAnimator.this.notifyAll(); + } + if( flushGLRunnables ) { + flushGLRunnables(); + } + if( throwCaughtException ) { + throw caughtException; } } } -- cgit v1.2.3