Skip to content

Commit

Permalink
OSX/CALayer: Revise CALayer 'RunOnMainThread' utilization, avoiding d…
Browse files Browse the repository at this point in the history
…eadlocks

RunOnMainThread(waitUntilDone:=true,..) can deadlock the main-thread if called from AWT-EDT,
since the main-thread may call back to AWT-EDT while injecting a new main-thread task.

This patch revises all RunOnMainThread CALayer usage, resulting in only one required left:
  - OSXUtil.AddCASublayer() w/ waitUntilDone:=false

Hence the CALayer code has no more potential to deadlock main-thread/AWT-EDT.

OSXUtil.AddCASublayer() must be performed on main-thread, otherwise the
CALayer attachment will fail - no visible rendering result.

+++

Note: A good trigger to test this deadlock is to magnify/zoom
      the OSX desktop (click background + ctrl-mouse_wheel)
      before running some unit tests.

      TestGLCanvasAWTActionDeadlock01AWT and TestAddRemove02GLWindowNewtCanvasAWT
      also have the potential to trigger the mentioned deadlock.
  • Loading branch information
sgothel committed Mar 14, 2013
1 parent 58f6f4e commit 896e8b0
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 123 deletions.
23 changes: 7 additions & 16 deletions make/config/jogl/cgl-macosx-CustomJavaCode.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@

/**
* Creates the NSOpenGLLayer for FBO/PBuffer w/ optional GL3 shader program on Main-Thread
* Creates the NSOpenGLLayer for FBO/PBuffer w/ optional GL3 shader program
* <p>
* It is mandatory that the shared context handle <code>ctx</code>
* is not locked while calling this method.
* The NSOpenGLLayer will immediatly create a OpenGL context sharing the given ctx,
* which will be used to render the texture offthread.
* </p>
* <p>
* The NSOpenGLLayer starts in enabled mode,
Expand All @@ -12,10 +12,7 @@
*/
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) {
return OSXUtil.RunOnMainThread(true, new Function<Long, Object>() {
public Long eval(Object... args) {
return Long.valueOf( createNSOpenGLLayerImpl(ctx, gl3ShaderProgramName, fmt, p, texID, opaque, texWidth, texHeight) );
} } ).longValue();
return createNSOpenGLLayerImpl(ctx, gl3ShaderProgramName, fmt, p, texID, opaque, texWidth, texHeight);
}

/**
Expand All @@ -26,19 +23,13 @@ public Long eval(Object... args) {
* </p>
*/
public static void setNSOpenGLLayerEnabled(final long nsOpenGLLayer, final boolean enable) {
OSXUtil.RunOnMainThread(true, new Runnable() {
public void run() {
setNSOpenGLLayerEnabledImpl(nsOpenGLLayer, enable);
} } );
setNSOpenGLLayerEnabledImpl(nsOpenGLLayer, enable);
}

/**
* Releases the NSOpenGLLayer on Main-Thread
* Releases the NSOpenGLLayer
*/
public static void releaseNSOpenGLLayer(final long nsOpenGLLayer) {
OSXUtil.RunOnMainThread(true, new Runnable() {
public void run() {
releaseNSOpenGLLayerImpl(nsOpenGLLayer);
} } );
releaseNSOpenGLLayerImpl(nsOpenGLLayer);
}

30 changes: 12 additions & 18 deletions src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -676,30 +676,24 @@ public void swapBuffers(boolean doubleBuffered) {
}

/**
* 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.
* NSOpenGLLayer creation is performed on the current thread,
* which immediately creates it's own GL ctx sharing this ctx
* not causing any locking issues.
*
* Subsequent attaching is performed on main-thread w/o blocking.
*
* This is a lock free operation.
*/
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, pixelFormat, pbufferHandle, texID, chosenCaps.isBackgroundOpaque(), lastWidth, lastHeight);
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);
}
}
nsOpenGLLayer = CGL.createNSOpenGLLayer(ctx, gl3ShaderProgramName, pixelFormat, pbufferHandle, texID, chosenCaps.isBackgroundOpaque(), lastWidth, lastHeight);
if (DEBUG) {
System.err.println("NS create nsOpenGLLayer "+toHexString(nsOpenGLLayer)+" w/ pbuffer "+toHexString(pbufferHandle)+", texID "+texID+", texSize "+lastWidth+"x"+lastHeight+", "+drawable);
}
backingLayerHost.layoutSurfaceLayer();
backingLayerHost.attachSurfaceLayer(nsOpenGLLayer);
setSwapInterval(1); // enabled per default in layered surface
} else {
lastWidth = drawable.getWidth();
lastHeight = drawable.getHeight();
Expand Down
36 changes: 15 additions & 21 deletions src/jogl/native/macosx/MacOSXWindowSystemInterface-calayer.m
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,10 @@ @interface MyNSOpenGLLayer: NSOpenGLLayer <NWDedicatedSize>
{
@private
GLfloat gl_texCoords[8];
NSOpenGLContext* myCtx;
NSOpenGLContext* glContext;
Bool isGLEnabled;

@protected
NSOpenGLContext* parentCtx;
GLuint gl3ShaderProgramName;
GLuint vboBufVert;
GLuint vboBufTexCoord;
Expand Down Expand Up @@ -247,7 +246,7 @@ static CVReturn renderMyNSOpenGLLayer(CVDisplayLinkRef displayLink,

@implementation MyNSOpenGLLayer

- (id) setupWithContext: (NSOpenGLContext*) _parentCtx
- (id) setupWithContext: (NSOpenGLContext*) parentCtx
gl3ShaderProgramName: (GLuint) _gl3ShaderProgramName
pixelFormat: (NSOpenGLPixelFormat*) _parentPixelFmt
pbuffer: (NSOpenGLPixelBuffer*) p
Expand All @@ -262,20 +261,19 @@ - (id) setupWithContext: (NSOpenGLContext*) _parentCtx
pthread_mutex_init(&renderLock, &renderLockAttr); // recursive
pthread_cond_init(&renderSignal, NULL); // no attribute

myCtx = NULL;
{
int i;
for(i=0; i<8; i++) {
gl_texCoords[i] = 0.0f;
}
}
parentCtx = _parentCtx;
parentPixelFmt = [_parentPixelFmt retain]; // until destruction
glContext = [[MyNSOpenGLContext alloc] initWithFormat:parentPixelFmt shareContext:parentCtx];
gl3ShaderProgramName = _gl3ShaderProgramName;
vboBufVert = 0;
vboBufTexCoord = 0;
vertAttrLoc = 0;
texCoordAttrLoc = 0;
parentPixelFmt = [_parentPixelFmt retain]; // until destruction
swapInterval = 1; // defaults to on (as w/ new GL profiles)
swapIntervalCounter = 0;
timespec_now(&lastWaitTime);
Expand Down Expand Up @@ -338,12 +336,12 @@ - (id) setupWithContext: (NSOpenGLContext*) _parentCtx
#ifdef VERBOSE_ON
CGRect lRect = [self bounds];
if(NULL != pbuffer) {
DBG_PRINT("MyNSOpenGLLayer::init (pbuffer) %p, ctx %p, pfmt %p, pbuffer %p, opaque %d, pbuffer %dx%d -> tex %dx%d, bounds: %lf/%lf %lfx%lf, displayLink %p (refcnt %d)\n",
self, parentCtx, parentPixelFmt, pbuffer, opaque, [pbuffer pixelsWide], [pbuffer pixelsHigh], texWidth, texHeight,
DBG_PRINT("MyNSOpenGLLayer::init (pbuffer) %p, pctx %p, pfmt %p, pbuffer %p, ctx %p, opaque %d, pbuffer %dx%d -> tex %dx%d, bounds: %lf/%lf %lfx%lf, displayLink %p (refcnt %d)\n",
self, parentCtx, parentPixelFmt, pbuffer, glContext, opaque, [pbuffer pixelsWide], [pbuffer pixelsHigh], texWidth, texHeight,
lRect.origin.x, lRect.origin.y, lRect.size.width, lRect.size.height, displayLink, (int)[self retainCount]);
} else {
DBG_PRINT("MyNSOpenGLLayer::init (texture) %p, ctx %p, pfmt %p, opaque %d, tex[id %d, %dx%d], bounds: %lf/%lf %lfx%lf, displayLink %p (refcnt %d)\n",
self, parentCtx, parentPixelFmt, opaque, (int)textureID, texWidth, texHeight,
DBG_PRINT("MyNSOpenGLLayer::init (texture) %p, pctx %p, pfmt %p, ctx %p, opaque %d, tex[id %d, %dx%d], bounds: %lf/%lf %lfx%lf, displayLink %p (refcnt %d)\n",
self, parentCtx, parentPixelFmt, glContext, opaque, (int)textureID, texWidth, texHeight,
lRect.origin.x, lRect.origin.y, lRect.size.width, lRect.size.height, displayLink, (int)[self retainCount]);
}
#endif
Expand Down Expand Up @@ -493,11 +491,10 @@ - (void)releaseLayer
pthread_mutex_lock(&renderLock);
[self deallocPBuffer];
// [[self openGLContext] release];
if( NULL != myCtx ) {
[myCtx release];
myCtx = NULL;
if( NULL != glContext ) {
[glContext release];
glContext = NULL;
}
parentCtx = NULL;
if( NULL != parentPixelFmt ) {
[parentPixelFmt release];
parentPixelFmt = NULL;
Expand Down Expand Up @@ -632,22 +629,19 @@ - (NSOpenGLPixelFormat *)openGLPixelFormatForDisplayMask:(uint32_t)mask

- (NSOpenGLContext *)openGLContextForPixelFormat:(NSOpenGLPixelFormat *)pixelFormat
{
DBG_PRINT("MyNSOpenGLLayer::openGLContextForPixelFormat.0: %p (refcnt %d) - pfmt %p, parent %p, DisplayLink %p\n",
self, (int)[self retainCount], pixelFormat, parentCtx, displayLink);
// NSLog(@"MyNSOpenGLLayer::openGLContextForPixelFormat: %@",[NSThread callStackSymbols]);
myCtx = [[MyNSOpenGLContext alloc] initWithFormat:pixelFormat shareContext:parentCtx];
DBG_PRINT("MyNSOpenGLLayer::openGLContextForPixelFormat.0: %p (refcnt %d) - pfmt %p, ctx %p, DisplayLink %p\n",
self, (int)[self retainCount], pixelFormat, glContext, displayLink);
#ifndef HAS_CADisplayLink
if(NULL != displayLink) {
CVReturn cvres;
DBG_PRINT("MyNSOpenGLLayer::openGLContextForPixelFormat.1: setup DisplayLink %p\n", displayLink);
cvres = CVDisplayLinkSetCurrentCGDisplayFromOpenGLContext(displayLink, [myCtx CGLContextObj], [pixelFormat CGLPixelFormatObj]);
cvres = CVDisplayLinkSetCurrentCGDisplayFromOpenGLContext(displayLink, [glContext CGLContextObj], [pixelFormat CGLPixelFormatObj]);
if(kCVReturnSuccess != cvres) {
DBG_PRINT("MyNSOpenGLLayer::init %p, CVDisplayLinkSetCurrentCGDisplayFromOpenGLContext failed: %d\n", self, cvres);
}
}
#endif
DBG_PRINT("MyNSOpenGLLayer::openGLContextForPixelFormat.X: new-ctx %p\n", myCtx);
return myCtx;
return glContext;
}

- (BOOL)canDrawInOpenGLContext:(NSOpenGLContext *)context pixelFormat:(NSOpenGLPixelFormat *)pixelFormat
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,6 @@ public interface OffscreenLayerSurface {
* <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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@
import javax.media.nativewindow.MutableSurface;
import javax.media.nativewindow.util.Point;

import com.jogamp.common.util.Function;
import com.jogamp.nativewindow.awt.JAWTWindow;

import jogamp.nativewindow.jawt.JAWT;
Expand Down Expand Up @@ -90,8 +89,11 @@ protected void invalidateNative() {
JAWT_DrawingSurfaceInfo dsi = null;
try {
dsi = ds.GetDrawingSurfaceInfo();
if(! UnsetJAWTRootSurfaceLayer(dsi.getBuffer(), rootSurfaceLayerHandle)) {
try {
UnsetJAWTRootSurfaceLayer(dsi.getBuffer(), rootSurfaceLayerHandle);
} catch (Exception e) {
System.err.println("Error clearing JAWT rootSurfaceLayerHandle "+toHexString(rootSurfaceLayerHandle));
e.printStackTrace();
}
} finally {
if ( null != dsi ) {
Expand Down Expand Up @@ -245,8 +247,12 @@ public Object run() {
rootSurfaceLayerHandle = OSXUtil.CreateCALayer(bounds.getX(), bounds.getY(), bounds.getWidth(), bounds.getHeight());
if(0 == rootSurfaceLayerHandle) {
errMsg = "Could not create root CALayer";
} else if(!SetJAWTRootSurfaceLayer(dsi.getBuffer(), rootSurfaceLayerHandle)) {
errMsg = "Could not set JAWT rootSurfaceLayerHandle "+toHexString(rootSurfaceLayerHandle);
} else {
try {
SetJAWTRootSurfaceLayer(dsi.getBuffer(), rootSurfaceLayerHandle);
} catch(Exception e) {
errMsg = "Could not set JAWT rootSurfaceLayerHandle "+toHexString(rootSurfaceLayerHandle)+", cause: "+e.getMessage();
}
}
}
}
Expand Down Expand Up @@ -307,22 +313,22 @@ public Point getLocationOnScreen(Point storage) {
}
protected Point getLocationOnScreenNativeImpl(final int x0, final int y0) { return null; }

private static boolean SetJAWTRootSurfaceLayer(final Buffer jawtDrawingSurfaceInfoBuffer, final long caLayer) {
return OSXUtil.RunOnMainThread(true, new Function<Boolean, Object>() {
public Boolean eval(Object... args) {
return Boolean.valueOf( SetJAWTRootSurfaceLayer0(jawtDrawingSurfaceInfoBuffer, caLayer) );
} } ).booleanValue();
/**
* Set the given root CALayer in the JAWT surface
*/
private static void SetJAWTRootSurfaceLayer(final Buffer jawtDrawingSurfaceInfoBuffer, final long caLayer) {
SetJAWTRootSurfaceLayer0(jawtDrawingSurfaceInfoBuffer, caLayer);
}

private static boolean UnsetJAWTRootSurfaceLayer(final Buffer jawtDrawingSurfaceInfoBuffer, final long caLayer) {
return OSXUtil.RunOnMainThread(true, new Function<Boolean, Object>() {
public Boolean eval(Object... args) {
return Boolean.valueOf( UnsetJAWTRootSurfaceLayer0(jawtDrawingSurfaceInfoBuffer, caLayer) );
} } ).booleanValue();
/**
* Unset the given root CALayer in the JAWT surface
*/
private static void UnsetJAWTRootSurfaceLayer(final Buffer jawtDrawingSurfaceInfoBuffer, final long caLayer) {
UnsetJAWTRootSurfaceLayer0(jawtDrawingSurfaceInfoBuffer, caLayer);
}

private static native boolean SetJAWTRootSurfaceLayer0(Buffer jawtDrawingSurfaceInfoBuffer, long caLayer);
private static native boolean UnsetJAWTRootSurfaceLayer0(Buffer jawtDrawingSurfaceInfoBuffer, long caLayer);
private static native void SetJAWTRootSurfaceLayer0(Buffer jawtDrawingSurfaceInfoBuffer, long caLayer);
private static native void UnsetJAWTRootSurfaceLayer0(Buffer jawtDrawingSurfaceInfoBuffer, long caLayer);

// Variables for lockSurface/unlockSurface
private JAWT_DrawingSurface ds;
Expand Down
37 changes: 12 additions & 25 deletions src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -137,26 +137,23 @@ public static long GetNSWindow(long nsView) {
}

/**
* Create a CALayer suitable to act as a root CALayer on the main-thread.
* Create a CALayer suitable to act as a root CALayer.
* @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();
return CreateCALayer0(x, y, width, height);
}

/**
* Attach a sub CALayer to the root CALayer on the main-thread w/ blocking.
* Attach a sub CALayer to the root CALayer on the main-thread w/o blocking.
* <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.
* Hence it is important that related resources are not locked <i>if</i>
* they will be used for creation.
* </p>
* @see #CreateCALayer(int, int, int, int)
* @see #RemoveCASublayer(long, long)
Expand All @@ -165,15 +162,15 @@ 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));
}
RunOnMainThread(true, new Runnable() {
RunOnMainThread(false, new Runnable() {
public void run() {
AddCASublayer0(rootCALayer, subCALayer);
}
});
}

/**
* Fix root and sub CALayer position to 0/0 and size on the main-thread w/o blocking.
* Fix root and sub CALayer position to 0/0 and size
* <p>
* If the sub CALayer implements the Objective-C NativeWindow protocol NWDedicatedSize (e.g. JOGL's MyNSOpenGLLayer),
* the dedicated size is passed to the layer, which propagates it appropriately.
Expand All @@ -192,38 +189,28 @@ public static void FixCALayerLayout(final long rootCALayer, final long subCALaye
if( 0==rootCALayer && 0==subCALayer ) {
return;
}
RunOnMainThread(false, new Runnable() {
public void run() {
FixCALayerLayout0(rootCALayer, subCALayer, width, height);
}
});
FixCALayerLayout0(rootCALayer, subCALayer, width, height);
}

/**
* Detach a sub CALayer from the root CALayer on the main-thread w/ blocking.
* Detach a sub CALayer from the root CALayer
*/
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));
}
RunOnMainThread(true, new Runnable() {
public void run() {
RemoveCASublayer0(rootCALayer, subCALayer);
} } );
RemoveCASublayer0(rootCALayer, subCALayer);
}

/**
* Destroy a CALayer on the main-thread w/ blocking.
* Destroy a CALayer
* @see #CreateCALayer(int, int, int, int)
*/
public static void DestroyCALayer(final long caLayer) {
if(0==caLayer) {
throw new IllegalArgumentException("caLayer 0x"+Long.toHexString(caLayer));
}
RunOnMainThread(true, new Runnable() {
public void run() {
DestroyCALayer0(caLayer);
} } );
DestroyCALayer0(caLayer);
}

/**
Expand Down
Loading

0 comments on commit 896e8b0

Please sign in to comment.