diff options
author | Sven Gothel <[email protected]> | 2014-09-23 03:56:04 +0200 |
---|---|---|
committer | Sven Gothel <[email protected]> | 2014-09-23 03:56:04 +0200 |
commit | 66ecb6c386d5c3d87d8be2600a0c7dd7d71a4329 (patch) | |
tree | 8a50d67fb0f3ea895d2522a9888f585c3737b470 | |
parent | c0c5fac5301f7264cfdd69117c1f85adfdc7b604 (diff) |
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):
- <http://docs.oracle.com/javase/specs/jvms/se7/html/jvms-2.html#jvms-2.11.10>
- <http://stackoverflow.com/questions/10743285/behavior-of-a-synchronized-method-with-try-and-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.
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! * </p> */ - protected final synchronized void flushGLRunnables() { + protected final void flushGLRunnables() { for (int i=0; i<drawables.size(); i++) { drawables.get(i).flushGLRunnables(); } diff --git a/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java b/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java index d8d746430..54d40f285 100644 --- a/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java +++ b/src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java @@ -149,7 +149,7 @@ public class FPSAnimator extends AnimatorBase { @Override public void run() { - UncaughtAnimatorException displayCaught = null; + UncaughtAnimatorException caughtException = null; if( justStarted ) { justStarted = false; @@ -175,7 +175,7 @@ public class FPSAnimator extends AnimatorBase { try { display(); } catch (final UncaughtAnimatorException dre) { - displayCaught = dre; + caughtException = dre; stopIssued = true; } } else if( pauseIssued && !stopIssued ) { // PAUSE @@ -191,11 +191,11 @@ public class FPSAnimator extends AnimatorBase { try { display(); // propagate exclusive context -> 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; } } } |