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/javax/media/opengl/GLAutoDrawable.java | 90 ++++++++++++++++++---- src/jogl/classes/javax/media/opengl/GLContext.java | 18 ++++- src/jogl/classes/javax/media/opengl/Threading.java | 42 ++++++++-- .../classes/javax/media/opengl/awt/GLCanvas.java | 10 ++- .../classes/javax/media/opengl/awt/GLJPanel.java | 15 +++- 5 files changed, 146 insertions(+), 29 deletions(-) (limited to 'src/jogl/classes/javax/media/opengl') 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

+
GLAutoDrawable Locking
+ GLAutoDrawable implementations perform locking in the following order: +
    +
  1. {@link #getUpstreamLock()}.{@link RecursiveLock#lock() lock()}
    2. +
  2. {@link #getNativeSurface()}.{@link NativeSurface#lockSurface() lockSurface()}
    3. +
+ and releases the locks accordingly: +
    +
  1. {@link #getNativeSurface()}.{@link NativeSurface#unlockSurface() unlockSurface()}
    2. +
  2. {@link #getUpstreamLock()}.{@link RecursiveLock#unlock() unlock()}
    3. +
+ Above locking order is mandatory to guarantee + atomicity of operation and to avoid race-conditions. + A custom implementation or user applications requiring exclusive access + shall follow the locking order. + See: + +

*/ 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, newtCtx, to this auto-drawable. *

- * The current context will be destroyed if destroyPrevCtx is true, - * otherwise it will be dis-associated from this auto-drawable - * via {@link GLContext#setGLDrawable(GLDrawable, boolean) setGLDrawable(null, true);} first. - *

- *

- * The new context will be associated with this auto-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 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: + *

*

* * @param newCtx the new context, maybe null 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. *

+ *

+ * To avoid a deadlock situation which causes an {@link IllegalStateException} one should + * avoid issuing {@link #invoke(boolean, GLRunnable) invoke} while this GLAutoDrawable is being locked.
+ * Detected deadlock situations throwing an {@link IllegalStateException} are: + *

+ *

* * @param wait if true block until execution of glRunnable is finished, otherwise return immediately 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 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 true block until execution of the last glRunnable is finished, otherwise return immediately w/o waiting * @param glRunnables the {@link GLRunnable}s to execute within {@link #display()} * @return true if the {@link GLRunnable}s has been processed or queued, otherwise false. + * @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 glRunnables); + public boolean invoke(boolean wait, List 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()}. + *

+ * See GLAutoDrawable Locking. + *

+ */ + public RecursiveLock getUpstreamLock(); + + /** + * Indicates whether the current thread is capable of + * performing OpenGL-related work. + *

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

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

* If the arguments reflect the current state of this context * this method is a no-operation and returns the old and current {@link GLDrawable}. *

*

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

    + *
  • {@link GL#glFinish() glFinish()} is issued if context {@link #isCreated()} and a {@link #getGLDrawable() previous drawable} was bound before disassociation.
    • + *
  • If the context was current on this thread, it is being released before drawable reassociation + * 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 readWrite The read/write drawable for framebuffer operations, maybe null to remove association. * @param setWriteOnly Only change the write-drawable, if setWriteOnly is true 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. + *

+ * Method always returns true + * if {@link #getMode()} == {@link Mode#MT} or {@link #isSingleThreaded()} == false. + *

+ */ public static final boolean isOpenGLThread() throws GLException { return ThreadingImpl.isOpenGLThread(); } @@ -173,7 +203,7 @@ public class Threading { } /** - * If {@link #isSingleThreaded()} and not {@link #isOpenGLThread()} + * If not {@link #isOpenGLThread()} * and the lock is not being hold by this thread, * invoke Runnable r on the OpenGL thread via {@link #invokeOnOpenGLThread(boolean, Runnable)}. *

@@ -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 @@ -299,6 +299,12 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing return this; } + @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 glRunnables) { + public boolean invoke(final boolean wait, final List 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; @@ -431,6 +436,12 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing return this; } + @Override + public final RecursiveLock getUpstreamLock() { return lock; } + + @Override + public final boolean isThreadGLCapable() { return EventQueue.isDispatchThread(); } + @Override public void display() { if( isShowing || ( printActive && isVisible() ) ) { @@ -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 glRunnables) { + public boolean invoke(final boolean wait, final List glRunnables) throws IllegalStateException { return helper.invoke(this, wait, glRunnables); } -- cgit v1.2.3