From c77b8f586cb2553582a42f5b90aeee5ef85f1efe Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Sun, 27 Jul 2014 03:49:21 +0200 Subject: 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 --- .../classes/jogamp/opengl/GLAutoDrawableBase.java | 36 +++--- src/jogl/classes/jogamp/opengl/GLContextImpl.java | 2 + .../classes/jogamp/opengl/GLDrawableHelper.java | 134 +++++++++++++++++---- src/jogl/classes/jogamp/opengl/ThreadingImpl.java | 70 ++++++----- .../jogamp/opengl/awt/AWTThreadingPlugin.java | 4 + 5 files changed, 172 insertions(+), 74 deletions(-) (limited to 'src/jogl/classes/jogamp/opengl') 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 true. * * @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 glRunnables) { + public boolean invoke(final boolean wait, final List glRunnables) throws IllegalStateException { return helper.invoke(this, wait, glRunnables); } @@ -604,6 +603,15 @@ public abstract class GLAutoDrawableBase implements GLAutoDrawable, GLStateKeepe return additionalCtxCreationFlags; } + /** + * {@inheritDoc} + *

+ * Implementation always supports multithreading, hence method always returns true. + *

+ */ + @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. *

- * The oldCtx will be destroyed if destroyPrevCtx is true, - * otherwise dis-associate oldCtx from drawable - * via {@link GLContext#setGLDrawable(GLDrawable, boolean) oldCtx.setGLDrawable(null, true);}. - *

- *

- * Re-associate newCtx with drawable - * via {@link GLContext#setGLDrawable(GLDrawable, boolean) newCtx.setGLDrawable(drawable, true);}. - *

- *

- * If the old or new context was current on this thread, it is being released before switching the drawable. - *

- *

- * No locking is being performed on the drawable, caller is required to take care of it. + * Remarks: + *

    + *
  • The oldCtx will be destroyed if destroyPrevCtx is true, + * otherwise disassociate oldCtx from drawable + * via {@link GLContext#setGLDrawable(GLDrawable, boolean) oldCtx.setGLDrawable(null, true);} including {@link GL#glFinish() glFinish()}.
    • + *
  • Reassociate newCtx with drawable + * via {@link GLContext#setGLDrawable(GLDrawable, boolean) newCtx.setGLDrawable(drawable, true);}.
    • + *
  • If the old context was current on this thread, it is being released after disassociating the drawable.
    • + *
  • If the new context was current on this thread, it is being released before associating the drawable + * and made current afterwards.
    • + *
  • Implementation may issue {@link #makeCurrent()} and {@link #release()} while drawable reassociation.
    • + *
  • 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 GLAutoDrawable Locking.
    • + *
*

* * @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; + } + } + /** *

* If wait is true the call blocks until the glRunnable @@ -805,13 +833,32 @@ public class GLDrawableHelper { * the call is ignored and returns false.
* This helps avoiding deadlocking the caller. *

+ *

+ * 

+   * 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
+   *
+ *

* * @param drawable the {@link GLAutoDrawable} to be used * @param wait if true block until execution of glRunnable is finished, otherwise return immediatly w/o waiting * @param glRunnable the {@link GLRunnable} to execute within {@link #display()} * @return true if the {@link GLRunnable} has been processed or queued, otherwise false. + * @throws IllegalStateException in case the drawable is locked by this thread, no animator is running on another thread and wait is true. */ - 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 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 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 + * Method always returns true + * if {@link #getMode()} == {@link Mode#MT} or {@link #isSingleThreaded()} == false. + *

+ */ 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()); } -- cgit v1.2.3