Fix Bug 691 (part-3): NSOpenGLLayer::openGLContextForPixelFormat(..) on main-thread deadlock'ed due to locked shared context

NSOpenGLLayer::openGLContextForPixelFormat(..) is performed on main-thread at 1st NSOpenGLLayer display method.
This happened irregulary, i.e. sometimes (T0) right after NSOpenGLLayer creation and attachSurfaceLayer()/AddCASublayer(..),
sometimes later (T1).

NSOpenGLLayer::openGLContextForPixelFormat(..) uses the passed shared user context.
The shared user context is locked at NSOpenGLLayer's creation (T0) and if performed at this early time
the call deadlocks due to pthread_mutex wait for the shared user context.

This fix performs NSOpenGLLayer creation and layer attachment while the shared user context
is kept unlocked and enforces NSOpenGLLayer display and hence NSOpenGLLayer::openGLContextForPixelFormat(..).

Added CGL.setNSOpenGLLayerEnabled(..) to enable/disable NSOpenGLLayer - currently not used.

- Passed AddRemove tests for GLCanvas/Swing and GLWindow/NewtCanvasAWT w/ 100 loops on Java6 and Java7 on OSX.
- Passed Instruments Leaks test w/ 10 loops on Java6 and Java7

11 files changed, 123 insertions, 21 deletions

diff --git a/make/config/jogl/cgl-macosx-CustomJavaCode.java b/make/config/jogl/cgl-macosx-CustomJavaCode.java
index b9a37d0c6..d32e0ae8f 100644
--- a/make/config/jogl/cgl-macosx-CustomJavaCode.java
+++ b/make/config/jogl/cgl-macosx-CustomJavaCode.java
@@ -1,6 +1,14 @@
 
 /**
  * Creates the NSOpenGLLayer for FBO/PBuffer w/ optional GL3 shader program on Main-Thread
+ * <p>
+ * It is mandatory that the shared context handle <code>ctx</code>
+ * is not locked while calling this method. 
+ * </p>
+ * <p>
+ * The NSOpenGLLayer starts in enabled mode, 
+ * you may enable/disable it via {@link #setNSOpenGLLayerEnabled(long, boolean)}.
+ * </p>
  */
 public static long createNSOpenGLLayer(final long ctx, final int gl3ShaderProgramName, final long fmt, final long p, 
                                        final int texID, final boolean opaque, final int texWidth, final int texHeight) {
@@ -11,6 +19,20 @@ public static long createNSOpenGLLayer(final long ctx, final int gl3ShaderProgra
 }
 
 /**
+ * Enable or disable NSOpenGLLayer. 
+ *
+ * <p>
+ * If disabled, the NSOpenGLLayer will not be displayed, i.e. rendered.
+ * </p>
+ */
+public static void setNSOpenGLLayerEnabled(final long nsOpenGLLayer, final boolean enable) {
+  OSXUtil.RunOnMainThread(true, new Runnable() {
+      public void run() {
+          setNSOpenGLLayerEnabledImpl(nsOpenGLLayer, enable);
+      } } );
+}
+
+/**
  * Releases the NSOpenGLLayer on Main-Thread
  */
 public static void releaseNSOpenGLLayer(final long nsOpenGLLayer) {
diff --git a/make/config/jogl/cgl-macosx.cfg b/make/config/jogl/cgl-macosx.cfg
index 98123f605..edb5bfbbf 100644
--- a/make/config/jogl/cgl-macosx.cfg
+++ b/make/config/jogl/cgl-macosx.cfg
@@ -35,8 +35,10 @@ CustomCCode #include </usr/include/machine/types.h>
 CustomCCode #include "macosx-window-system.h"
 
 AccessControl createNSOpenGLLayerImpl PRIVATE
+AccessControl setNSOpenGLLayerEnabledImpl PRIVATE
 AccessControl releaseNSOpenGLLayerImpl PRIVATE
 RenameJavaMethod createNSOpenGLLayer createNSOpenGLLayerImpl
+RenameJavaMethod setNSOpenGLLayerEnabled setNSOpenGLLayerEnabledImpl
 RenameJavaMethod releaseNSOpenGLLayer releaseNSOpenGLLayerImpl
 
 IncludeAs CustomJavaCode CGL cgl-macosx-CustomJavaCode.java
diff --git a/make/scripts/tests.sh b/make/scripts/tests.sh
index 559c2aa6a..a20ec507c 100755
--- a/make/scripts/tests.sh
+++ b/make/scripts/tests.sh
@@ -301,8 +301,8 @@ function testawtswt() {
 #testnoawt com.jogamp.opengl.test.junit.jogl.acore.ect.TestExclusiveContext12FPSAnimNEWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.acore.TestOffscreenLayer01GLCanvasAWT $*
 #testawt com.jogamp.opengl.test.junit.jogl.acore.TestOffscreenLayer02NewtCanvasAWT $*
-#testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove01GLCanvasSwingAWT $*
-testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove02GLWindowNewtCanvasAWT $*
+testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove01GLCanvasSwingAWT $*
+#testawt com.jogamp.opengl.test.junit.jogl.acore.TestAddRemove02GLWindowNewtCanvasAWT $*
 
 #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestGLAutoDrawableDelegateOnOffscrnCapsNEWT $*
 #testnoawt com.jogamp.opengl.test.junit.jogl.acore.TestGLAutoDrawableFactoryOffscrnCapsNEWT $*
diff --git a/make/stub_includes/opengl/macosx-window-system.h b/make/stub_includes/opengl/macosx-window-system.h
index a2da66878..402a16efc 100644
--- a/make/stub_includes/opengl/macosx-window-system.h
+++ b/make/stub_includes/opengl/macosx-window-system.h
@@ -56,6 +56,7 @@ void setContextTextureImageToPBuffer(NSOpenGLContext* ctx, NSOpenGLPixelBuffer*
 Bool isNSOpenGLPixelBuffer(uint64_t object);
 
 NSOpenGLLayer* createNSOpenGLLayer(NSOpenGLContext* ctx, int gl3ShaderProgramName, NSOpenGLPixelFormat* fmt, NSOpenGLPixelBuffer* p, uint32_t texID, Bool opaque, int texWidth, int texHeight);
+void setNSOpenGLLayerEnabled(NSOpenGLLayer* layer, Bool enable);
 void setNSOpenGLLayerSwapInterval(NSOpenGLLayer* layer, int interval);
 void waitUntilNSOpenGLLayerIsReady(NSOpenGLLayer* layer, long to_micros);
 void setNSOpenGLLayerNeedsDisplayFBO(NSOpenGLLayer* layer, uint32_t texID);
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
index 838a0387d..37aca0cd7 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
@@ -687,13 +687,32 @@ public abstract class MacOSXCGLContext extends GLContextImpl
                   } else {
                       gl3ShaderProgramName = 0;
                   }
-                  nsOpenGLLayer = CGL.createNSOpenGLLayer(ctx, gl3ShaderProgramName, nsOpenGLLayerPFmt, pbufferHandle, texID, chosenCaps.isBackgroundOpaque(), lastWidth, lastHeight);
-                  nsOpenGLLayerPFmt = 0; // NSOpenGLLayer will release pfmt
-                  if (DEBUG) {
-                      System.err.println("NS create nsOpenGLLayer "+toHexString(nsOpenGLLayer)+" w/ pbuffer "+toHexString(pbufferHandle)+", texID "+texID+", texSize "+lastWidth+"x"+lastHeight+", "+drawable);
+                   
+                  /**
+                   * Perform NSOpenGLLayer creation and attaching on main-thread,
+                   * hence release the lock on our context - which will be used to 
+                   * create a shared context within NSOpenGLLayer.
+                   */
+                  final long cglCtx = CGL.getCGLContext(ctx);
+                  if(0 == cglCtx) {
+                      throw new InternalError("Null CGLContext for: "+this);
+                  }
+                  final boolean ctxUnlocked = CGL.kCGLNoError == CGL.CGLUnlockContext(cglCtx);
+                  try {                  
+                      nsOpenGLLayer = CGL.createNSOpenGLLayer(ctx, gl3ShaderProgramName, nsOpenGLLayerPFmt, pbufferHandle, texID, chosenCaps.isBackgroundOpaque(), lastWidth, lastHeight);
+                      nsOpenGLLayerPFmt = 0; // NSOpenGLLayer will release pfmt
+                      if (DEBUG) {
+                          System.err.println("NS create nsOpenGLLayer "+toHexString(nsOpenGLLayer)+" w/ pbuffer "+toHexString(pbufferHandle)+", texID "+texID+", texSize "+lastWidth+"x"+lastHeight+", "+drawable);
+                      }
+                      backingLayerHost.attachSurfaceLayer(nsOpenGLLayer);
+                      setSwapInterval(1); // enabled per default in layered surface
+                  } finally {
+                      if( ctxUnlocked ) {
+                          if( CGL.kCGLNoError != CGL.CGLLockContext(cglCtx) ) {
+                              throw new InternalError("Could not re-lock CGLContext for: "+this);
+                          }
+                      }
                   }
-                  backingLayerHost.attachSurfaceLayer(nsOpenGLLayer);
-                  setSwapInterval(1); // enabled per default in layered surface                
               } else {
                   lastWidth = drawable.getWidth();
                   lastHeight = drawable.getHeight();                  
diff --git a/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m b/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m
index 8d1286169..55c4ad053 100644
--- a/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m
+++ b/src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m
@@ -130,6 +130,7 @@ extern GLboolean glIsVertexArray (GLuint array);
 @private
     GLfloat gl_texCoords[8];
     NSOpenGLContext* myCtx;
+    Bool isGLEnabled;
 
 @protected
     NSOpenGLContext* parentCtx;
@@ -171,6 +172,7 @@ extern GLboolean glIsVertexArray (GLuint array);
        texWidth: (int) texWidth 
        texHeight: (int) texHeight;
 
+- (void) setGLEnabled: (Bool) enable;
 - (Bool) validateTexSizeWithNewSize;
 - (Bool) validateTexSize: (int) _texWidth texHeight: (int) _texHeight;
 - (void) setTextureID: (int) _texID;
@@ -266,6 +268,7 @@ static const GLfloat gl_verts[] = {
     swapIntervalCounter = 0;
     timespec_now(&lastWaitTime);
     shallDraw = NO;
+    isGLEnabled = YES;
     newTexWidth = _texWidth;
     newTexHeight = _texHeight;
     [self validateTexSizeWithNewSize];
@@ -335,6 +338,12 @@ static const GLfloat gl_verts[] = {
     return self;
 }
 
+- (void) setGLEnabled: (Bool) enable
+{
+    DBG_PRINT("MyNSOpenGLLayer::setGLEnabled: %p, %d -> %d\n", self, (int)isGLEnabled, (int)enable);
+    isGLEnabled = enable;
+}
+
 - (Bool) validateTexSizeWithNewSize
 {
     return [self validateTexSize: newTexWidth texHeight: newTexHeight];
@@ -444,6 +453,7 @@ static const GLfloat gl_verts[] = {
 {
     DBG_PRINT("MyNSOpenGLLayer::openGLPixelFormatForDisplayMask: %p (refcnt %d) - parent-pfmt %p -> new-pfmt %p\n", 
         self, (int)[self retainCount], parentPixelFmt, parentPixelFmt);
+    // We simply take over ownership of parent PixelFormat ..
     return parentPixelFmt;
 }
 
@@ -531,7 +541,6 @@ static const GLfloat gl_verts[] = {
     [self disableAnimation];
     pthread_mutex_lock(&renderLock);
     [self deallocPBuffer];
-    // [[self openGLContext] dealloc];
     pthread_mutex_unlock(&renderLock);
     pthread_cond_destroy(&renderSignal);
     pthread_mutex_destroy(&renderLock);
@@ -548,8 +557,8 @@ static const GLfloat gl_verts[] = {
  {
     CGRect lRectS = [[self superlayer] bounds];
 
-    DBG_PRINT("MyNSOpenGLLayer::resizeWithOldSuperlayerSize: %p, texSize %dx%d, bounds: %lfx%lf -> %lfx%lf (refcnt %d)\n", 
-        self, texWidth, texHeight, size.width, size.height, lRectS.size.width, lRectS.size.height, (int)[self retainCount]);
+    DBG_PRINT("MyNSOpenGLLayer::resizeWithOldSuperlayerSize: %p, texSize %dx%d, bounds: %lfx%lf -> %lf/%lf %lfx%lf (refcnt %d)\n", 
+        self, texWidth, texHeight, size.width, size.height, lRectS.origin.x, lRectS.origin.y, lRectS.size.width, lRectS.size.height, (int)[self retainCount]);
 
     newTexWidth = lRectS.size.width;
     newTexHeight = lRectS.size.height;
@@ -562,8 +571,8 @@ static const GLfloat gl_verts[] = {
 - (BOOL)canDrawInOpenGLContext:(NSOpenGLContext *)context pixelFormat:(NSOpenGLPixelFormat *)pixelFormat 
         forLayerTime:(CFTimeInterval)timeInterval displayTime:(const CVTimeStamp *)timeStamp
 {
-    SYNC_PRINT("<? %d>", (int)shallDraw);
-    return shallDraw;
+    SYNC_PRINT("<? %d, %d>", (int)shallDraw, (int)isGLEnabled);
+    return shallDraw && isGLEnabled;
 }
 
 - (void)drawInOpenGLContext:(NSOpenGLContext *)context pixelFormat:(NSOpenGLPixelFormat *)pixelFormat 
@@ -573,7 +582,7 @@ static const GLfloat gl_verts[] = {
     SYNC_PRINT("<* ");
     // NSLog(@"MyNSOpenGLLayer::DRAW: %@",[NSThread callStackSymbols]);
 
-    if( shallDraw && ( NULL != pbuffer || NULL != newPBuffer || 0 != textureID ) ) {
+    if( isGLEnabled && shallDraw && ( NULL != pbuffer || NULL != newPBuffer || 0 != textureID ) ) {
         [context makeCurrentContext];
 
         if( NULL != newPBuffer ) { // volatile OK
@@ -825,6 +834,13 @@ NSOpenGLLayer* createNSOpenGLLayer(NSOpenGLContext* ctx, int gl3ShaderProgramNam
   return [[[MyNSOpenGLLayer alloc] init] setupWithContext:ctx gl3ShaderProgramName: (GLuint)gl3ShaderProgramName pixelFormat: fmt pbuffer: p texIDArg: (GLuint)texID
                                                               opaque: opaque texWidth: texWidth texHeight: texHeight];
 }
+ 
+void setNSOpenGLLayerEnabled(NSOpenGLLayer* layer, Bool enable) {
+    NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
+    MyNSOpenGLLayer* l = (MyNSOpenGLLayer*) layer;
+    [l setGLEnabled: enable];
+    [pool release];
+}
 
 void setNSOpenGLLayerSwapInterval(NSOpenGLLayer* layer, int interval) {
     NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
diff --git a/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java b/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java
index f6bc5822b..ba60a7f38 100644
--- a/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java
+++ b/src/nativewindow/classes/javax/media/nativewindow/OffscreenLayerSurface.java
@@ -33,6 +33,14 @@ package javax.media.nativewindow;
 public interface OffscreenLayerSurface {
   /** 
    * Attach the offscreen layer to this offscreen layer surface.
+   * <p>
+   * Implementation may realize all required resources at this point.
+   * </p>
+   * <p>
+   * It is mandatory that any related resources, e.g. a shared context,
+   * are not locked while calling this method. 
+   * </p>
+   * 
    * @see #isOffscreenLayerSurfaceEnabled()
    * @throws NativeWindowException if {@link #isOffscreenLayerSurfaceEnabled()} == false
    */
diff --git a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
index b765a68c3..d85d1a84b 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
@@ -136,12 +136,31 @@ public class OSXUtil implements ToolkitProperties {
       return GetNSWindow0(nsView);
     }
     
+    /** 
+     * Create a CALayer suitable to act as a root CALayer on the main-thread.
+     * @see #DestroyCALayer(long)
+     * @see #AddCASublayer(long, long) 
+     */
     public static long CreateCALayer(final int x, final int y, final int width, final int height) {
       return OSXUtil.RunOnMainThread(true, new Function<Long, Object>() {
        public Long eval(Object... args) {
            return Long.valueOf( CreateCALayer0(x, y, width, height) );
        } } ).longValue();
     }
+    
+    /** 
+     * Attach a sub CALayer to the root CALayer on the main-thread.
+     * <p>
+     * Method will trigger a <code>display</code>
+     * call to the CALayer hierarchy to enforce resource creation if required, e.g. an NSOpenGLContext.
+     * </p>
+     * <p>
+     * It is mandatory that any related resources, e.g. a shared NSOpenGLContext,
+     * are not locked while calling this method. 
+     * </p>
+     * @see #CreateCALayer(int, int, int, int)
+     * @see #RemoveCASublayer(long, long)
+     */
     public static void AddCASublayer(final long rootCALayer, final long subCALayer) {
         if(0==rootCALayer || 0==subCALayer) {
             throw new IllegalArgumentException("rootCALayer 0x"+Long.toHexString(rootCALayer)+", subCALayer 0x"+Long.toHexString(subCALayer));
@@ -152,6 +171,10 @@ public class OSXUtil implements ToolkitProperties {
            }
         });
     }
+    
+    /** 
+     * Detach a sub CALayer from the root CALayer on the main-thread.
+     */
     public static void RemoveCASublayer(final long rootCALayer, final long subCALayer) {
         if(0==rootCALayer || 0==subCALayer) {
             throw new IllegalArgumentException("rootCALayer 0x"+Long.toHexString(rootCALayer)+", subCALayer 0x"+Long.toHexString(subCALayer));
@@ -161,6 +184,11 @@ public class OSXUtil implements ToolkitProperties {
                RemoveCASublayer0(rootCALayer, subCALayer);
            } } );
     }
+    
+    /** 
+     * Destroy a CALayer on the main-thread.
+     * @see #CreateCALayer(int, int, int, int)
+     */    
     public static void DestroyCALayer(final long caLayer) {
         if(0==caLayer) {
             throw new IllegalArgumentException("caLayer 0x"+Long.toHexString(caLayer));
diff --git a/src/nativewindow/native/macosx/OSXmisc.m b/src/nativewindow/native/macosx/OSXmisc.m
index 83f3c821f..8d876c175 100644
--- a/src/nativewindow/native/macosx/OSXmisc.m
+++ b/src/nativewindow/native/macosx/OSXmisc.m
@@ -430,6 +430,9 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AddCASublayer0
     MyCALayer* rootLayer = (MyCALayer*) ((intptr_t) rootCALayer);
     CALayer* subLayer = (CALayer*) ((intptr_t) subCALayer);
 
+    [CATransaction begin];
+    [CATransaction setValue:(id)kCFBooleanTrue forKey:kCATransactionDisableActions];
+
     CGRect lRectRoot = [rootLayer frame];
     DBG_PRINT("CALayer::AddCASublayer0.0: Origin %p frame0: %lf/%lf %lfx%lf\n", 
         rootLayer, lRectRoot.origin.x, lRectRoot.origin.y, lRectRoot.size.width, lRectRoot.size.height);
@@ -444,9 +447,6 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AddCASublayer0
         rootLayer, (int)[rootLayer retainCount],
         subLayer, lRectRoot.origin.x, lRectRoot.origin.y, lRectRoot.size.width, lRectRoot.size.height, (int)[subLayer retainCount]);
 
-    [CATransaction begin];
-    [CATransaction setValue:(id)kCFBooleanTrue forKey:kCATransactionDisableActions];
-
     // simple 1:1 layout !
     [subLayer setFrame:lRectRoot];
     [rootLayer addSublayer:subLayer];
@@ -454,14 +454,20 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AddCASublayer0
     // no animations for add/remove/swap sublayers etc 
     // doesn't work: [layer removeAnimationForKey: kCAOnOrderIn, kCAOnOrderOut, kCATransition]
     [rootLayer removeAllAnimations];
-    // [rootLayer addAnimation:nil forKey:kCATransition];
+    // [rootLayer addAnimation:nil forKey:kCATransition]; // JAU
     [rootLayer setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)];
     [rootLayer setNeedsDisplayOnBoundsChange: YES];
     [subLayer removeAllAnimations];
-    // [sublayer addAnimation:nil forKey:kCATransition];
+    // [subLayer addAnimation:nil forKey:kCATransition]; // JAU
     [subLayer setAutoresizingMask: (kCALayerWidthSizable|kCALayerHeightSizable)];
     [subLayer setNeedsDisplayOnBoundsChange: YES];
 
+    // Trigger display and hence ctx creation.
+    // The latter is essential since since the parent-context lock is cleared
+    // only for this window of time (method call).
+    [rootLayer setNeedsDisplay];
+    [rootLayer displayIfNeeded];
+
     [CATransaction commit];
 
     [pool release];
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java
index 2038124b5..61652371b 100644
--- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove01GLCanvasSwingAWT.java
@@ -140,8 +140,8 @@ public class TestAddRemove01GLCanvasSwingAWT extends UITestCase {
         SwingUtilities.invokeAndWait(new Runnable() {
                 public void run() {
                     if( visible ) {
-                        jFrame.validate();
                         jFrame.pack();
+                        jFrame.validate();
                     }
                     jFrame.setVisible(visible);
                 } } ) ;        
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java
index 0f12f967a..b4272a9c0 100644
--- a/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/acore/TestAddRemove02GLWindowNewtCanvasAWT.java
@@ -142,8 +142,8 @@ public class TestAddRemove02GLWindowNewtCanvasAWT extends UITestCase {
         SwingUtilities.invokeAndWait(new Runnable() {
                 public void run() {
                     if( visible ) {
-                        jFrame.validate();
                         jFrame.pack();
+                        jFrame.validate();
                     }
                     jFrame.setVisible(visible);
                 } } ) ;