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

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) );
     }
   }