- Fix: Native X11 Display deadlock (Linux x86_64 32bit on 64bit)

    It turns out that under some circumstances,
    e.g. >3 threads within initialization time,
    the static X11Display usage result in a 
    native deadlock within glXQueryServerString().
    The call never returned. May be 
    This happend even with NativeWindowFactory.getDefaultFactory().getToolkitLock().lock();

    Removed X11Util.getStaticDefaultDisplay()

    This allows us to remove the ToolkitLock around
    these segments, due to a thread local X11Display utilization.




git-svn-id: file:///usr/local/projects/SUN/JOGL/git-svn/svn-server-sync/jogl/trunk@1977 232f8b59-042b-4e1e-8c03-345bb8c30851

5 files changed, 87 insertions, 174 deletions

diff --git a/src/jogl/classes/com/sun/opengl/impl/x11/glx/GLXUtil.java b/src/jogl/classes/com/sun/opengl/impl/x11/glx/GLXUtil.java
index 8ab99ac0a..b3d25b52c 100644
--- a/src/jogl/classes/com/sun/opengl/impl/x11/glx/GLXUtil.java
+++ b/src/jogl/classes/com/sun/opengl/impl/x11/glx/GLXUtil.java
@@ -57,36 +57,30 @@ public class GLXUtil {
         if (!isInit) {
             synchronized (GLXUtil.class) {
                 if (!isInit) {
-                    NativeWindowFactory.getDefaultFactory().getToolkitLock().lock();
-                    try {
-                        long staticDisplay = X11Util.getStaticDefaultDisplay();
-                        if(staticDisplay!=0) {
-                            if (DEBUG) {
-                                long display = staticDisplay;
-                                int screen = X11Lib.DefaultScreen(display);
-                                System.err.println("!!! GLX server vendor : " +
-                                                   GLX.glXQueryServerString(display, screen, GLX.GLX_VENDOR));
-                                System.err.println("!!! GLX server version: " +
-                                                   GLX.glXQueryServerString(display, screen, GLX.GLX_VERSION));
-                                System.err.println("!!! GLX client vendor : " +
-                                                   GLX.glXGetClientString(display, GLX.GLX_VENDOR));
-                                System.err.println("!!! GLX client version: " +
-                                                   GLX.glXGetClientString(display, GLX.GLX_VERSION));
-                            }
-                            String vendor = GLX.glXGetClientString(staticDisplay, GLX.GLX_VENDOR);
-                            if (vendor != null && vendor.startsWith("ATI")) {
-                                isVendorATI = true;
-                            }
-                            String exts = GLX.glXGetClientString(staticDisplay, GLX.GLX_EXTENSIONS);
-                            if (exts != null) {
-                                multisampleAvailable = (exts.indexOf("GLX_ARB_multisample") >= 0);
-                            }
-                            isInit=true;
-                        } else {
-                            throw new GLException("Unable to open default display, needed for visual selection and offscreen surface handling");
+                    long locDisplay = X11Util.getThreadLocalDefaultDisplay();
+                    if(locDisplay!=0) {
+                        if (DEBUG) {
+                            int screen = X11Lib.DefaultScreen(locDisplay);
+                            System.err.println("!!! GLX server vendor : " +
+                                               GLX.glXQueryServerString(locDisplay, screen, GLX.GLX_VENDOR));
+                            System.err.println("!!! GLX server version: " +
+                                               GLX.glXQueryServerString(locDisplay, screen, GLX.GLX_VERSION));
+                            System.err.println("!!! GLX client vendor : " +
+                                               GLX.glXGetClientString(locDisplay, GLX.GLX_VENDOR));
+                            System.err.println("!!! GLX client version: " +
+                                               GLX.glXGetClientString(locDisplay, GLX.GLX_VERSION));
                         }
-                    } finally {
-                        NativeWindowFactory.getDefaultFactory().getToolkitLock().unlock();
+                        String vendor = GLX.glXGetClientString(locDisplay, GLX.GLX_VENDOR);
+                        if (vendor != null && vendor.startsWith("ATI")) {
+                            isVendorATI = true;
+                        }
+                        String exts = GLX.glXGetClientString(locDisplay, GLX.GLX_EXTENSIONS);
+                        if (exts != null) {
+                            multisampleAvailable = (exts.indexOf("GLX_ARB_multisample") >= 0);
+                        }
+                        isInit=true;
+                    } else {
+                        throw new GLException("Unable to open default display, needed for visual selection and offscreen surface handling");
                     }
                 }
             }
diff --git a/src/jogl/classes/com/sun/opengl/impl/x11/glx/X11GLXDrawableFactory.java b/src/jogl/classes/com/sun/opengl/impl/x11/glx/X11GLXDrawableFactory.java
index 7491c6d7c..08e8b2b2f 100644
--- a/src/jogl/classes/com/sun/opengl/impl/x11/glx/X11GLXDrawableFactory.java
+++ b/src/jogl/classes/com/sun/opengl/impl/x11/glx/X11GLXDrawableFactory.java
@@ -86,42 +86,32 @@ public class X11GLXDrawableFactory extends GLDrawableFactoryImpl implements Dyna
   private boolean canCreateGLPbuffer = false;
   public boolean canCreateGLPbuffer() {
     if (!pbufferSupportInitialized) {
-      Runnable r = new Runnable() {
-        public void run() {
-            lockToolkit();
-            try {
-              long display = X11Util.getStaticDefaultDisplay();
-              int[] major = new int[1];
-              int[] minor = new int[1];
-              int screen = 0; // FIXME: provide way to specify this?
-
-              if (!GLX.glXQueryVersion(display, major, 0, minor, 0)) {
-                throw new GLException("glXQueryVersion failed");
-              }
-              if (DEBUG) {
-                System.err.println("!!! GLX version: major " + major[0] +
-                                   ", minor " + minor[0]);
-              }
+        long display = X11Util.getThreadLocalDefaultDisplay();
+        int[] major = new int[1];
+        int[] minor = new int[1];
+        int screen = 0; // FIXME: provide way to specify this?
 
-              // Work around bugs in ATI's Linux drivers where they report they
-              // only implement GLX version 1.2 on the server side
-              if (major[0] == 1 && minor[0] == 2) {
-                String str = GLX.glXGetClientString(display, GLX.GLX_VERSION);
-                if (str != null && str.startsWith("1.") &&
-                   (str.charAt(2) >= '3')) {
-                  canCreateGLPbuffer = true;
-                }
-              } else {
-                canCreateGLPbuffer = ((major[0] > 1) || (minor[0] > 2));
-              }
+        if (!GLX.glXQueryVersion(display, major, 0, minor, 0)) {
+          throw new GLException("glXQueryVersion failed");
+        }
+        if (DEBUG) {
+          System.err.println("!!! GLX version: major " + major[0] +
+                             ", minor " + minor[0]);
+        }
 
-              pbufferSupportInitialized = true;        
-            } finally {
-              unlockToolkit();
-            }
+        // Work around bugs in ATI's Linux drivers where they report they
+        // only implement GLX version 1.2 on the server side
+        if (major[0] == 1 && minor[0] == 2) {
+          String str = GLX.glXGetClientString(display, GLX.GLX_VERSION);
+          if (str != null && str.startsWith("1.") &&
+             (str.charAt(2) >= '3')) {
+            canCreateGLPbuffer = true;
+          }
+        } else {
+          canCreateGLPbuffer = ((major[0] > 1) || (minor[0] > 2));
         }
-      };
-      maybeDoSingleThreadedWorkaround(r);
+
+        pbufferSupportInitialized = true;        
     }
     return canCreateGLPbuffer;
   }
@@ -208,17 +198,12 @@ public class X11GLXDrawableFactory extends GLDrawableFactoryImpl implements Dyna
     }
 
     int[] size = new int[1];
-    lockToolkit();
-    try {
-        long display = X11Util.getStaticDefaultDisplay();
-        boolean res = X11Lib.XF86VidModeGetGammaRampSize(display,
-                                                      X11Lib.DefaultScreen(display),
-                                                      size, 0);
-        if (!res) {
-          return 0;
-        }
-    } finally {
-        unlockToolkit();
+    long display = X11Util.getThreadLocalDefaultDisplay();
+    boolean res = X11Lib.XF86VidModeGetGammaRampSize(display,
+                                                  X11Lib.DefaultScreen(display),
+                                                  size, 0);
+    if (!res) {
+      return 0;
     }
     gotGammaRampLength = true;
     gammaRampLength = size[0];
@@ -232,19 +217,14 @@ public class X11GLXDrawableFactory extends GLDrawableFactoryImpl implements Dyna
       rampData[i] = (short) (ramp[i] * 65535);
     }
 
-    lockToolkit();
-    try {
-        long display = X11Util.getStaticDefaultDisplay();
-        boolean res = X11Lib.XF86VidModeSetGammaRamp(display,
-                                                  X11Lib.DefaultScreen(display),
-                                                  rampData.length,
-                                                  rampData, 0,
-                                                  rampData, 0,
-                                                  rampData, 0);
-        return res;
-    } finally {
-        unlockToolkit();
-    }
+    long display = X11Util.getThreadLocalDefaultDisplay();
+    boolean res = X11Lib.XF86VidModeSetGammaRamp(display,
+                                              X11Lib.DefaultScreen(display),
+                                              rampData.length,
+                                              rampData, 0,
+                                              rampData, 0,
+                                              rampData, 0);
+    return res;
   }
 
   protected Buffer getGammaRamp() {
@@ -259,20 +239,15 @@ public class X11GLXDrawableFactory extends GLDrawableFactoryImpl implements Dyna
     rampData.position(2 * size);
     rampData.limit(3 * size);
     ShortBuffer blueRampData = rampData.slice();
-    lockToolkit();
-    try {
-        long display = X11Util.getStaticDefaultDisplay();
-        boolean res = X11Lib.XF86VidModeGetGammaRamp(display,
-                                                  X11Lib.DefaultScreen(display),
-                                                  size,
-                                                  redRampData,
-                                                  greenRampData,
-                                                  blueRampData);
-        if (!res) {
-          return null;
-        }
-    } finally {
-        unlockToolkit();
+    long display = X11Util.getThreadLocalDefaultDisplay();
+    boolean res = X11Lib.XF86VidModeGetGammaRamp(display,
+                                              X11Lib.DefaultScreen(display),
+                                              size,
+                                              redRampData,
+                                              greenRampData,
+                                              blueRampData);
+    if (!res) {
+      return null;
     }
     return rampData;
   }
@@ -295,17 +270,12 @@ public class X11GLXDrawableFactory extends GLDrawableFactoryImpl implements Dyna
     rampData.position(2 * size);
     rampData.limit(3 * size);
     ShortBuffer blueRampData = rampData.slice();
-    lockToolkit();
-    try {
-        long display = X11Util.getStaticDefaultDisplay();
-        X11Lib.XF86VidModeSetGammaRamp(display,
-                                    X11Lib.DefaultScreen(display),
-                                    size,
-                                    redRampData,
-                                    greenRampData,
-                                    blueRampData);
-    } finally {
-        unlockToolkit();
-    }
+    long display = X11Util.getThreadLocalDefaultDisplay();
+    X11Lib.XF86VidModeSetGammaRamp(display,
+                                X11Lib.DefaultScreen(display),
+                                size,
+                                redRampData,
+                                greenRampData,
+                                blueRampData);
   }
 }
diff --git a/src/nativewindow/classes/com/sun/nativewindow/impl/x11/X11Util.java b/src/nativewindow/classes/com/sun/nativewindow/impl/x11/X11Util.java
index 38d0fb73d..acd55a823 100644
--- a/src/nativewindow/classes/com/sun/nativewindow/impl/x11/X11Util.java
+++ b/src/nativewindow/classes/com/sun/nativewindow/impl/x11/X11Util.java
@@ -39,28 +39,11 @@ import javax.media.nativewindow.*;
 import com.sun.nativewindow.impl.*;
 
 /**
- * Contains a thread safe X11 utility to retrieve tread local display connection,<br>
+ * Contains a thread safe X11 utility to retrieve thread local display connection,<br>
  * as well as the static global discplay connection.<br>
  *
- * The TLS variant is thread safe per se, but has the memory leak risk on applications
- * heavily utilizing runnables on a new thread.<br>
- * 
- * The static variant is more nice to resources, but involves the default toolkit
- * locking mechanism, which invocation you have to provide as shown below.<br>
- * <PRE>
-   NativeWindowFactory.getDefaultFactory().getToolkitLock().lock();
-   try {
-     long displayHandle = X11Util.getStaticDefaultDisplay();
-     ...
-   } finally {
-     NativeWindowFactory.getDefaultFactory().getToolkitLock().unlock();
-   }
- * </PRE><br>
- *
- * We will use the TLS variant, where a long term display connection is being used,
- * ie in JOGL's <code>X11AWTGLXGraphicsConfigurationFactory</code>,
- * on all other situations where we just query some X11 attributes,
- * the locked static variant shall be used instead.<br>
+ * The TLS variant is thread safe per se, but be aware of the memory leak risk 
+ * where an application heavily utilizing this class on temporary new threads.<br>
  */
 public class X11Util {
     private static final boolean DEBUG = Debug.debug("X11Util");
@@ -73,43 +56,13 @@ public class X11Util {
 
     private static ThreadLocal currentDisplayAssociation = new ThreadLocal();
 
-    private static volatile long    staticDefaultDisplay=0; 
-    private static boolean staticDefaultDisplayXineramaEnable=false; 
-
-    private static long fetchStaticDefaultDisplay() {
-        if(0==staticDefaultDisplay) {
-            synchronized (X11Util.class) {
-                if(0==staticDefaultDisplay) {
-                    NativeWindowFactory.getDefaultFactory().getToolkitLock().lock();
-                    try {
-                        staticDefaultDisplay = X11Lib.XOpenDisplay(null);
-                        if(0==staticDefaultDisplay) {
-                            throw new NativeWindowException("Unable to create a static default display connection");
-                        }
-                        staticDefaultDisplayXineramaEnable = X11Lib.XineramaEnabled(staticDefaultDisplay);
-                    } finally {
-                        NativeWindowFactory.getDefaultFactory().getToolkitLock().unlock();
-                    }
-                }
-            }
-        }
-        return staticDefaultDisplay;
-    }
-
-    /** Returns the global static default display connection, read the toolkit lock/unlock
-      * requirements {@link X11Util above} for synchronization. */
-    public static long getStaticDefaultDisplay() {
-        return fetchStaticDefaultDisplay();
-    }
-
     /** Returns the global static default display connection, read the toolkit lock/unlock
       * requirements {@link X11Util above} for synchronization. */
-    public static boolean isXineramaEnabledOnStaticDefaultDisplay() {
-        fetchStaticDefaultDisplay();
-        return staticDefaultDisplayXineramaEnable;
+    public static boolean isXineramaEnabledOnThreadLocalDefaultDisplay() {
+        long dpy = getThreadLocalDefaultDisplay();
+        return X11Lib.XineramaEnabled(dpy);
     }
 
-
     /** Returns this thread current default display. */
     public static long getThreadLocalDefaultDisplay() {
         Long dpyL = (Long) currentDisplayAssociation.get();
diff --git a/src/nativewindow/classes/javax/media/nativewindow/ToolkitLock.java b/src/nativewindow/classes/javax/media/nativewindow/ToolkitLock.java
index 27e59d8c8..6f83896fa 100644
--- a/src/nativewindow/classes/javax/media/nativewindow/ToolkitLock.java
+++ b/src/nativewindow/classes/javax/media/nativewindow/ToolkitLock.java
@@ -47,8 +47,9 @@ package javax.media.nativewindow;
     <PRE>
     NativeWindowFactory.getDefaultFactory().getToolkitLock().lock();
     try {
-      long displayHandle = X11Util.getStaticDefaultDisplay();
-      ...
+      long displayHandle = X11Util.getThreadLocalDefaultDisplay();
+      ... some code dealing with shared resources 
+      ... ie the window surface
     } finally {
       NativeWindowFactory.getDefaultFactory().getToolkitLock().unlock();
     }
diff --git a/src/nativewindow/classes/javax/media/nativewindow/x11/X11GraphicsScreen.java b/src/nativewindow/classes/javax/media/nativewindow/x11/X11GraphicsScreen.java
index f434050f2..03121dfaa 100644
--- a/src/nativewindow/classes/javax/media/nativewindow/x11/X11GraphicsScreen.java
+++ b/src/nativewindow/classes/javax/media/nativewindow/x11/X11GraphicsScreen.java
@@ -74,13 +74,8 @@ public class X11GraphicsScreen extends DefaultGraphicsScreen implements Cloneabl
     }
 
     private static int fetchScreen(int screen) {
-        NativeWindowFactory.getDefaultFactory().getToolkitLock().lock();
-        try {
-            if(!com.sun.nativewindow.impl.x11.X11Util.isXineramaEnabledOnStaticDefaultDisplay()) {
-                return screen;
-            }
-        } finally {
-            NativeWindowFactory.getDefaultFactory().getToolkitLock().unlock();
+        if(!com.sun.nativewindow.impl.x11.X11Util.isXineramaEnabledOnThreadLocalDefaultDisplay()) {
+            return screen;
         }
         return 0;
     }