Fix NEWT Window destroy/close race condition,

where a programatic window.destroy() call from thread 1
triggers a destroy() call via the native windowing toolkit
via windowDestroyNotify().
It has to be checked/locked if a destroy is in progress,
otherwise they could deadlock (OSX and Win32).

4 files changed, 94 insertions, 57 deletions

diff --git a/src/newt/classes/com/jogamp/newt/Window.java b/src/newt/classes/com/jogamp/newt/Window.java
index 410144653..171bb2468 100755
--- a/src/newt/classes/com/jogamp/newt/Window.java
+++ b/src/newt/classes/com/jogamp/newt/Window.java
@@ -321,12 +321,12 @@ public abstract class Window implements NativeWindow
         return lockedStack;
     }
 
-    public synchronized void destroy() {
+    public void destroy() {
         destroy(false);
     }
 
     /** @param deep If true, the linked Screen and Display will be destroyed as well. */
-    public synchronized void destroy(boolean deep) {
+    public void destroy(boolean deep) {
         if(DEBUG_WINDOW_EVENT) {
             System.out.println("Window.destroy() start (deep "+deep+" - "+Thread.currentThread());
         }
@@ -342,26 +342,33 @@ public abstract class Window implements NativeWindow
         synchronized(keyListeners) {
             keyListeners = new ArrayList();
         }
-        Screen scr = screen;
-        Display dpy = (null!=screen) ? screen.getDisplay() : null;
-        EventDispatchThread edt = (null!=dpy) ? dpy.getEDT() : null;
-        if(null!=edt) {
-            final Window f_win = this;
-            edt.invokeAndWait(new Runnable() {
-                public void run() {
-                    f_win.closeNative();
+        synchronized(this) {
+            destructionLock.lock();
+            try {
+                Screen scr = screen;
+                Display dpy = (null!=screen) ? screen.getDisplay() : null;
+                EventDispatchThread edt = (null!=dpy) ? dpy.getEDT() : null;
+                if(null!=edt) {
+                    final Window f_win = this;
+                    edt.invokeAndWait(new Runnable() {
+                        public void run() {
+                            f_win.closeNative();
+                        }
+                    } );
+                } else {
+                    closeNative();
                 }
-            } );
-        } else {
-            closeNative();
-        }
-        invalidate();
-        if(deep) {
-            if(null!=scr) {
-                scr.destroy();
-            }
-            if(null!=dpy) {
-                dpy.destroy();
+                invalidate();
+                if(deep) {
+                    if(null!=scr) {
+                        scr.destroy();
+                    }
+                    if(null!=dpy) {
+                        dpy.destroy();
+                    }
+                }
+            } finally {
+                destructionLock.unlock();
             }
         }
         if(DEBUG_WINDOW_EVENT) {
@@ -487,12 +494,12 @@ public abstract class Window implements NativeWindow
 
     protected void windowDestroyNotify() {
         if(DEBUG_WINDOW_EVENT) {
-            System.out.println("Window.windowDestroyeNotify start "+Thread.currentThread());
+            System.out.println("Window.windowDestroyNotify start "+Thread.currentThread());
         }
 
         sendWindowEvent(WindowEvent.EVENT_WINDOW_DESTROY_NOTIFY);
 
-        if(!autoDrawableMember) {
+        if(!autoDrawableMember && !destructionLock.isLocked()) {
             destroy();
         }
 
@@ -505,7 +512,9 @@ public abstract class Window implements NativeWindow
         if(DEBUG_WINDOW_EVENT) {
             System.out.println("Window.windowDestroyed "+Thread.currentThread());
         }
-        invalidate();
+        if(!destructionLock.isLocked()) {
+            invalidate();
+        }
     }
 
     public abstract void    setVisible(boolean visible);
@@ -554,6 +563,12 @@ public abstract class Window implements NativeWindow
         }
     }
 
+    public void removeAllSurfaceUpdatedListener() {
+        synchronized(surfaceUpdatedListeners) {
+            surfaceUpdatedListeners = new ArrayList();
+        }
+    }
+
     public SurfaceUpdatedListener[] getSurfaceUpdatedListener() {
         synchronized(surfaceUpdatedListeners) {
             return (SurfaceUpdatedListener[]) surfaceUpdatedListeners.toArray();
@@ -926,4 +941,54 @@ public abstract class Window implements NativeWindow
         }
         return sb.toString();
     }
+
+    //
+    // Reentrance locking toolkit
+    // 
+    public static class WindowToolkitLock implements ToolkitLock {
+            private Thread owner;
+            private int recursionCount;
+            
+            public boolean isOwner() {
+                return isOwner(Thread.currentThread());
+            }
+
+            public synchronized boolean isOwner(Thread thread) {
+                return owner == thread ;
+            }
+
+            public synchronized boolean isLocked() {
+                return null != owner;
+            }
+
+            public synchronized void lock() {
+                Thread cur = Thread.currentThread();
+                if (owner == cur) {
+                    ++recursionCount;
+                    return;
+                }
+                while (owner != null) {
+                    try {
+                        wait();
+                    } catch (InterruptedException e) {
+                        throw new RuntimeException(e);
+                    }
+                }
+                owner = cur;
+            }
+
+            public synchronized void unlock() {
+                if (owner != Thread.currentThread()) {
+                    throw new RuntimeException("Not owner");
+                }
+                if (recursionCount > 0) {
+                    --recursionCount;
+                    return;
+                }
+                owner = null;
+                notifyAll();
+            }
+    }
+    private WindowToolkitLock destructionLock = new WindowToolkitLock();
 }
+
diff --git a/src/newt/classes/com/jogamp/newt/macosx/MacWindow.java b/src/newt/classes/com/jogamp/newt/macosx/MacWindow.java
index 06e73caa5..276843709 100755
--- a/src/newt/classes/com/jogamp/newt/macosx/MacWindow.java
+++ b/src/newt/classes/com/jogamp/newt/macosx/MacWindow.java
@@ -212,38 +212,7 @@ public class MacWindow extends Window {
         }
     }
 
-    private ToolkitLock nsViewLock = new ToolkitLock() {
-            private Thread owner;
-            private int recursionCount;
-            
-            public synchronized void lock() {
-                Thread cur = Thread.currentThread();
-                if (owner == cur) {
-                    ++recursionCount;
-                    return;
-                }
-                while (owner != null) {
-                    try {
-                        wait();
-                    } catch (InterruptedException e) {
-                        throw new RuntimeException(e);
-                    }
-                }
-                owner = cur;
-            }
-
-            public synchronized void unlock() {
-                if (owner != Thread.currentThread()) {
-                    throw new RuntimeException("Not owner");
-                }
-                if (recursionCount > 0) {
-                    --recursionCount;
-                    return;
-                }
-                owner = null;
-                notifyAll();
-            }
-        };
+    private WindowToolkitLock nsViewLock = new WindowToolkitLock();
 
     public synchronized int lockSurface() throws NativeWindowException {
         nsViewLock.lock();
diff --git a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java
index 29a392f04..4a7f27f3a 100644
--- a/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java
+++ b/src/newt/classes/com/jogamp/newt/opengl/GLWindow.java
@@ -335,6 +335,9 @@ public class GLWindow extends Window implements GLAutoDrawable {
     public void removeSurfaceUpdatedListener(SurfaceUpdatedListener l) {
         window.removeSurfaceUpdatedListener(l);
     }
+    public void removeAllSurfaceUpdatedListener() {
+        window.removeAllSurfaceUpdatedListener();
+    }
     public SurfaceUpdatedListener[] getSurfaceUpdatedListener() {
         return window.getSurfaceUpdatedListener();
     }
diff --git a/src/newt/native/NewtMacWindow.m b/src/newt/native/NewtMacWindow.m
index 146c04de1..3d8d32a40 100755
--- a/src/newt/native/NewtMacWindow.m
+++ b/src/newt/native/NewtMacWindow.m
@@ -441,7 +441,7 @@ static jint mods2JavaMods(NSUInteger mods)
     }
 
     (*env)->CallVoidMethod(env, javaWindowObject, windowDestroyNotifyID);
-    // Will be called by Window.java (*env)->CallVoidMethod(env, javaWindowObject, windowDestroyedID);
+    (*env)->CallVoidMethod(env, javaWindowObject, windowDestroyedID); // No OSX hook for DidClose, so do it here 
 
     // EOL ..
     (*env)->DeleteGlobalRef(env, javaWindowObject);