Cleanup shutdown mechanism ; Fix X11/ATI SIGV at shutdown ; EGLDisplayUtil: Check for leaked display handles

GLProfile / all shutdown methods: Remove ShutdownType to remove complexity (not required)

Proper shutdown sequence:
  GLProfile - GLDrawableFactory+ - GLContext - NativeWindowFactory - [X11Util, OSXUtil, ..]

GLDrawableFactory: Always keep shutdown-hook alive, required for X11Util shutdown (@ JVMShutdown only)

X11Util: Shutdown
  - @ JVMShutdown only
  - If GL vendor ATI: close pending X11 display connections in proper order of creation.
    This finally removes the SIGV when shutting down the JVM on X11 w/ ATI driver.

EGLDisplayUtil: Add shutdown, allowing to validate whether leaked EGL display handles remain.

5 files changed, 142 insertions, 58 deletions

diff --git a/src/nativewindow/classes/javax/media/nativewindow/NativeWindowFactory.java b/src/nativewindow/classes/javax/media/nativewindow/NativeWindowFactory.java
index afcd0a008..89d476a3b 100644
--- a/src/nativewindow/classes/javax/media/nativewindow/NativeWindowFactory.java
+++ b/src/nativewindow/classes/javax/media/nativewindow/NativeWindowFactory.java
@@ -102,6 +102,8 @@ public abstract class NativeWindowFactory {
     private static Constructor<?> x11ToolkitLockConstructor;
     private static boolean requiresToolkitLock;
 
+    private static volatile boolean isJVMShuttingDown = false;
+    
     /** Creates a new NativeWindowFactory instance. End users do not
         need to call this method. */
     protected NativeWindowFactory() {
@@ -168,6 +170,22 @@ public abstract class NativeWindowFactory {
         }        
     }
 
+    private static void shutdownNativeImpl(final ClassLoader cl) {
+        final String clazzName;
+        if( TYPE_X11 == nativeWindowingTypePure ) {
+            clazzName = X11UtilClassName;
+        } else if( TYPE_WINDOWS == nativeWindowingTypePure ) {
+            clazzName = GDIClassName;
+        } else if( TYPE_MACOSX == nativeWindowingTypePure ) {
+            clazzName = OSXUtilClassName;
+        } else {
+            clazzName = null;
+        }
+        if( null != clazzName ) {
+            ReflectionUtil.callStaticMethod(clazzName, "shutdown", null, null, cl );
+        }        
+    }
+    
     /**
      * Static one time initialization of this factory.<br>
      * This initialization method <b>must be called</b> once by the program or utilizing modules!
@@ -268,22 +286,28 @@ public abstract class NativeWindowFactory {
         }
     }
 
-    public static synchronized void shutdown() {
+    public static synchronized void shutdown(boolean _isJVMShuttingDown) {
+        isJVMShuttingDown = _isJVMShuttingDown;
+        if(DEBUG) {
+            System.err.println(Thread.currentThread().getName()+" - NativeWindowFactory.shutdown() START: JVM Shutdown "+isJVMShuttingDown);                
+        }
         if(initialized) {
             initialized = false;
-            if(DEBUG) {
-                System.err.println(Thread.currentThread().getName()+" - NativeWindowFactory.shutdown() START");                
+            if(null != registeredFactories) {
+                registeredFactories.clear();
+                registeredFactories = null;
             }
-            registeredFactories.clear();
-            registeredFactories = null;
             GraphicsConfigurationFactory.shutdown();
-            // X11Util.shutdown(..) already called via GLDrawableFactory.shutdown() ..
-            if(DEBUG) {
-                System.err.println(Thread.currentThread().getName()+" - NativeWindowFactory.shutdown() END");                
-            }
+        }
+        shutdownNativeImpl(NativeWindowFactory.class.getClassLoader()); // always re-shutdown
+        if(DEBUG) {
+            System.err.println(Thread.currentThread().getName()+" - NativeWindowFactory.shutdown() END JVM Shutdown "+isJVMShuttingDown);
         }
     }
     
+    /** Returns true if the JVM is shutting down, otherwise false. */ 
+    public static final boolean isJVMShuttingDown() { return isJVMShuttingDown; }
+    
     /** @return true if the underlying toolkit requires locking, otherwise false. */
     public static boolean requiresToolkitLock() {
         return requiresToolkitLock;
diff --git a/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java b/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java
index 36d7c3727..f1e8a786a 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java
@@ -48,6 +48,7 @@ import java.util.ArrayList;
 import java.util.Map;
 
 import javax.media.nativewindow.NativeWindowException;
+import javax.media.nativewindow.NativeWindowFactory;
 import javax.media.nativewindow.ToolkitLock;
 
 import jogamp.nativewindow.Debug;
@@ -271,10 +272,18 @@ public class JAWTUtil {
     }
   }
 
+  /**
+   * Called by {@link NativeWindowFactory#initSingleton()}
+   */
   public static void initSingleton() {
       // just exist to ensure static init has been run
   }
-
+  
+  /**
+   * Called by {@link NativeWindowFactory#shutdown()}
+   */
+  public static void shutdown() {      
+  }
 
   public static boolean hasJava2D() {
     return j2dExist;
diff --git a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
index f5f735051..149ebdf4a 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/macosx/OSXUtil.java
@@ -28,6 +28,7 @@
 package jogamp.nativewindow.macosx;
 
 import javax.media.nativewindow.NativeWindowException;
+import javax.media.nativewindow.NativeWindowFactory;
 import javax.media.nativewindow.util.Insets;
 import javax.media.nativewindow.util.Point;
 
@@ -38,6 +39,9 @@ public class OSXUtil {
     private static boolean isInit = false;  
     private static final boolean DEBUG = Debug.debug("OSXUtil");
     
+    /**
+     * Called by {@link NativeWindowFactory#initSingleton()}
+     */
     public static synchronized void initSingleton() {
       if(!isInit) {
           if(DEBUG) {
@@ -54,6 +58,12 @@ public class OSXUtil {
       }
     }
 
+    /**
+     * Called by {@link NativeWindowFactory#shutdown()}
+     */
+    public static void shutdown() {      
+    }
+    
     public static boolean requiresToolkitLock() {
         return false;
     }
diff --git a/src/nativewindow/classes/jogamp/nativewindow/windows/GDIUtil.java b/src/nativewindow/classes/jogamp/nativewindow/windows/GDIUtil.java
index fda1649b6..613c76032 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/windows/GDIUtil.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/windows/GDIUtil.java
@@ -29,6 +29,7 @@ package jogamp.nativewindow.windows;
 
 import javax.media.nativewindow.util.Point;
 import javax.media.nativewindow.NativeWindowException;
+import javax.media.nativewindow.NativeWindowFactory;
 
 import jogamp.nativewindow.NWJNILibLoader;
 import jogamp.nativewindow.Debug;
@@ -41,6 +42,9 @@ public class GDIUtil {
     private static RegisteredClassFactory dummyWindowClassFactory;
     private static boolean isInit = false;
   
+    /**
+     * Called by {@link NativeWindowFactory#initSingleton()}
+     */
     public static synchronized void initSingleton() {
         if(!isInit) {
             synchronized(X11Util.class) {
@@ -61,6 +65,12 @@ public class GDIUtil {
         }
     }
   
+    /**
+     * Called by {@link NativeWindowFactory#shutdown()}
+     */
+    public static void shutdown() {      
+    }
+    
     public static boolean requiresToolkitLock() { return false; }
   
     private static RegisteredClass dummyWindowClass = null;
diff --git a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
index 860238649..93b7f3487 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
@@ -67,13 +67,13 @@ public class X11Util {
      * </p>
      * <p>
      * You may test this, ie just reverse the destroy order below.
-     * See also native test: jogl/test/native/displayMultiple02.c
+     * See also native test: jogl/test-native/displayMultiple02.c
      * </p>
      * <p>
      * Workaround is to not close them at all if driver vendor is ATI.
      * </p>
      */
-    public static final boolean ATI_HAS_XCLOSEDISPLAY_BUG = true;
+    public static final boolean ATI_HAS_XCLOSEDISPLAY_BUG = !Debug.isPropertyDefined("nativewindow.debug.X11Util.ATI_HAS_NO_XCLOSEDISPLAY_BUG", true);
 
     /** Value is <code>true</code>, best 'stable' results if always using XInitThreads(). */
     public static final boolean XINITTHREADS_ALWAYS_ENABLED = true;
@@ -95,6 +95,9 @@ public class X11Util {
     private static Object setX11ErrorHandlerLock = new Object();
 
     
+    /**
+     * Called by {@link NativeWindowFactory#initSingleton()}
+     */
     public static void initSingleton() {
         if(!isInit) {
             synchronized(X11Util.class) {
@@ -138,6 +141,52 @@ public class X11Util {
         }
     }
     
+    /** 
+     * Cleanup resources.
+     * <p>
+     * Called by {@link NativeWindowFactory#shutdown()}
+     * </p>
+     */
+    public static void shutdown() {
+        if(isInit) {
+            synchronized(X11Util.class) {
+                if(isInit) {                    
+                    final boolean isJVMShuttingDown = NativeWindowFactory.isJVMShuttingDown() ;
+                    if(DEBUG || openDisplayMap.size() > 0 || reusableDisplayList.size() > 0 || pendingDisplayList.size() > 0) {
+                        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()+
+                                           ")");
+                        if(DEBUG) {
+                            Thread.dumpStack();
+                        }
+                        if( openDisplayList.size() > 0) {
+                            X11Util.dumpOpenDisplayConnections();
+                        }
+                        if( reusableDisplayList.size() > 0 || pendingDisplayList.size() > 0 ) {
+                            X11Util.dumpPendingDisplayConnections();
+                        }
+                    }
+            
+                    synchronized(globalLock) {
+                        // Only at JVM shutdown time, since AWT impl. seems to 
+                        // dislike closing of X11 Display's (w/ ATI driver). 
+                        if( isJVMShuttingDown ) {
+                            isInit = false;                            
+                            closePendingDisplayConnections();    
+                            openDisplayList.clear();
+                            reusableDisplayList.clear();
+                            pendingDisplayList.clear();
+                            openDisplayMap.clear();
+                            shutdown0();
+                        }
+                    }
+                }
+            }
+        }
+    }
+
     public static synchronized boolean isNativeLockAvailable() {
         return isX11LockAvailable;
     }
@@ -183,6 +232,7 @@ public class X11Util {
     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 {
@@ -218,8 +268,7 @@ public class X11Util {
         public final boolean equals(Object obj) {
             if(this == obj) { return true; }
             if(obj instanceof NamedDisplay) {
-                NamedDisplay n = (NamedDisplay) obj;
-                return handle == n.handle;
+                return handle == ((NamedDisplay) obj).handle;
             }
             return false;
         }
@@ -246,43 +295,6 @@ public class X11Util {
         }
     }
 
-    /** 
-     * Cleanup resources. 
-     * If <code>realXCloseOpenAndPendingDisplays</code> is <code>false</code>, 
-     * keep alive all references (open display connection) for restart on same ClassLoader.
-     * 
-     * @return number of unclosed X11 Displays.<br>
-     * @param realXCloseOpenAndPendingDisplays if true, {@link #closePendingDisplayConnections()} is called.
-     */
-    public static int shutdown(boolean realXCloseOpenAndPendingDisplays, boolean verbose) {
-        int num=0;
-        if(DEBUG || verbose || openDisplayMap.size() > 0 || pendingDisplayList.size() > 0) {
-            System.err.println("X11Util.Display: Shutdown (close open / pending Displays: "+realXCloseOpenAndPendingDisplays+
-                               ", open (no close attempt): "+openDisplayMap.size()+"/"+openDisplayList.size()+
-                               ", pending (not closed, marked uncloseable): "+pendingDisplayList.size()+")");
-            if(DEBUG) {
-                Thread.dumpStack();
-            }
-            if( openDisplayList.size() > 0) {
-                X11Util.dumpOpenDisplayConnections();
-            }
-            if( pendingDisplayList.size() > 0 ) {
-                X11Util.dumpPendingDisplayConnections();
-            }
-        }
-
-        synchronized(globalLock) {
-            if(realXCloseOpenAndPendingDisplays) {
-                closePendingDisplayConnections();    
-                openDisplayList.clear();
-                pendingDisplayList.clear();
-                openDisplayMap.clear();
-                shutdown0();
-            }
-        }
-        return num;
-    }
-
     /**
      * Closing pending Display connections in reverse order.
      *
@@ -292,9 +304,9 @@ public class X11Util {
         int num=0;
         synchronized(globalLock) {
             if(DEBUG) {
-                System.err.println("X11Util: Closing Pending X11 Display Connections: "+pendingDisplayList.size());
+                System.err.println("X11Util: Closing Pending X11 Display Connections in order of their creation: "+pendingDisplayList.size());
             }
-            for(int i=pendingDisplayList.size()-1; i>=0; i--) {
+            for(int i=0; i<pendingDisplayList.size(); i++) {
                 NamedDisplay ndpy = (NamedDisplay) pendingDisplayList.get(i);
                 if(DEBUG) {
                     System.err.println("X11Util.closePendingDisplayConnections(): Closing ["+i+"]: "+ndpy);
@@ -328,6 +340,12 @@ public class X11Util {
         }
     }
     
+    public static int getReusableDisplayConnectionNumber() {
+        synchronized(globalLock) {
+            return reusableDisplayList.size();
+        }
+    }
+
     public static int getPendingDisplayConnectionNumber() {
         synchronized(globalLock) {
             return pendingDisplayList.size();
@@ -336,7 +354,18 @@ public class X11Util {
 
     public static void dumpPendingDisplayConnections() {
         synchronized(globalLock) {
-            System.err.println("X11Util: Pending X11 Display Connections: "+pendingDisplayList.size());
+            System.err.println("X11Util: Reusable X11 Display Connections: "+reusableDisplayList.size());
+            for(int i=0; i<reusableDisplayList.size(); i++) {
+                NamedDisplay ndpy = (NamedDisplay) reusableDisplayList.get(i);
+                System.err.println("X11Util: Reusable["+i+"]: "+ndpy);
+                if(null!=ndpy) {
+                    Throwable t = ndpy.getCreationStack();
+                    if(null!=t) {
+                        t.printStackTrace();
+                    }
+                }
+            }
+            System.err.println("X11Util: Pending X11 Display Connections (creation order): "+pendingDisplayList.size());
             for(int i=0; i<pendingDisplayList.size(); i++) {
                 NamedDisplay ndpy = (NamedDisplay) pendingDisplayList.get(i);
                 System.err.println("X11Util: Pending["+i+"]: "+ndpy);
@@ -370,9 +399,9 @@ public class X11Util {
         boolean reused = false;
         
         synchronized(globalLock) {            
-            for(int i=0; i<pendingDisplayList.size(); i++) {
-                if(pendingDisplayList.get(i).getName().equals(name)) {
-                    namedDpy = pendingDisplayList.remove(i);
+            for(int i=0; i<reusableDisplayList.size(); i++) {
+                if(reusableDisplayList.get(i).getName().equals(name)) {
+                    namedDpy = reusableDisplayList.remove(i);
                     dpy = namedDpy.getHandle();
                     reused = true;
                     break;
@@ -386,6 +415,7 @@ public class X11Util {
                 // if you like to debug and synchronize X11 commands ..
                 // setSynchronizeDisplay(dpy, true);
                 namedDpy = new NamedDisplay(name, dpy);
+                pendingDisplayList.add(namedDpy);
             }
             namedDpy.addRef();
             openDisplayMap.put(dpy, namedDpy);
@@ -420,9 +450,10 @@ public class X11Util {
             
             if(!namedDpy.isUncloseable()) {
                 XCloseDisplay(namedDpy.getHandle());
+                pendingDisplayList.remove(namedDpy);
             } else {
                 // for reuse
-                pendingDisplayList.add(namedDpy);
+                reusableDisplayList.add(namedDpy);
             }
             
             if(DEBUG) {