From d644e9321dceeecdd94a1797e25e6e356d957c23 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Fri, 18 Nov 2011 07:14:41 +0100 Subject: Fix OS X JAWT SIGSEGV @ NEWT Window creation ; Reloc AttachJAWTSurfaceLayer() OSXUtil -> MacOSXJAWTWindow Threading/sync issue when creating a NEWT window, which issues a Java callback from native code (positionChanged()). The latter requires a location validation w/ getLocationOnScreen() involved. Hence getLocationOnScreen() shall not lock the JAWT native resources, since it may not be ready yet (-> threading/sync issue). --- .../jogamp/nativewindow/jawt/JAWTWindow.java | 85 ++++++++++++++++------ .../nativewindow/jawt/macosx/MacOSXJAWTWindow.java | 42 +++++++---- .../jawt/windows/WindowsJAWTWindow.java | 2 +- .../nativewindow/jawt/x11/X11JAWTWindow.java | 2 +- .../jogamp/nativewindow/macosx/OSXUtil.java | 10 --- src/nativewindow/native/macosx/OSXmisc.m | 49 +++++++------ 6 files changed, 119 insertions(+), 71 deletions(-) diff --git a/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTWindow.java b/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTWindow.java index 4d5fac891..0f97b0b9f 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTWindow.java +++ b/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTWindow.java @@ -378,44 +378,85 @@ public abstract class JAWTWindow implements NativeWindow, OffscreenLayerSurface public final int getY() { return component.getY(); } - + + /** + * {@inheritDoc} + * + *

+ * This JAWT default implementation is currently still using + * a blocking implementation. It first attempts to retrieve the location + * via a native implementation. If this fails, it tries the blocking AWT implementation. + * If the latter fails due to an external AWT tree-lock, the non block + * implementation {@link #getLocationOnScreenNonBlocking(Point, Component)} is being used. + * The latter simply traverse up to the AWT component tree and sums the rel. position. + * We have to determine whether the latter is good enough for all cases, + * currently only OS X utilizes the non blocking method per default. + *

+ */ public Point getLocationOnScreen(Point storage) { + Point los = getLocationOnScreenNative(storage); + if(null == los) { + if(!Thread.holdsLock(component.getTreeLock())) { + // avoid deadlock .. + if(DEBUG) { + System.err.println("Warning: JAWT Lock hold, but not the AWT tree lock: "+this); + Thread.dumpStack(); + } + return getLocationOnScreenNonBlocking(storage, component); + } + java.awt.Point awtLOS = component.getLocationOnScreen(); + if(null!=storage) { + los = storage.translate(awtLOS.x, awtLOS.y); + } else { + los = new Point(awtLOS.x, awtLOS.y); + } + } + return los; + } + + protected Point getLocationOnScreenNative(Point storage) { int lockRes = lockSurface(); if(LOCK_SURFACE_NOT_READY == lockRes) { - // FIXME: Shall we deal with already locked or unrealized surfaces ? - System.err.println("Warning: JAWT Lock couldn't be acquired!"); - Thread.dumpStack(); + if(DEBUG) { + System.err.println("Warning: JAWT Lock couldn't be acquired: "+this); + Thread.dumpStack(); + } return null; } try { - Point d = getLocationOnScreenImpl(0, 0); + Point d = getLocationOnScreenNativeImpl(0, 0); if(null!=d) { if(null!=storage) { storage.translate(d.getX(),d.getY()); return storage; } - return d; - } - // fall through intended .. - if(!Thread.holdsLock(component.getTreeLock())) { - // FIXME: Verify if this check is still required! - System.err.println("Warning: JAWT Lock hold, but not the AWT tree lock!"); - Thread.dumpStack(); - return null; // avoid deadlock .. - } - java.awt.Point awtLOS = component.getLocationOnScreen(); - int dx = (int) ( awtLOS.getX() + .5 ) ; - int dy = (int) ( awtLOS.getY() + .5 ) ; - if(null!=storage) { - return storage.translate(dx, dy); } - return new Point(dx, dy); + return d; } finally { unlockSurface(); - } + } } - protected abstract Point getLocationOnScreenImpl(int x, int y); + protected abstract Point getLocationOnScreenNativeImpl(int x, int y); + protected static Point getLocationOnScreenNonBlocking(Point storage, Component comp) { + int x = 0; + int y = 0; + while(null != comp) { + x += comp.getX(); + y += comp.getY(); + comp = comp.getParent(); + } + if(null!=storage) { + storage.translate(x, y); + return storage; + } + return new Point(x, y); + } + + public boolean hasFocus() { + return component.hasFocus(); + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); diff --git a/src/nativewindow/classes/jogamp/nativewindow/jawt/macosx/MacOSXJAWTWindow.java b/src/nativewindow/classes/jogamp/nativewindow/jawt/macosx/MacOSXJAWTWindow.java index de7feb3b8..70fc1d62f 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/jawt/macosx/MacOSXJAWTWindow.java +++ b/src/nativewindow/classes/jogamp/nativewindow/jawt/macosx/MacOSXJAWTWindow.java @@ -41,12 +41,12 @@ package jogamp.nativewindow.jawt.macosx; import java.awt.Component; +import java.nio.Buffer; import java.security.AccessController; import java.security.PrivilegedAction; import javax.media.nativewindow.AbstractGraphicsConfiguration; import javax.media.nativewindow.Capabilities; -import javax.media.nativewindow.NativeSurface; import javax.media.nativewindow.NativeWindow; import javax.media.nativewindow.NativeWindowException; import javax.media.nativewindow.SurfaceChangeable; @@ -208,7 +208,7 @@ public class MacOSXJAWTWindow extends JAWTWindow implements SurfaceChangeable { unlockSurfaceImpl(); throw new NativeWindowException("Could not create root CALayer: "+this); } - if(!OSXUtil.AttachJAWTSurfaceLayer(dsi, rootSurfaceLayerHandle)) { + if(!AttachJAWTSurfaceLayer(dsi, rootSurfaceLayerHandle)) { OSXUtil.DestroyCALayer(rootSurfaceLayerHandle); rootSurfaceLayerHandle = 0; OSXUtil.DestroyNSWindow(drawable); @@ -249,18 +249,34 @@ public class MacOSXJAWTWindow extends JAWTWindow implements SurfaceChangeable { // Thread.dumpStack(); } - protected Point getLocationOnScreenImpl(final int x0, final int y0) { - int x = x0; - int y = y0; - Component c = component; - while(null != c) { - x += c.getX(); - y += c.getY(); - c = c.getParent(); - } - return new Point(x, y); - } + /** + * {@inheritDoc} + *

+ * On OS X locking the surface at this point (ie after creation and for location validation) + * is 'tricky' since the JVM traverses through many threads and crashes at: + * lockSurfaceImpl() { + * .. + * ds = getJAWT().GetDrawingSurface(component); + * due to a SIGSEGV. + * + * Hence we have some threading / sync issues with the native JAWT implementation. + *

+ */ + @Override + public Point getLocationOnScreen(Point storage) { + return getLocationOnScreenNonBlocking(storage, component); + } + protected Point getLocationOnScreenNativeImpl(final int x0, final int y0) { return null; } + private static boolean AttachJAWTSurfaceLayer(JAWT_DrawingSurfaceInfo dsi, long caLayer) { + if(0==caLayer) { + throw new IllegalArgumentException("caLayer 0x"+Long.toHexString(caLayer)); + } + return AttachJAWTSurfaceLayer0(dsi.getBuffer(), caLayer); + } + + private static native boolean AttachJAWTSurfaceLayer0(Buffer jawtDrawingSurfaceInfoBuffer, long caLayer); + // Variables for lockSurface/unlockSurface private JAWT_DrawingSurface ds; private boolean dsLocked; diff --git a/src/nativewindow/classes/jogamp/nativewindow/jawt/windows/WindowsJAWTWindow.java b/src/nativewindow/classes/jogamp/nativewindow/jawt/windows/WindowsJAWTWindow.java index 581e2337c..adb9353ce 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/jawt/windows/WindowsJAWTWindow.java +++ b/src/nativewindow/classes/jogamp/nativewindow/jawt/windows/WindowsJAWTWindow.java @@ -139,7 +139,7 @@ public class WindowsJAWTWindow extends JAWTWindow { return windowHandle; } - protected Point getLocationOnScreenImpl(int x, int y) { + protected Point getLocationOnScreenNativeImpl(int x, int y) { return GDI.GetRelativeLocation( getWindowHandle(), 0 /*root win*/, x, y); } diff --git a/src/nativewindow/classes/jogamp/nativewindow/jawt/x11/X11JAWTWindow.java b/src/nativewindow/classes/jogamp/nativewindow/jawt/x11/X11JAWTWindow.java index e469fe337..236d380d8 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/jawt/x11/X11JAWTWindow.java +++ b/src/nativewindow/classes/jogamp/nativewindow/jawt/x11/X11JAWTWindow.java @@ -156,7 +156,7 @@ public class X11JAWTWindow extends JAWTWindow { x11dsi = null; } - protected Point getLocationOnScreenImpl(int x, int y) { + protected Point getLocationOnScreenNativeImpl(int x, int y) { return X11Util.GetRelativeLocation( getDisplayHandle(), getScreenIndex(), getWindowHandle(), 0 /*root win*/, x, y); } diff --git a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java index 4ed46596a..a93ab2ac1 100644 --- a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java +++ b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java @@ -70,15 +70,6 @@ public class OSXUtil { throw new IllegalArgumentException("caLayer 0x"+Long.toHexString(caLayer)); } DestroyCALayer0(caLayer); - } - public static boolean AttachJAWTSurfaceLayer(Object jawtDrawingSurfaceInfo, long caLayer) { - final jogamp.nativewindow.jawt.JAWT_DrawingSurfaceInfo dsi = - (jogamp.nativewindow.jawt.JAWT_DrawingSurfaceInfo) jawtDrawingSurfaceInfo; - - if(0==caLayer) { - throw new IllegalArgumentException("caLayer 0x"+Long.toHexString(caLayer)); - } - return AttachJAWTSurfaceLayer0(dsi.getBuffer(), caLayer); } public static void RunOnMainThread(boolean waitUntilDone, Runnable runnable) { @@ -103,7 +94,6 @@ public class OSXUtil { private static native void AddCASublayer0(long rootCALayer, long subCALayer); private static native void RemoveCASublayer0(long rootCALayer, long subCALayer); private static native void DestroyCALayer0(long caLayer); - private static native boolean AttachJAWTSurfaceLayer0(Buffer jawtDrawingSurfaceInfoBuffer, long caLayer); private static native void RunOnMainThread0(boolean waitUntilDone, Runnable runnable); private static native boolean IsMainThread0(); } diff --git a/src/nativewindow/native/macosx/OSXmisc.m b/src/nativewindow/native/macosx/OSXmisc.m index 45ff1bd4c..d7a2feff3 100644 --- a/src/nativewindow/native/macosx/OSXmisc.m +++ b/src/nativewindow/native/macosx/OSXmisc.m @@ -35,6 +35,7 @@ #include "NativewindowCommon.h" #include "jogamp_nativewindow_macosx_OSXUtil.h" +#include "jogamp_nativewindow_jawt_macosx_MacOSXJAWTWindow.h" #include #import @@ -323,30 +324,6 @@ JNIEXPORT void JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_DestroyCALayer0 JNF_COCOA_EXIT(env); } -/* - * Class: Java_jogamp_nativewindow_macosx_OSXUtil - * Method: attachJAWTSurfaceLayer - * Signature: (JJ)Z - */ -JNIEXPORT jboolean JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_AttachJAWTSurfaceLayer0 - (JNIEnv *env, jclass unused, jobject jawtDrawingSurfaceInfoBuffer, jlong caLayer) -{ - JNF_COCOA_ENTER(env); - JAWT_DrawingSurfaceInfo* dsi = (JAWT_DrawingSurfaceInfo*) (*env)->GetDirectBufferAddress(env, jawtDrawingSurfaceInfoBuffer); - if (NULL == dsi) { - NativewindowCommon_throwNewRuntimeException(env, "Argument \"jawtDrawingSurfaceInfoBuffer\" was not a direct buffer"); - return JNI_FALSE; - } - CALayer* layer = (CALayer*) (intptr_t) caLayer; - [JNFRunLoop performOnMainThreadWaiting:YES withBlock:^(){ - id surfaceLayers = (id )dsi->platformInfo; - DBG_PRINT("CALayer::attachJAWTSurfaceLayer: %p -> %p\n", surfaceLayers.layer, layer); - surfaceLayers.layer = [layer autorelease]; - }]; - JNF_COCOA_EXIT(env); - return JNI_TRUE; -} - @interface MainRunnable : NSObject { @@ -432,3 +409,27 @@ JNIEXPORT jboolean JNICALL Java_jogamp_nativewindow_macosx_OSXUtil_IsMainThread0 return ( [NSThread isMainThread] == YES ) ? JNI_TRUE : JNI_FALSE ; } +/* + * Class: Java_jogamp_nativewindow_jawt_macosx_MacOSXJAWTWindow + * Method: AttachJAWTSurfaceLayer + * Signature: (JJ)Z + */ +JNIEXPORT jboolean JNICALL Java_jogamp_nativewindow_jawt_macosx_MacOSXJAWTWindow_AttachJAWTSurfaceLayer0 + (JNIEnv *env, jclass unused, jobject jawtDrawingSurfaceInfoBuffer, jlong caLayer) +{ + JNF_COCOA_ENTER(env); + JAWT_DrawingSurfaceInfo* dsi = (JAWT_DrawingSurfaceInfo*) (*env)->GetDirectBufferAddress(env, jawtDrawingSurfaceInfoBuffer); + if (NULL == dsi) { + NativewindowCommon_throwNewRuntimeException(env, "Argument \"jawtDrawingSurfaceInfoBuffer\" was not a direct buffer"); + return JNI_FALSE; + } + CALayer* layer = (CALayer*) (intptr_t) caLayer; + [JNFRunLoop performOnMainThreadWaiting:YES withBlock:^(){ + id surfaceLayers = (id )dsi->platformInfo; + DBG_PRINT("CALayer::attachJAWTSurfaceLayer: %p -> %p\n", surfaceLayers.layer, layer); + surfaceLayers.layer = [layer autorelease]; + }]; + JNF_COCOA_EXIT(env); + return JNI_TRUE; +} + -- cgit v1.2.3