summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSven Gothel <[email protected]>2014-09-23 03:56:04 +0200
committerSven Gothel <[email protected]>2014-09-23 03:57:05 +0200
commit8e9407ab74f672c2a0d1e196a3ba2e7d8743debf (patch)
tree827d4f0dd8973ec19d31338b239eb52a6352cf04
parent2fc3a60ed01501727f0645f35ffe75eb56a32aec (diff)
Fix synchronization issues in Animator* Exception casev2.2.2
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.
-rw-r--r--src/jogl/classes/com/jogamp/opengl/util/Animator.java85
-rw-r--r--src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java9
-rw-r--r--src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java57
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;
}
}
}