NEWT: Using WeakReferences for global cache of Display, Screen and Window instances; Removing ref. at API destroy() is wrong ; Allow GC to clear ..

- Removing ref. at API destroy() is wrong
  - Since all instances can be recreated, removing ref at destroy() is simply wrong.
  - Keep weak references until GC collects, i.e. user does not claim them anymore.
    - Safe for Display, since it holds it's EDT thread.
    - Window/Screen .. if user abandons reference .. nothing we can do here.

- Allow GC to clear ..
  No need to hold ref loonger than user.

6 files changed, 166 insertions, 84 deletions

diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawable.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawable.java
index 910158d1f..4bd7bc994 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawable.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLDrawable.java
@@ -117,8 +117,7 @@ public abstract class MacOSXCGLDrawable extends GLDrawableImpl {
             createdContexts.add(new WeakReference<MacOSXCGLContext>(osxCtx));
         } else {
             for(int i=0; i<createdContexts.size(); ) {
-                final WeakReference<MacOSXCGLContext> ref = createdContexts.get(i); 
-                final MacOSXCGLContext _ctx = ref.get();
+                final MacOSXCGLContext _ctx = createdContexts.get(i).get();
                 if( _ctx == null || _ctx == ctx) {
                     createdContexts.remove(i);
                 } else {
@@ -134,8 +133,7 @@ public abstract class MacOSXCGLDrawable extends GLDrawableImpl {
     if(doubleBuffered) { 
         synchronized (createdContexts) {
             for(int i=0; i<createdContexts.size(); ) {
-                final WeakReference<MacOSXCGLContext> ref = createdContexts.get(i); 
-                final MacOSXCGLContext ctx = ref.get();
+                final MacOSXCGLContext ctx = createdContexts.get(i).get();
                 if (ctx != null) {
                     ctx.swapBuffers();
                     i++;
diff --git a/src/newt/classes/com/jogamp/newt/Display.java b/src/newt/classes/com/jogamp/newt/Display.java
index d6ddd9613..4f5df6c70 100644
--- a/src/newt/classes/com/jogamp/newt/Display.java
+++ b/src/newt/classes/com/jogamp/newt/Display.java
@@ -31,6 +31,7 @@ package com.jogamp.newt;
 import com.jogamp.newt.util.EDTUtil;
 import jogamp.newt.Debug;
 
+import java.lang.ref.WeakReference;
 import java.util.*;
 
 import javax.media.nativewindow.AbstractGraphicsDevice;
@@ -178,16 +179,16 @@ public abstract class Display {
     public abstract void dispatchMessages();
     
     // Global Displays
-    protected static final ArrayList<Display> displayList = new ArrayList<Display>();
+    protected static final ArrayList<WeakReference<Display>> displayList = new ArrayList<WeakReference<Display>>();
     protected static int displaysActive = 0;
 
     public static void dumpDisplayList(String prefix) {
         synchronized(displayList) {
-            Iterator<Display> i = displayList.iterator();
             System.err.println(prefix+" DisplayList[] entries: "+displayList.size()+" - "+getThreadName());
-            for(int j=0; i.hasNext(); j++) {
-                Display d = i.next();
-                System.err.println("  ["+j+"] : "+d);
+            final Iterator<WeakReference<Display>> ri = displayList.iterator();
+            for(int j=0; ri.hasNext(); j++) {
+                final Display d = ri.next().get();
+                System.err.println("  ["+j+"] : "+d+", GC'ed "+(null==d));
             }
         }
     }
@@ -216,29 +217,62 @@ public abstract class Display {
         return getDisplayOfImpl(type, name, fromIndex, -1, shared);
     }
 
-    private static Display getDisplayOfImpl(String type, String name, int fromIndex, int incr, boolean shared) {
+    private static Display getDisplayOfImpl(String type, String name, final int fromIndex, final int incr, boolean shared) {
         synchronized(displayList) {
             int i = fromIndex >= 0 ? fromIndex : displayList.size() - 1 ;
             while( ( incr > 0 ) ? i < displayList.size() : i >= 0 ) {
-                Display display = (Display) displayList.get(i);
-                if( display.getType().equals(type) &&
-                    display.getName().equals(name) &&
-                    ( !shared || shared && !display.isExclusive() ) 
-                  ) {
-                    return display;
+                final Display display = (Display) displayList.get(i).get();
+                if( null == display ) {
+                    // Clear GC'ed dead reference entry!
+                    displayList.remove(i);
+                    if( incr < 0 ) {
+                        // decrease
+                        i+=incr;
+                    } // else nop - remove shifted subsequent elements to the left
+                } else {
+                    if( display.getType().equals(type) &&
+                        display.getName().equals(name) &&
+                        ( !shared || shared && !display.isExclusive() ) 
+                      ) {
+                        return display;
+                    }
+                    i+=incr;
                 }
-                i+=incr;
             }
         }
         return null;
     }
-
+    
+    protected static void addDisplay2List(Display display) {
+        synchronized(displayList) {
+            // GC before add
+            int i=0;
+            while( i < displayList.size() ) {
+                if( null == displayList.get(i).get() ) {
+                    displayList.remove(i);
+                } else {
+                    i++;
+                }
+            }
+            displayList.add(new WeakReference<Display>(display));
+        }
+    }
+    
     /** Returns the global display collection */
-    @SuppressWarnings("unchecked")
     public static Collection<Display> getAllDisplays() {
         ArrayList<Display> list;
         synchronized(displayList) {
-            list = (ArrayList<Display>) displayList.clone();
+            list = new ArrayList<Display>();
+            int i = 0;
+            while( i < displayList.size() ) {
+                final Display d = displayList.get(i).get();
+                if( null == d ) {
+                    displayList.remove(i);
+                } else {
+                    list.add( displayList.get(i).get() );
+                    i++;
+                }
+            }
         }
         return list;
     }
diff --git a/src/newt/classes/com/jogamp/newt/Screen.java b/src/newt/classes/com/jogamp/newt/Screen.java
index cf8145561..f56ee344b 100644
--- a/src/newt/classes/com/jogamp/newt/Screen.java
+++ b/src/newt/classes/com/jogamp/newt/Screen.java
@@ -29,6 +29,8 @@ package com.jogamp.newt;
 
 import com.jogamp.newt.event.MonitorModeListener;
 import jogamp.newt.Debug;
+
+import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
@@ -224,7 +226,7 @@ public abstract class Screen {
     public abstract void removeMonitorModeListener(MonitorModeListener sml);
 
     // Global Screens
-    protected static ArrayList<Screen> screenList = new ArrayList<Screen>();
+    protected static final ArrayList<WeakReference<Screen>> screenList = new ArrayList<WeakReference<Screen>>();
     protected static int screensActive = 0;
 
     /**
@@ -253,26 +255,60 @@ public abstract class Screen {
         synchronized(screenList) {
             int i = fromIndex >= 0 ? fromIndex : screenList.size() - 1 ;
             while( ( incr > 0 ) ? i < screenList.size() : i >= 0 ) {
-                Screen screen = (Screen) screenList.get(i);
-                if( screen.getDisplay().equals(display) &&
-                    screen.getIndex() == idx ) {
-                    return screen;
+                final Screen screen = (Screen) screenList.get(i).get();
+                if( null == screen ) {
+                    // Clear GC'ed dead reference entry!
+                    screenList.remove(i);
+                    if( incr < 0 ) {
+                        // decrease
+                        i+=incr;
+                    } // else nop - remove shifted subsequent elements to the left
+                } else {
+                    if( screen.getDisplay().equals(display) &&
+                        screen.getIndex() == idx ) {
+                        return screen;
+                    }
+                    i+=incr;
                 }
-                i+=incr;
             }
         }
         return null;
     }
-    /** Returns the global display collection */
-    @SuppressWarnings("unchecked")
+    
+    protected static void addScreen2List(Screen screen) {
+        synchronized(screenList) {
+            // GC before add
+            int i=0;
+            while( i < screenList.size() ) {
+                if( null == screenList.get(i).get() ) {
+                    screenList.remove(i);
+                } else {
+                    i++;
+                }
+            }
+            screenList.add(new WeakReference<Screen>(screen));
+        }
+    }
+    
+    /** Returns the global screen collection */
     public static Collection<Screen> getAllScreens() {
         ArrayList<Screen> list;
         synchronized(screenList) {
-            list = (ArrayList<Screen>) screenList.clone();
+            list = new ArrayList<Screen>();
+            int i = 0;
+            while( i < screenList.size() ) {
+                final Screen s = screenList.get(i).get();
+                if( null == s ) {
+                    screenList.remove(i);
+                } else {
+                    list.add( screenList.get(i).get() );
+                    i++;
+                }
+            }
         }
         return list;
     }
-
+    
     public static int getActiveScreenNumber() {
         synchronized(screenList) {
             return screensActive;
diff --git a/src/newt/classes/jogamp/newt/DisplayImpl.java b/src/newt/classes/jogamp/newt/DisplayImpl.java
index 0d1dcf5ab..bb493cbbd 100644
--- a/src/newt/classes/jogamp/newt/DisplayImpl.java
+++ b/src/newt/classes/jogamp/newt/DisplayImpl.java
@@ -41,6 +41,7 @@ import com.jogamp.newt.event.NEWTEventConsumer;
 
 import jogamp.newt.event.NEWTEventTask;
 import com.jogamp.newt.util.EDTUtil;
+
 import java.util.ArrayList;
 
 import javax.media.nativewindow.AbstractGraphicsDevice;
@@ -96,7 +97,7 @@ public abstract class DisplayImpl extends Display {
                 display.id = serialno++;
                 display.fqname = getFQName(display.type, display.name, display.id);
                 display.hashCode = display.fqname.hashCode();
-                displayList.add(display);
+                Display.addDisplay2List(display);
             }
             display.setEDTUtil(display.edtUtil); // device's default if EDT is used, or null
             
@@ -155,11 +156,11 @@ public abstract class DisplayImpl extends Display {
             if(null==aDevice) {
                 throw new NativeWindowException("Display.createNative() failed to instanciate an AbstractGraphicsDevice");
             }
-            if(DEBUG) {
-                System.err.println("Display.createNative() END ("+getThreadName()+", "+this+")");
-            }
             synchronized(displayList) {
                 displaysActive++;
+                if(DEBUG) {
+                    System.err.println("Display.createNative() END ("+getThreadName()+", "+this+", active "+displaysActive+")");
+                }
             }
         }
     }
@@ -238,13 +239,12 @@ public abstract class DisplayImpl extends Display {
             dumpDisplayList("Display.destroy("+getFQName()+") BEGIN");
         }
         synchronized(displayList) {
-            displayList.remove(this);
             if(0 < displaysActive) {
                 displaysActive--;
             }
-        }
-        if(DEBUG) {
-            System.err.println("Display.destroy(): "+this+" "+getThreadName());
+            if(DEBUG) {
+                System.err.println("Display.destroy(): "+this+", active "+displaysActive+" "+getThreadName());
+            }
         }
         final DisplayImpl f_dpy = this;
         removeEDT( new Runnable() { // blocks!
@@ -268,32 +268,34 @@ public abstract class DisplayImpl extends Display {
             dumpDisplayList("Display.shutdownAll "+dCount+" instances, on thread "+getThreadName());
         }
         for(int i=0; i<dCount && displayList.size()>0; i++) { // be safe ..
-            final DisplayImpl d = (DisplayImpl) displayList.remove(0);
-            if(0 < displaysActive) {
-                displaysActive--;
-            }
+            final DisplayImpl d = (DisplayImpl) displayList.remove(0).get();
             if(DEBUG) {
-                System.err.println("Display.shutdownAll["+(i+1)+"/"+dCount+"]: "+d);
+                System.err.println("Display.shutdownAll["+(i+1)+"/"+dCount+"]: "+d+", GCed "+(null==d));
             }
-            final Runnable closeNativeTask = new Runnable() {
-                public void run() {
-                    if ( null != d.getGraphicsDevice() ) {
-                        d.closeNativeImpl();
+            if( null != d ) { // GC'ed ?
+                if(0 < displaysActive) {
+                    displaysActive--;
+                }
+                final Runnable closeNativeTask = new Runnable() {
+                    public void run() {
+                        if ( null != d.getGraphicsDevice() ) {
+                            d.closeNativeImpl();
+                        }
                     }
+                };
+                final EDTUtil edtUtil = d.getEDTUtil();
+                if(null != edtUtil) {
+                    final long coopSleep = edtUtil.getPollPeriod() * 2;
+                    edtUtil.invokeStop(false, closeNativeTask); // don't block
+                    try {
+                        Thread.sleep( coopSleep < 50 ? coopSleep : 50 );
+                    } catch (InterruptedException e) { }
+                } else {
+                    closeNativeTask.run();
                 }
-            };
-            final EDTUtil edtUtil = d.getEDTUtil();
-            if(null != edtUtil) {
-                final long coopSleep = edtUtil.getPollPeriod() * 2;
-                edtUtil.invokeStop(false, closeNativeTask); // don't block
-                try {
-                    Thread.sleep( coopSleep < 50 ? coopSleep : 50 );
-                } catch (InterruptedException e) { }
-            } else {
-                closeNativeTask.run();
+                d.aDevice = null;
+                d.refCount=0;
             }
-            d.aDevice = null;
-            d.refCount=0;
         }
     }
 
diff --git a/src/newt/classes/jogamp/newt/ScreenImpl.java b/src/newt/classes/jogamp/newt/ScreenImpl.java
index c02f4f288..fe9e91b57 100644
--- a/src/newt/classes/jogamp/newt/ScreenImpl.java
+++ b/src/newt/classes/jogamp/newt/ScreenImpl.java
@@ -125,7 +125,7 @@ public abstract class ScreenImpl extends Screen implements MonitorModeListener {
                 screen.screen_idx = idx;
                 screen.fqname = display.getFQName()+"-s"+idx;
                 screen.hashCode = screen.fqname.hashCode();
-                screenList.add(screen);
+                Screen.addScreen2List(screen);
                 if(DEBUG) {
                     System.err.println("Screen.create() NEW: "+screen+" "+Display.getThreadName());
                 }
@@ -169,8 +169,7 @@ public abstract class ScreenImpl extends Screen implements MonitorModeListener {
                 System.err.println("Screen.createNative() START ("+DisplayImpl.getThreadName()+", "+this+")");
             } else {
                 tCreated = 0;
-            }
-            
+            }            
             display.addReference();
             
             createNativeImpl();
@@ -179,11 +178,11 @@ public abstract class ScreenImpl extends Screen implements MonitorModeListener {
             }
 
             initMonitorState();
-            if(DEBUG) {
-                System.err.println("Screen.createNative() END ("+DisplayImpl.getThreadName()+", "+this+"), total "+ (System.nanoTime()-tCreated)/1e6 +"ms");
-            }
             synchronized(screenList) {
                 screensActive++;
+                if(DEBUG) {
+                    System.err.println("Screen.createNative() END ("+DisplayImpl.getThreadName()+", "+this+"), active "+screensActive+", total "+ (System.nanoTime()-tCreated)/1e6 +"ms");
+                }
             }
             ScreenMonitorState.getScreenMonitorState(this.getFQName()).addListener(this);
         }
@@ -192,10 +191,12 @@ public abstract class ScreenImpl extends Screen implements MonitorModeListener {
     @Override
     public synchronized final void destroy() {
         synchronized(screenList) {
-            if( screenList.remove(this) ) {
-                if(0 < screensActive) {
-                    screensActive--;
-                }
+            if(0 < screensActive) {
+                screensActive--;
+            }
+            if(DEBUG) {
+                System.err.println("Screen.destroy() ("+DisplayImpl.getThreadName()+"): active "+screensActive);
+                // Thread.dumpStack();
             }
         }
 
@@ -667,11 +668,13 @@ public abstract class ScreenImpl extends Screen implements MonitorModeListener {
             System.err.println("Screen.shutdownAll "+sCount+" instances, on thread "+Display.getThreadName());
         }
         for(int i=0; i<sCount && screenList.size()>0; i++) { // be safe ..
-            final ScreenImpl s = (ScreenImpl) screenList.remove(0);
+            final ScreenImpl s = (ScreenImpl) screenList.remove(0).get();
             if(DEBUG) {
-                System.err.println("Screen.shutdownAll["+(i+1)+"/"+sCount+"]: "+s);
+                System.err.println("Screen.shutdownAll["+(i+1)+"/"+sCount+"]: "+s+", GCed "+(null==s));
+            }
+            if( null != s ) {
+                s.shutdown();
             }
-            s.shutdown();
         }
     }
 }
diff --git a/src/newt/classes/jogamp/newt/WindowImpl.java b/src/newt/classes/jogamp/newt/WindowImpl.java
index 7c4011f34..460763f47 100644
--- a/src/newt/classes/jogamp/newt/WindowImpl.java
+++ b/src/newt/classes/jogamp/newt/WindowImpl.java
@@ -36,6 +36,7 @@ package jogamp.newt;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.lang.ref.WeakReference;
 import java.lang.reflect.Method;
 
 import com.jogamp.common.util.IntBitfield;
@@ -82,7 +83,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
 {
     public static final boolean DEBUG_TEST_REPARENT_INCOMPATIBLE = Debug.isPropertyDefined("newt.test.Window.reparent.incompatible", true);
     
-    protected static final ArrayList<WindowImpl> windowList = new ArrayList<WindowImpl>();
+    protected static final ArrayList<WeakReference<WindowImpl>> windowList = new ArrayList<WeakReference<WindowImpl>>();    
     
     static {
         ScreenImpl.initSingleton();
@@ -95,13 +96,28 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
             System.err.println("Window.shutdownAll "+wCount+" instances, on thread "+getThreadName());
         }
         for(int i=0; i<wCount && windowList.size()>0; i++) { // be safe ..
-            final WindowImpl w = windowList.remove(0);
+            final WindowImpl w = windowList.remove(0).get();
             if(DEBUG_IMPLEMENTATION) {
-                System.err.println("Window.shutdownAll["+(i+1)+"/"+wCount+"]: "+toHexString(w.getWindowHandle()));
+                final long wh = null != w ? w.getWindowHandle() : 0;
+                System.err.println("Window.shutdownAll["+(i+1)+"/"+wCount+"]: "+toHexString(wh)+", GCed "+(null==w));
             }
             w.shutdown();
         }
     }
+    private static void addWindow2List(WindowImpl window) {
+        synchronized(windowList) {
+            // GC before add
+            int i=0;
+            while( i < windowList.size() ) {
+                if( null == windowList.get(i).get() ) {
+                    windowList.remove(i);
+                } else {
+                    i++;
+                }
+            }
+            windowList.add(new WeakReference<WindowImpl>(window));
+        }
+    }
     
     /** Timeout of queued events (repaint and resize) */
     static final long QUEUED_EVENT_TO = 1200; // ms    
@@ -208,9 +224,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
             window.screen = (ScreenImpl) screen;
             window.capsRequested = (CapabilitiesImmutable) caps.cloneMutable();
             window.instantiationFinished();
-            synchronized( windowList ) {
-                windowList.add(window);
-            }
+            addWindow2List(window);
             return window;
         } catch (Throwable t) {
             t.printStackTrace();
@@ -233,9 +247,7 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
             window.screen = (ScreenImpl) screen;
             window.capsRequested = (CapabilitiesImmutable) caps.cloneMutable();
             window.instantiationFinished();
-            synchronized( windowList ) {
-                windowList.add(window);
-            }
+            addWindow2List(window);
             return window;
         } catch (Throwable t) {
             throw new NativeWindowException(t);
@@ -1062,9 +1074,6 @@ public abstract class WindowImpl implements Window, NEWTEventConsumer
 
     @Override
     public void destroy() {
-        synchronized( windowList ) {
-            windowList.remove(this);
-        }        
         visible = false; // Immediately mark synchronized visibility flag, avoiding possible recreation 
         runOnEDTIfAvail(true, destroyAction);
     }