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, 146 insertions, 29 deletions

diff --git a/src/jogl/classes/javax/media/opengl/GLAutoDrawable.java b/src/jogl/classes/javax/media/opengl/GLAutoDrawable.java
index 377dce190..5745e197f 100644
--- a/src/jogl/classes/javax/media/opengl/GLAutoDrawable.java
+++ b/src/jogl/classes/javax/media/opengl/GLAutoDrawable.java
@@ -42,6 +42,10 @@ package javax.media.opengl;
 
 import java.util.List;
 
+import javax.media.nativewindow.NativeSurface;
+
+import com.jogamp.common.util.locks.RecursiveLock;
+
 import jogamp.opengl.Debug;
 
 /** A higher-level abstraction than {@link GLDrawable} which supplies
@@ -116,6 +120,28 @@ import jogamp.opengl.Debug;
     -Djogl.screenchange.action=true  Enable  the {@link GLDrawable} reconfiguration
     </PRE>
     </p>
+    <h5><a name="locking">GLAutoDrawable Locking</a></h5>
+    GLAutoDrawable implementations perform locking in the following order:
+    <ol>
+      <li> {@link #getUpstreamLock()}.{@link RecursiveLock#lock() lock()}</li>
+      <li> {@link #getNativeSurface()}.{@link NativeSurface#lockSurface() lockSurface()} </li>
+    </ol>
+    and releases the locks accordingly:
+    <ol>
+      <li> {@link #getNativeSurface()}.{@link NativeSurface#unlockSurface() unlockSurface()} </li>
+      <li> {@link #getUpstreamLock()}.{@link RecursiveLock#unlock() unlock()}</li>
+    </ol>
+    Above <i>locking order</i> is mandatory to guarantee
+    atomicity of operation and to avoid race-conditions.
+    A custom implementation or user applications requiring exclusive access
+    shall follow the <i>locking order</i>.
+    See:
+    <ul>
+      <li>{@link #getUpstreamLock()}</li>
+      <li>{@link #invoke(boolean, GLRunnable)}</li>
+      <li>{@link #invoke(boolean, List)}</li>
+    </ul>
+    </p>
   */
 public interface GLAutoDrawable extends GLDrawable {
   /** Flag reflecting whether the {@link GLDrawable} reconfiguration will be issued in
@@ -139,19 +165,22 @@ public interface GLAutoDrawable extends GLDrawable {
   /**
    * Associate the new context, <code>newtCtx</code>, to this auto-drawable.
    * <p>
-   * The current context will be destroyed if <code>destroyPrevCtx</code> is <code>true</code>,
-   * otherwise it will be dis-associated from this auto-drawable
-   * via {@link GLContext#setGLDrawable(GLDrawable, boolean) setGLDrawable(null, true);} first.
-   * </p>
-   * <p>
-   * The new context will be associated with this auto-drawable
-   * 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 association.
-   * The new context will be made current afterwards, if it was current before.
-   * However the user shall take extra care that no other thread
-   * attempts to make this context current.
+   * Remarks:
+   * <ul>
+   *   <li>The currently associated context will be destroyed if <code>destroyPrevCtx</code> is <code>true</code>,
+   *       otherwise it will be disassociated from this auto-drawable
+   *       via {@link GLContext#setGLDrawable(GLDrawable, boolean) setGLDrawable(null, true);} including {@link GL#glFinish() glFinish()}.</li>
+   *   <li>The new context will be associated with this auto-drawable
+   *       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 this auto-drawable.</li>
+   *   <li>If the new context was current on this thread, it is being released before associating this auto-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 GLAutoDrawable auto-drawable's}
+   *       {@link GLAutoDrawable#getUpstreamLock() upstream-locks} and {@link GLAutoDrawable#getNativeSurface() surfaces}
+   *       to avoid a race condition. See <a href="#locking">GLAutoDrawable Locking</a>.</li>
+   * </ul>
    * </p>
    *
    * @param newCtx the new context, maybe <code>null</code> for dis-association.
@@ -410,17 +439,27 @@ public interface GLAutoDrawable extends GLDrawable {
    * The internal queue of {@link GLRunnable}'s is being flushed with {@link #destroy()}
    * where all blocked callers are being notified.
    * </p>
+   * <p>
+   * To avoid a deadlock situation which causes an {@link IllegalStateException} one should
+   * avoid issuing {@link #invoke(boolean, GLRunnable) invoke} while this <a href="#locking">GLAutoDrawable is being locked</a>.<br>
+   * Detected deadlock situations throwing an {@link IllegalStateException} are:
+   * <ul>
+   *   <li>{@link #getAnimator() Animator} is running on another thread and waiting and is locked on current thread, but is not {@link #isThreadGLCapable() GL-Thread}</li>
+   *   <li>No {@link #getAnimator() Animator} is running on another thread and is locked on current thread, but is not {@link #isThreadGLCapable() GL-Thread}</li>
+   * </ul>
+   * </p>
    *
    * @param wait if <code>true</code> block until execution of <code>glRunnable</code> is finished, otherwise return immediately 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 of a detected deadlock situation ahead, see above.
    *
    * @see #setAnimator(GLAnimatorControl)
    * @see #display()
    * @see GLRunnable
    * @see #invoke(boolean, List)
    */
-  public boolean invoke(boolean wait, GLRunnable glRunnable);
+  public boolean invoke(boolean wait, GLRunnable glRunnable) throws IllegalStateException ;
 
   /**
    * Extends {@link #invoke(boolean, GLRunnable)} functionality
@@ -428,9 +467,10 @@ public interface GLAutoDrawable extends GLDrawable {
    * @param wait if <code>true</code> block until execution of the last <code>glRunnable</code> is finished, otherwise return immediately w/o waiting
    * @param glRunnables the {@link GLRunnable}s to execute within {@link #display()}
    * @return <code>true</code> if the {@link GLRunnable}s has been processed or queued, otherwise <code>false</code>.
+   * @throws IllegalStateException in case of a detected deadlock situation ahead, see {@link #invoke(boolean, GLRunnable)}.
    * @see #invoke(boolean, GLRunnable)
    */
-  public boolean invoke(boolean wait, List<GLRunnable> glRunnables);
+  public boolean invoke(boolean wait, List<GLRunnable> glRunnables) throws IllegalStateException;
 
   /** Destroys all resources associated with this GLAutoDrawable,
       inclusive the GLContext.
@@ -556,4 +596,24 @@ public interface GLAutoDrawable extends GLDrawable {
    */
   public Object getUpstreamWidget();
 
+  /**
+   * Returns the recursive lock object of the {@link #getUpstreamWidget() upstream widget}
+   * to synchronize multithreaded access on top of {@link NativeSurface#lockSurface()}.
+   * <p>
+   * See <a href="#locking">GLAutoDrawable Locking</a>.
+   * </p>
+   */
+  public RecursiveLock getUpstreamLock();
+
+  /**
+   * Indicates whether the current thread is capable of
+   * performing OpenGL-related work.
+   * <p>
+   * Implementation utilizes this knowledge to determine
+   * whether {@link #display()} performs the OpenGL commands on the current thread directly
+   * or spawns them on the dedicated OpenGL thread.
+   * </p>
+   */
+  public boolean isThreadGLCapable();
+
 }
diff --git a/src/jogl/classes/javax/media/opengl/GLContext.java b/src/jogl/classes/javax/media/opengl/GLContext.java
index d5d8792d8..6fb943613 100644
--- a/src/jogl/classes/javax/media/opengl/GLContext.java
+++ b/src/jogl/classes/javax/media/opengl/GLContext.java
@@ -48,6 +48,7 @@ import java.util.List;
 import java.util.Set;
 
 import javax.media.nativewindow.AbstractGraphicsDevice;
+import javax.media.nativewindow.NativeSurface;
 
 import jogamp.opengl.Debug;
 import jogamp.opengl.GLContextImpl;
@@ -282,15 +283,24 @@ public abstract class GLContext {
   }
 
   /**
-   * Sets the read/write drawable for framebuffer operations.
+   * Sets the read/write drawable for framebuffer operations, i.e. reassociation of the context's drawable.
    * <p>
    * If the arguments reflect the current state of this context
    * this method is a no-operation and returns the old and current {@link GLDrawable}.
    * </p>
    * <p>
-   * If the context was current on this thread, it is being released before switching the drawable
-   * and made current afterwards. However the user shall take extra care that not other thread
-   * attempts to make this context current. Otherwise a race condition may happen.
+   * Remarks:
+   * <ul>
+   *   <li>{@link GL#glFinish() glFinish()} is issued if context {@link #isCreated()} and a {@link #getGLDrawable() previous drawable} was bound before disassociation.</li>
+   *   <li>If the context was current on this thread, it is being released before drawable reassociation
+   *       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="GLAutoDrawable.html#locking">GLAutoDrawable Locking</a>.</li>
+   * </ul>
    * </p>
    * @param readWrite The read/write drawable for framebuffer operations, maybe <code>null</code> to remove association.
    * @param setWriteOnly Only change the write-drawable, if <code>setWriteOnly</code> is <code>true</code> and
diff --git a/src/jogl/classes/javax/media/opengl/Threading.java b/src/jogl/classes/javax/media/opengl/Threading.java
index 6c64cbe31..c8d8d0071 100644
--- a/src/jogl/classes/javax/media/opengl/Threading.java
+++ b/src/jogl/classes/javax/media/opengl/Threading.java
@@ -117,10 +117,36 @@ import jogamp.opengl.ThreadingImpl;
 */
 
 public class Threading {
+    public static enum Mode {
+        /**
+         * Full multithreaded OpenGL,
+         * i.e. any {@link Threading#invoke(boolean, Runnable, Object) invoke}
+         * {@link Threading#invokeOnOpenGLThread(boolean, Runnable) commands}
+         * will be issued on the current thread immediately.
+         */
+        MT(0),
+
+        /** Single-Threaded OpenGL on AWT EDT */
+        ST_AWT(1),
+
+        /** Single-Threaded OpenGL on dedicated worker thread. */
+        ST_WORKER(2);
+
+        public final int id;
+
+        Mode(final int id){
+            this.id = id;
+        }
+    }
 
     /** No reason to ever instantiate this class */
     private Threading() {}
 
+    /** Returns the threading mode */
+    public static Mode getMode() {
+        return ThreadingImpl.getMode();
+    }
+
     /** If an implementation of the javax.media.opengl APIs offers a
         multithreading option but the default behavior is single-threading,
         this API provides a mechanism for end users to disable single-threading
@@ -150,10 +176,14 @@ public class Threading {
         return ThreadingImpl.isToolkitThread();
     }
 
-    /** 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 {
         return ThreadingImpl.isOpenGLThread();
     }
@@ -173,7 +203,7 @@ public class Threading {
     }
 
     /**
-     * If {@link #isSingleThreaded()} <b>and</b> not {@link #isOpenGLThread()}
+     * If not {@link #isOpenGLThread()}
      * <b>and</b> the <code>lock</code> is not being hold by this thread,
      * invoke Runnable <code>r</code> on the OpenGL thread via {@link #invokeOnOpenGLThread(boolean, Runnable)}.
      * <p>
@@ -186,7 +216,7 @@ public class Threading {
      * @throws GLException
      */
     public static final void invoke(final boolean wait, final Runnable r, final Object lock) throws GLException {
-        if ( isSingleThreaded() && !isOpenGLThread() &&
+        if ( !isOpenGLThread() &&
              ( null == lock || !Thread.holdsLock(lock) ) ) {
             invokeOnOpenGLThread(wait, r);
         } else {
diff --git a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
index 2d5e12429..a8aa7382d 100644
--- a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
+++ b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
@@ -300,6 +300,12 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
   }
 
   @Override
+  public final RecursiveLock getUpstreamLock() { return lock; }
+
+  @Override
+  public final boolean isThreadGLCapable() { return Threading.isOpenGLThread(); }
+
+  @Override
   public void setShallUseOffscreenLayer(final boolean v) {
       shallUseOffscreenLayer = v;
   }
@@ -1040,12 +1046,12 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
   }
 
   @Override
-  public boolean invoke(final boolean wait, final GLRunnable glRunnable) {
+  public 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);
   }
 
diff --git a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
index 549b6e96f..f0ba08c3e 100644
--- a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
+++ b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
@@ -98,6 +98,8 @@ import jogamp.opengl.util.glsl.GLSLTextureRaster;
 
 import com.jogamp.common.util.PropertyAccess;
 import com.jogamp.common.util.awt.AWTEDTExecutor;
+import com.jogamp.common.util.locks.LockFactory;
+import com.jogamp.common.util.locks.RecursiveLock;
 import com.jogamp.nativewindow.awt.AWTPrintLifecycle;
 import com.jogamp.nativewindow.awt.AWTWindowClosingProtocol;
 import com.jogamp.opengl.FBObject;
@@ -232,6 +234,9 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
       return singleAWTGLPixelBufferProvider;
   }
 
+  /** Currently not used internally, exist merely to satisfy {@link #getUpstreamLock()}. */
+  private final RecursiveLock lock = LockFactory.createRecursiveLock();
+
   private final GLDrawableHelper helper;
   private boolean autoSwapBufferMode;
 
@@ -432,6 +437,12 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
   }
 
   @Override
+  public final RecursiveLock getUpstreamLock() { return lock; }
+
+  @Override
+  public final boolean isThreadGLCapable() { return EventQueue.isDispatchThread(); }
+
+  @Override
   public void display() {
     if( isShowing || ( printActive && isVisible() ) ) {
         if (EventQueue.isDispatchThread()) {
@@ -927,12 +938,12 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
   }
 
   @Override
-  public boolean invoke(final boolean wait, final GLRunnable glRunnable) {
+  public 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);
   }