From b13f4efae1ad14117efef6a3bd5eee47aaa98cdf Mon Sep 17 00:00:00 2001
From: Sven Gothel <sgothel@jausoft.com>
Date: Mon, 22 Sep 2014 23:17:28 +0200
Subject: 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
---
 .../classes/jogamp/opengl/GLDrawableHelper.java    | 44 +++++++++-------------
 1 file changed, 18 insertions(+), 26 deletions(-)

(limited to 'src/jogl/classes/jogamp')

diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
index f770f2a59..c58fdbff6 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);
     }
@@ -749,43 +751,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();
         }
     }
   }
@@ -915,6 +903,7 @@ public class GLDrawableHelper {
             rTask = new GLRunnableTask(glRunnable,
                                        wait ? rTaskLock : null,
                                        wait  /* catch Exceptions if waiting for result */);
+            glRunnableCount++;
             glRunnables.add(rTask);
         }
         if( !deferredHere ) {
@@ -979,11 +968,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 ) {
@@ -1010,6 +1001,7 @@ public class GLDrawableHelper {
         return;
     }
     synchronized(glRunnablesLock) {
+        glRunnableCount++;
         glRunnables.add( new GLRunnableTask(glRunnable, null, false) );
     }
   }
-- 
cgit v1.2.3