Bug 1033: Guarantee atomicity of high-level GLAutoDrawable operations, avoiding race conditions.

GLAutoDrawable (API CHANGE) allowing atomic operations:
  - Add class API-doc chapter about 'GLAutoDrawable Locking'

  - Add method invoke(..) API-doc description about throwing IllegalStateException in case of a detected deadlock situation ahead
    (Note: Implemented in GLDrawableHelper.invoke(..) for all implementations)

  - Add new methods for proper multithread handling:
    - public RecursiveLock getUpstreamLock();
    - public boolean isThreadGLCapable();

+++

GLEventListenerState/GLDrawableUtil:

  - Perform operation in a atomic fashion,
    i.e. lock GLAutoDrawable during whole operations:
      - GLDrawableUtil.swapGLContext(..)
      - GLDrawableUtil.swapGLContextAndAllGLEventListener(..)
      - GLEventListenerState.moveFrom(..)
      - GLEventListenerState.moveTo(..)

  - ReshapeGLEventListener:
    - Moved from GLEventListenerState.ReshapeGLEventListener -> GLDrawableUtil.ReshapeGLEventListener
    - Takes 'displayAfterReshape' case into account.

+++

javax.media.opengl.Threading Clarifications:
  - Public 'enum Mode', i.e. Threading.Mode

  - Public getMode()

  - Clarified 'isOpenGLThread()':
    - Take 'singleThreaded' into account directly,
      i.e. always return 'true' if singleThreaded == false

5 files changed, 172 insertions, 74 deletions

diff --git a/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java b/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java
index fce5c1fcc..605c3fce8 100644
--- a/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java
+++ b/src/jogl/classes/jogamp/opengl/GLAutoDrawableBase.java
@@ -57,9 +57,11 @@ import com.jogamp.opengl.GLStateKeeper;
 
 
 /**
- * Abstract common code for GLAutoDrawable implementations.
+ * Abstract common code for GLAutoDrawable implementations
+ * utilizing multithreading, i.e. {@link #isThreadGLCapable()} always returns <code>true</code>.
  *
  * @see GLAutoDrawable
+ * @see GLAutoDrawable#getThreadingMode()
  * @see GLAutoDrawableDelegate
  * @see GLOffscreenAutoDrawable
  * @see GLOffscreenAutoDrawableImpl
@@ -123,9 +125,6 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
         helper.setSharedAutoDrawable(this, sharedAutoDrawable);
     }
 
-    /** Returns the recursive lock object of the upstream implementation, which synchronizes multithreaded access on top of {@link NativeSurface#lockSurface()}. */
-    protected abstract RecursiveLock getLock();
-
     @Override
     public final GLStateKeeper.Listener setGLStateKeeperListener(final Listener l) {
         final GLStateKeeper.Listener pre = glStateKeeperListener;
@@ -240,7 +239,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
                 System.err.println("GLAutoDrawableBase.sizeChanged: ("+getThreadName()+"): "+newWidth+"x"+newHeight+" - surfaceHandle 0x"+Long.toHexString(surfaceHandle));
             }
             if( ! _drawable.getChosenGLCapabilities().isOnscreen() ) {
-                final RecursiveLock _lock = getLock();
+                final RecursiveLock _lock = getUpstreamLock();
                 _lock.lock();
                 try {
                     final GLDrawableImpl _drawableNew = GLDrawableHelper.resizeOffscreenDrawable(_drawable, context, newWidth, newHeight);
@@ -332,7 +331,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
      * Calls {@link #destroyImplInLock()} while claiming the lock.
      */
     protected final void defaultDestroy() {
-        final RecursiveLock lock = getLock();
+        final RecursiveLock lock = getUpstreamLock();
         lock.lock();
         try {
             destroyImplInLock();
@@ -380,7 +379,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
     }
 
     public final void defaultSwapBuffers() throws GLException {
-        final RecursiveLock _lock = getLock();
+        final RecursiveLock _lock = getUpstreamLock();
         _lock.lock();
         try {
             if(null != drawable) {
@@ -421,7 +420,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
             destroy();
             return;
         }
-        final RecursiveLock _lock = getLock();
+        final RecursiveLock _lock = getUpstreamLock();
         _lock.lock();
         try {
             if( null == context ) {
@@ -452,7 +451,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
     }
 
     protected final GLEventListener defaultDisposeGLEventListener(final GLEventListener listener, final boolean remove) {
-        final RecursiveLock _lock = getLock();
+        final RecursiveLock _lock = getUpstreamLock();
         _lock.lock();
         try {
             return helper.disposeGLEventListener(GLAutoDrawableBase.this, drawable, context, listener, remove);
@@ -473,7 +472,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
 
     @Override
     public final GLContext setContext(final GLContext newCtx, final boolean destroyPrevCtx) {
-        final RecursiveLock lock = getLock();
+        final RecursiveLock lock = getUpstreamLock();
         lock.lock();
         try {
             final GLContext oldCtx = context;
@@ -571,12 +570,12 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
     }
 
     @Override
-    public final boolean invoke(final boolean wait, final GLRunnable glRunnable) {
+    public final boolean invoke(final boolean wait, final GLRunnable glRunnable) throws IllegalStateException {
         return helper.invoke(this, wait, glRunnable);
     }
 
     @Override
-    public boolean invoke(final boolean wait, final List<GLRunnable> glRunnables) {
+    public boolean invoke(final boolean wait, final List<GLRunnable> glRunnables) throws IllegalStateException {
         return helper.invoke(this, wait, glRunnables);
     }
 
@@ -604,6 +603,15 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
         return additionalCtxCreationFlags;
     }
 
+    /**
+     * {@inheritDoc}
+     * <p>
+     * Implementation always supports multithreading, hence method always returns <code>true</code>.
+     * </p>
+     */
+    @Override
+    public final boolean isThreadGLCapable() { return true; }
+
     //
     // FPSCounter
     //
@@ -664,7 +672,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
 
     @Override
     public final GLContext createContext(final GLContext shareWith) {
-        final RecursiveLock lock = getLock();
+        final RecursiveLock lock = getUpstreamLock();
         lock.lock();
         try {
             if(drawable != null) {
@@ -680,7 +688,7 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe
 
     @Override
     public final void setRealized(final boolean realized) {
-        final RecursiveLock _lock = getLock();
+        final RecursiveLock _lock = getUpstreamLock();
         _lock.lock();
         try {
             final GLDrawable _drawable = drawable;
diff --git a/src/jogl/classes/jogamp/opengl/GLContextImpl.java b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
index c175243ae..d7578b9ea 100644
--- a/src/jogl/classes/jogamp/opengl/GLContextImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
@@ -230,6 +230,8 @@ public abstract class GLContextImpl extends GLContext {
         if(!lockHeld) {
             makeCurrent();
         }
+        // sync GL ctx w/ drawable's framebuffer before de-association
+        gl.glFinish();
         associateDrawable(false);
         if(!lockHeld) {
             release();
diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
index eac14fdff..ad1b9a556 100644
--- a/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
+++ b/src/jogl/classes/jogamp/opengl/GLDrawableHelper.java
@@ -208,19 +208,23 @@ public class GLDrawableHelper {
   /**
    * Switch {@link GLContext} / {@link GLDrawable} association.
    * <p>
-   * The <code>oldCtx</code> will be destroyed if <code>destroyPrevCtx</code> is <code>true</code>,
-   * otherwise dis-associate <code>oldCtx</code> from <code>drawable</code>
-   * via {@link GLContext#setGLDrawable(GLDrawable, boolean) oldCtx.setGLDrawable(null, true);}.
-   * </p>
-   * <p>
-   * Re-associate <code>newCtx</code> with <code>drawable</code>
-   * via {@link GLContext#setGLDrawable(GLDrawable, boolean) newCtx.setGLDrawable(drawable, true);}.
-   * </p>
-   * <p>
-   * If the old or new context was current on this thread, it is being released before switching the drawable.
-   * </p>
-   * <p>
-   * No locking is being performed on the drawable, caller is required to take care of it.
+   * Remarks:
+   * <ul>
+   *   <li>The <code>oldCtx</code> will be destroyed if <code>destroyPrevCtx</code> is <code>true</code>,
+   *       otherwise disassociate <code>oldCtx</code> from <code>drawable</code>
+   *       via {@link GLContext#setGLDrawable(GLDrawable, boolean) oldCtx.setGLDrawable(null, true);} including {@link GL#glFinish() glFinish()}.</li>
+   *   <li>Reassociate <code>newCtx</code> with <code>drawable</code>
+   *       via {@link GLContext#setGLDrawable(GLDrawable, boolean) newCtx.setGLDrawable(drawable, true);}.</li>
+   *   <li>If the old context was current on this thread, it is being released after disassociating the drawable.</li>
+   *   <li>If the new context was current on this thread, it is being released before associating the drawable
+   *       and made current afterwards.</li>
+   *   <li>Implementation may issue {@link #makeCurrent()} and {@link #release()} while drawable reassociation.</li>
+   *   <li>The user shall take extra care of thread synchronization,
+   *       i.e. lock the involved {@link GLDrawable#getNativeSurface() drawable's} {@link NativeSurface}s
+   *       to avoid a race condition. In case {@link GLAutoDrawable auto-drawable's} are used,
+   *       their {@link GLAutoDrawable#getUpstreamLock() upstream-lock} must be locked beforehand
+   *       see <a href="../../javax/media/opengl/GLAutoDrawable.html#locking">GLAutoDrawable Locking</a>.</li>
+   * </ul>
    * </p>
    *
    * @param drawable the drawable which context is changed
@@ -795,6 +799,30 @@ public class GLDrawableHelper {
     return ( null != animatorCtrl ) ? animatorCtrl.isAnimating() : false ;
   }
 
+  public static final boolean isLockedByOtherThread(final GLAutoDrawable d) {
+      final Thread currentThread = Thread.currentThread();
+      final Thread upstreamLockOwner = d.getUpstreamLock().getOwner();
+      if( null != upstreamLockOwner && currentThread != upstreamLockOwner ) {
+          return true;
+      } else {
+          final NativeSurface s = d.getNativeSurface();
+          final Thread surfaceLockOwner = null != s ? s.getSurfaceLockOwner() : null;
+          return null != surfaceLockOwner  && currentThread != surfaceLockOwner;
+      }
+  }
+
+  public static final boolean isLockedByThisThread(final GLAutoDrawable d) {
+      final Thread currentThread = Thread.currentThread();
+      final Thread upstreamLockOwner = d.getUpstreamLock().getOwner();
+      if( currentThread == upstreamLockOwner ) {
+          return true;
+      } else {
+          final NativeSurface s = d.getNativeSurface();
+          final Thread surfaceLockOwner = null != s ? s.getSurfaceLockOwner() : null;
+          return currentThread == surfaceLockOwner;
+      }
+  }
+
   /**
    * <p>
    * If <code>wait</code> is <code>true</code> the call blocks until the <code>glRunnable</code>
@@ -805,13 +833,32 @@ public class GLDrawableHelper {
    * the call is ignored and returns <code>false</code>.<br>
    * This helps avoiding deadlocking the caller.
    * </p>
+   * <p>
+   * <pre>
+   * 0 == deferredHere && 0 == isGLThread -> display() will issue on GL thread, blocking!
+   *
+   *         deferredHere wait isGLThread   lockedByThisThread  Note
+   *  OK     0            x    1            x
+   *  OK     0            x    0            0
+   *  ERROR  0            x    0            1                   Will be deferred on GL thread by display() (blocking),
+   *                                                            but locked by this thread -> ERROR
+   *
+   *         1            0    x            x                   All good, due to no wait, non blocking
+   *
+   *         1            1    1            0
+   *         1            1    0            0
+   *  SWITCH 1            1    1            1                   Run immediately, don't defer since locked by this thread, but isGLThread
+   *  ERROR  1            1    0            1                   Locked by this thread, but _not_ isGLThread -> ERROR
+   * </pre>
+   * </p>
    *
    * @param drawable the {@link GLAutoDrawable} to be used
    * @param wait if <code>true</code> block until execution of <code>glRunnable</code> is finished, otherwise return immediatly w/o waiting
    * @param glRunnable the {@link GLRunnable} to execute within {@link #display()}
    * @return <code>true</code> if the {@link GLRunnable} has been processed or queued, otherwise <code>false</code>.
+   * @throws IllegalStateException in case the drawable is locked by this thread, no animator is running on another thread and <code>wait</code> is <code>true</code>.
    */
-  public final boolean invoke(final GLAutoDrawable drawable, boolean wait, final GLRunnable glRunnable) {
+  public final boolean invoke(final GLAutoDrawable drawable, boolean wait, final GLRunnable glRunnable) throws IllegalStateException {
     if( null == glRunnable || null == drawable ||
         wait && ( !drawable.isRealized() || null==drawable.getContext() ) ) {
         return false;
@@ -821,18 +868,33 @@ public class GLDrawableHelper {
     final Object rTaskLock = new Object();
     Throwable throwable = null;
     synchronized(rTaskLock) {
-        final boolean deferred;
+        boolean deferredHere;
         synchronized(glRunnablesLock) {
-            deferred = isAnimatorAnimatingOnOtherThread();
-            if(!deferred) {
-                wait = false; // don't wait if exec immediatly
+            final boolean isGLThread = drawable.isThreadGLCapable();
+            deferredHere = isAnimatorAnimatingOnOtherThread();
+            if( deferredHere ) {
+                if( wait && isLockedByThisThread(drawable) ) {
+                    if( isGLThread ) {
+                        // Run immediately, don't defer since locked by this thread, but isGLThread
+                        deferredHere = false;
+                    } else {
+                        // Locked by this thread, but _not_ isGLThread -> ERROR
+                        throw new IllegalStateException("Deferred, wait, isLocked on current and not GL-Thread: thread "+Thread.currentThread());
+                    }
+                }
+            } else {
+                if( !isGLThread && isLockedByThisThread(drawable) ) {
+                    // Will be deferred on GL thread by display() (blocking), but locked by this thread -> ERROR
+                    throw new IllegalStateException("Not deferred, isLocked on current and not GL-Thread: thread "+Thread.currentThread());
+                }
+                wait = false; // don't wait if exec immediately
             }
             rTask = new GLRunnableTask(glRunnable,
                                        wait ? rTaskLock : null,
                                        wait  /* catch Exceptions if waiting for result */);
             glRunnables.add(rTask);
         }
-        if( !deferred ) {
+        if( !deferredHere ) {
             drawable.display();
         } else if( wait ) {
             try {
@@ -851,7 +913,16 @@ public class GLDrawableHelper {
     return true;
   }
 
-  public final boolean invoke(final GLAutoDrawable drawable, boolean wait, final List<GLRunnable> newGLRunnables) {
+  /**
+   * @see #invoke(GLAutoDrawable, boolean, GLRunnable)
+   *
+   * @param drawable
+   * @param wait
+   * @param newGLRunnables
+   * @return
+   * @throws IllegalStateException
+   */
+  public final boolean invoke(final GLAutoDrawable drawable, boolean wait, final List<GLRunnable> newGLRunnables) throws IllegalStateException {
     if( null == newGLRunnables || newGLRunnables.size() == 0 || null == drawable ||
         wait && ( !drawable.isRealized() || null==drawable.getContext() ) ) {
         return false;
@@ -862,10 +933,25 @@ public class GLDrawableHelper {
     final Object rTaskLock = new Object();
     Throwable throwable = null;
     synchronized(rTaskLock) {
-        final boolean deferred;
+        boolean deferredHere;
         synchronized(glRunnablesLock) {
-            deferred = isAnimatorAnimatingOnOtherThread() || !drawable.isRealized();
-            if(!deferred) {
+            final boolean isGLThread = drawable.isThreadGLCapable();
+            deferredHere = isAnimatorAnimatingOnOtherThread();
+            if( deferredHere ) {
+                if( wait && isLockedByThisThread(drawable) ) {
+                    if( isGLThread ) {
+                        // Run immediately, don't defer since locked by this thread, but isGLThread
+                        deferredHere = false;
+                    } else {
+                        // Locked by this thread, but _not_ isGLThread -> ERROR
+                        throw new IllegalStateException("Deferred, wait, isLocked on current and not GL-Thread: thread "+Thread.currentThread());
+                    }
+                }
+            } else {
+                if( !isGLThread && isLockedByThisThread(drawable) ) {
+                    // Will be deferred on GL thread by display() (blocking), but locked by this thread -> ERROR
+                    throw new IllegalStateException("Not deferred, isLocked on current and not GL-Thread: thread "+Thread.currentThread());
+                }
                 wait = false; // don't wait if exec immediately
             }
             for(int i=0; i<count-1; i++) {
@@ -876,7 +962,7 @@ public class GLDrawableHelper {
                                        wait  /* catch Exceptions if waiting for result */);
             glRunnables.add(rTask);
         }
-        if( !deferred ) {
+        if( !deferredHere ) {
             drawable.display();
         } else if( wait ) {
             try {
diff --git a/src/jogl/classes/jogamp/opengl/ThreadingImpl.java b/src/jogl/classes/jogamp/opengl/ThreadingImpl.java
index 2b017e8e9..7b405e524 100644
--- a/src/jogl/classes/jogamp/opengl/ThreadingImpl.java
+++ b/src/jogl/classes/jogamp/opengl/ThreadingImpl.java
@@ -41,6 +41,7 @@ import java.security.PrivilegedAction;
 import javax.media.nativewindow.NativeWindowFactory;
 import javax.media.opengl.GLException;
 import javax.media.opengl.GLProfile;
+import javax.media.opengl.Threading.Mode;
 
 import com.jogamp.common.JogampRuntimeException;
 import com.jogamp.common.util.PropertyAccess;
@@ -49,16 +50,6 @@ import com.jogamp.common.util.ReflectionUtil;
 /** Implementation of the {@link javax.media.opengl.Threading} class. */
 
 public class ThreadingImpl {
-    public enum Mode {
-        MT(0), ST_AWT(1), ST_WORKER(2);
-
-        public final int id;
-
-        Mode(final int id){
-            this.id = id;
-        }
-    }
-
     protected static final boolean DEBUG = Debug.debug("Threading");
 
     private static boolean singleThreaded;
@@ -93,28 +84,23 @@ public class ThreadingImpl {
 
                         _isX11 = NativeWindowFactory.TYPE_X11 == NativeWindowFactory.getNativeWindowType(false);
 
-                        // default setting
-                        singleThreaded = true;
-                        mode  = ( hasAWT ? Mode.ST_AWT : Mode.ST_WORKER );
-
                         if (singleThreadProp != null) {
                             if (singleThreadProp.equals("true") ||
                                 singleThreadProp.equals("auto")) {
-                                singleThreaded = true;
-                                mode  = ( hasAWT ? Mode.ST_AWT : Mode.ST_WORKER );
+                                mode  = ( hasAWT ? Mode.ST_AWT : Mode.MT );
                             } else if (singleThreadProp.equals("worker")) {
-                                singleThreaded = true;
                                 mode = Mode.ST_WORKER;
                             } else if (hasAWT && singleThreadProp.equals("awt")) {
-                                singleThreaded = true;
                                 mode = Mode.ST_AWT;
                             } else if (singleThreadProp.equals("false")) {
-                                singleThreaded = false;
                                 mode = Mode.MT;
                             } else {
                                 throw new RuntimeException("Unsupported value for property jogl.1thread: "+singleThreadProp+", should be [true/auto, worker, awt or false]");
                             }
+                        } else {
+                            mode  = ( hasAWT ? Mode.ST_AWT : Mode.MT );
                         }
+                        singleThreaded = Mode.MT != mode;
 
                         ToolkitThreadingPlugin threadingPlugin=null;
                         if(hasAWT) {
@@ -155,9 +141,11 @@ public class ThreadingImpl {
         method. This method should be called as early as possible in an
         application. */
     public static final void disableSingleThreading() {
-        singleThreaded = false;
-        if (Debug.verbose()) {
-            System.err.println("Application forced disabling of single-threading of javax.media.opengl implementation");
+        if( Mode.MT != mode ) {
+            singleThreaded = false;
+            if (Debug.verbose()) {
+                System.err.println("Application forced disabling of single-threading of javax.media.opengl implementation");
+            }
         }
     }
 
@@ -167,22 +155,28 @@ public class ThreadingImpl {
         return singleThreaded;
     }
 
-    /** Indicates whether the current thread is the single thread on
-        which this implementation of the javax.media.opengl APIs
-        performs all of its OpenGL-related work. This method should only
-        be called if the single-thread model is in effect. */
+    /**
+     * Indicates whether the current thread is capable of
+     * performing OpenGL-related work.
+     * <p>
+     * Method always returns <code>true</code>
+     * if {@link #getMode()} == {@link Mode#MT} or {@link #isSingleThreaded()} == <code>false</code>.
+     * </p>
+     */
     public static final boolean isOpenGLThread() throws GLException {
-        if(null!=threadingPlugin) {
+        if( Mode.MT == mode || !singleThreaded ) {
+            return true;
+        } else if( null != threadingPlugin ) {
             return threadingPlugin.isOpenGLThread();
-        }
-
-        switch (mode) {
-            case ST_AWT:
-                throw new InternalError();
-            case ST_WORKER:
-                return GLWorkerThread.isWorkerThread();
-            default:
-                throw new InternalError("Illegal single-threading mode " + mode);
+        } else {
+            switch (mode) {
+                case ST_AWT:
+                    throw new InternalError();
+                case ST_WORKER:
+                    return GLWorkerThread.isWorkerThread();
+                default:
+                    throw new InternalError("Illegal single-threading mode " + mode);
+            }
         }
     }
 
@@ -213,6 +207,10 @@ public class ThreadingImpl {
                 invokeOnWorkerThread(wait, r);
                 break;
 
+            case MT:
+                r.run();
+                break;
+
             default:
                 throw new InternalError("Illegal single-threading mode " + mode);
         }
diff --git a/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java b/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java
index 26ec62785..3f8910fb5 100644
--- a/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java
+++ b/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java
@@ -108,6 +108,10 @@ public class AWTThreadingPlugin implements ToolkitThreadingPlugin {
         ThreadingImpl.invokeOnWorkerThread(wait, r);
         break;
 
+      case MT:
+        r.run();
+        break;
+
       default:
         throw new InternalError("Illegal single-threading mode " + ThreadingImpl.getMode());
     }