summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorSven Gothel <[email protected]>2014-09-22 23:17:28 +0200
committerSven Gothel <[email protected]>2014-09-22 23:17:28 +0200
commitcef7ba607ad7e8eb1ff2a438d77710a29aa0bda6 (patch)
tree3023dbb624f261047d99ac6bc034eec781a89d7a /src
parentf9acc8f7b9072b8c12cdc16dee657349180e6bf0 (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')
-rw-r--r--src/jogl/classes/com/jogamp/opengl/util/Animator.java42
-rw-r--r--src/jogl/classes/com/jogamp/opengl/util/AnimatorBase.java9
-rw-r--r--src/jogl/classes/com/jogamp/opengl/util/FPSAnimator.java49
-rw-r--r--src/jogl/classes/jogamp/opengl/GLDrawableHelper.java44
4 files changed, 78 insertions, 66 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();
}
}
}
diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
index ec0d85c91..3deeafd27 100644
--- a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
+++ b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
@@ -78,7 +78,8 @@ public class GLDrawableHelper {
private final ArrayList<GLEventListener> listeners = new ArrayList<GLEventListener>();
private final HashSet<GLEventListener> listenersToBeInit = new HashSet<GLEventListener>();
private final Object glRunnablesLock = new Object();
- private volatile ArrayList<GLRunnableTask> glRunnables = new ArrayList<GLRunnableTask>();
+ private ArrayList<GLRunnableTask> glRunnables = new ArrayList<GLRunnableTask>();
+ private volatile int glRunnableCount = 0;
private boolean autoSwapBufferMode;
private volatile Thread exclusiveContextThread;
/** -1 release, 0 nop, 1 claim */
@@ -103,6 +104,7 @@ public class GLDrawableHelper {
exclusiveContextThread = null;
exclusiveContextSwitch = 0;
synchronized(glRunnablesLock) {
+ glRunnableCount = 0;
glRunnables.clear();
}
animatorCtrl = null;
@@ -669,7 +671,7 @@ public class GLDrawableHelper {
public final void display(final GLAutoDrawable drawable) {
displayImpl(drawable);
// runForAllGLEventListener(drawable, displayAction);
- if( glRunnables.size()>0 && !execGLRunnables(drawable) ) { // glRunnables volatile OK; execGL.. only executed if size > 0
+ if( glRunnableCount > 0 && !execGLRunnables(drawable) ) { // glRunnableCount volatile OK; execGL.. only executed if size > 0
displayImpl(drawable);
// runForAllGLEventListener(drawable, displayAction);
}
@@ -748,43 +750,29 @@ public class GLDrawableHelper {
}
private final boolean execGLRunnables(final GLAutoDrawable drawable) { // glRunnables.size()>0
- boolean res = true;
// swap one-shot list asap
final ArrayList<GLRunnableTask> _glRunnables;
synchronized(glRunnablesLock) {
- if(glRunnables.size()>0) {
+ if( glRunnables.size() > 0 ) {
+ glRunnableCount = 0;
_glRunnables = glRunnables;
glRunnables = new ArrayList<GLRunnableTask>();
} else {
- _glRunnables = null;
+ return true;
}
}
-
- if(null!=_glRunnables) {
- for (int i=0; i < _glRunnables.size(); i++) {
- res = _glRunnables.get(i).run(drawable) && res;
- }
+ boolean res = true;
+ for (int i=0; i < _glRunnables.size(); i++) {
+ res = _glRunnables.get(i).run(drawable) && res;
}
return res;
}
public final void flushGLRunnables() {
- if(glRunnables.size()>0) { // volatile OK
- // swap one-shot list asap
- final ArrayList<GLRunnableTask> _glRunnables;
- synchronized(glRunnablesLock) {
- if(glRunnables.size()>0) {
- _glRunnables = glRunnables;
- glRunnables = new ArrayList<GLRunnableTask>();
- } else {
- _glRunnables = null;
- }
- }
-
- if(null!=_glRunnables) {
- for (int i=0; i < _glRunnables.size(); i++) {
- _glRunnables.get(i).flush();
- }
+ synchronized(glRunnablesLock) {
+ glRunnableCount = 0;
+ while( glRunnables.size() > 0 ) {
+ glRunnables.remove(0).flush();
}
}
}
@@ -914,6 +902,7 @@ public class GLDrawableHelper {
rTask = new GLRunnableTask(glRunnable,
wait ? rTaskLock : null,
wait /* catch Exceptions if waiting for result */);
+ glRunnableCount++;
glRunnables.add(rTask);
}
if( !deferredHere ) {
@@ -978,11 +967,13 @@ public class GLDrawableHelper {
wait = false; // don't wait if exec immediately
}
for(int i=0; i<count-1; i++) {
+ glRunnableCount++;
glRunnables.add( new GLRunnableTask(newGLRunnables.get(i), null, false) );
}
rTask = new GLRunnableTask(newGLRunnables.get(count-1),
wait ? rTaskLock : null,
wait /* catch Exceptions if waiting for result */);
+ glRunnableCount++;
glRunnables.add(rTask);
}
if( !deferredHere ) {
@@ -1009,6 +1000,7 @@ public class GLDrawableHelper {
return;
}
synchronized(glRunnablesLock) {
+ glRunnableCount++;
glRunnables.add( new GLRunnableTask(glRunnable, null, false) );
}
}