Fix Bug 621, JVM Crashes During X11 Shutdown while closing all unclosed display connections.x11_xinitthreads

This is a regression of JOGL commit f2cfb6119a3663715ed2d572643949b3bef58662 to fix Bug 610.

Commit f2cfb6119a3663715ed2d572643949b3bef58662 closed all open display connections at
JVM shutdown in order of their creation.
This is required IF the driver is the proprietary ATI on X11.
X11Util.setMarkAllDisplaysUnclosable(true) is called in this case.

However, this behavior causes a SIGSEGV for other GPU driver, i.e. Nvidia :)

This fix only closes the pending display connections in their respective order
IF X11Util.getMarkAllDisplaysUnclosable() == true.
Hence the 'new' cleanup as introduced in the offending commit
is only performed on the erroneous ATI driver.

Manually tested w/ TestGearsES2AWT, see it's commandline options for details.

2 files changed, 45 insertions, 23 deletions

diff --git a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
index 93b7f3487..2ea75c7fb 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
@@ -70,7 +70,12 @@ public class X11Util {
      * See also native test: jogl/test-native/displayMultiple02.c
      * </p>
      * <p>
-     * Workaround is to not close them at all if driver vendor is ATI.
+     * Workaround is to not close them at all if driver vendor is ATI
+     * during operation.
+     * </p>
+     * <p>
+     * With ATI X11 drivers all connections must be closed at JVM shutdown,
+     * otherwise a SIGSEGV (after JVM safepoint) will be caused. 
      * </p>
      */
     public static final boolean ATI_HAS_XCLOSEDISPLAY_BUG = !Debug.isPropertyDefined("nativewindow.debug.X11Util.ATI_HAS_NO_XCLOSEDISPLAY_BUG", true);
@@ -141,6 +146,14 @@ public class X11Util {
         }
     }
     
+    // not exactly thread safe, but good enough for our purpose,
+    // which is to tag a NamedDisplay uncloseable after creation.
+    private static Object globalLock = new Object(); 
+    private static LongObjectHashMap openDisplayMap = new LongObjectHashMap(); // handle -> name
+    private static List<NamedDisplay> openDisplayList = new ArrayList<NamedDisplay>();     // open, no close attempt
+    private static List<NamedDisplay> reusableDisplayList = new ArrayList<NamedDisplay>(); // close attempt, marked uncloseable, for reuse
+    private static List<NamedDisplay> pendingDisplayList = new ArrayList<NamedDisplay>();  // all open (close attempt or reusable) in creation order
+    
     /** 
      * Cleanup resources.
      * <p>
@@ -156,7 +169,7 @@ public class X11Util {
                         System.err.println("X11Util.Display: Shutdown (JVM shutdown: "+isJVMShuttingDown+
                                            ", open (no close attempt): "+openDisplayMap.size()+"/"+openDisplayList.size()+
                                            ", reusable (open, marked uncloseable): "+reusableDisplayList.size()+
-                                           ", pending (post closing): "+pendingDisplayList.size()+
+                                           ", pending (open in creation order): "+pendingDisplayList.size()+
                                            ")");
                         if(DEBUG) {
                             Thread.dumpStack();
@@ -227,14 +240,6 @@ public class X11Util {
     
     private X11Util() {}
 
-    // not exactly thread safe, but good enough for our purpose,
-    // which is to tag a NamedDisplay uncloseable after creation.
-    private static Object globalLock = new Object(); 
-    private static LongObjectHashMap openDisplayMap = new LongObjectHashMap(); // handle -> name
-    private static List<NamedDisplay> openDisplayList = new ArrayList<NamedDisplay>();
-    private static List<NamedDisplay> reusableDisplayList = new ArrayList<NamedDisplay>();
-    private static List<NamedDisplay> pendingDisplayList = new ArrayList<NamedDisplay>();
-
     public static class NamedDisplay {
         final String name;
         final long   handle;
@@ -296,23 +301,21 @@ public class X11Util {
     }
 
     /**
-     * Closing pending Display connections in reverse order.
+     * Closing pending Display connections in original creation order, if {@link #getMarkAllDisplaysUnclosable()} is true.
      *
      * @return number of closed Display connections
      */
-    public static int closePendingDisplayConnections() {
+    private static int closePendingDisplayConnections() {
         int num=0;
         synchronized(globalLock) {
-            if(DEBUG) {
-                System.err.println("X11Util: Closing Pending X11 Display Connections in order of their creation: "+pendingDisplayList.size());
-            }
-            for(int i=0; i<pendingDisplayList.size(); i++) {
-                NamedDisplay ndpy = (NamedDisplay) pendingDisplayList.get(i);
-                if(DEBUG) {
-                    System.err.println("X11Util.closePendingDisplayConnections(): Closing ["+i+"]: "+ndpy);
+            if( getMarkAllDisplaysUnclosable() ) {
+                for(int i=0; i<pendingDisplayList.size(); i++) {
+                    final NamedDisplay ndpy = (NamedDisplay) pendingDisplayList.get(i);
+                    final boolean closeAttempted = !openDisplayMap.containsKey(ndpy.getHandle());
+                    System.err.println("X11Util.closePendingDisplayConnections(): Closing ["+i+"]: "+ndpy+" - closeAttempted "+closeAttempted);
+                    XCloseDisplay(ndpy.getHandle());
+                    num++;
                 }
-                XCloseDisplay(ndpy.getHandle());
-                num++;
             }
         }
         return num;
diff --git a/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2AWT.java b/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2AWT.java
index 732036eb0..c97054d18 100644
--- a/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2AWT.java
+++ b/src/test/com/jogamp/opengl/test/junit/jogl/demos/es2/awt/TestGearsES2AWT.java
@@ -65,6 +65,9 @@ public class TestGearsES2AWT extends UITestCase {
     static boolean shallUseOffscreenPBufferLayer = false;
     static boolean useMSAA = false;
     static boolean addComp = true;
+    static boolean shutdownRemoveGLCanvas = true;
+    static boolean shutdownDisposeFrame = true;
+    static boolean shutdownSystemExit = false;
     static int swapInterval = 1;
 
     @BeforeClass
@@ -128,8 +131,15 @@ public class TestGearsES2AWT extends UITestCase {
         Assert.assertEquals(false, frame.isVisible());
         javax.swing.SwingUtilities.invokeAndWait(new Runnable() {
             public void run() {
-                frame.remove(glCanvas);
-                frame.dispose();
+                if(shutdownRemoveGLCanvas) {
+                    frame.remove(glCanvas);
+                }
+                if(shutdownDisposeFrame) {
+                    frame.dispose();
+                }
+                if(shutdownSystemExit) {
+                    System.exit(0);
+                }
             }});
     }
 
@@ -175,6 +185,15 @@ public class TestGearsES2AWT extends UITestCase {
                 waitForKey = true;
             } else if(args[i].equals("-justGears")) {
                 addComp = false;
+            } else if(args[i].equals("-shutdownKeepGLCanvas")) {
+                shutdownRemoveGLCanvas = false;
+            } else if(args[i].equals("-shutdownKeepFrame")) {
+                shutdownDisposeFrame = false;
+            } else if(args[i].equals("-shutdownKeepAll")) {
+                shutdownRemoveGLCanvas = false;
+                shutdownDisposeFrame = false;
+            } else if(args[i].equals("-shutdownSystemExit")) {
+                shutdownSystemExit = true;
             }
         }
         System.err.println("forceES2 "+forceES2);