NEWT: Fix EDTUtil/DefaultEDTUtil - Locking/Behaviour

EDTUtil:
    - Added documentation to EDTUtil interface
    - Removed 'stop()', introduced 'invokeStop(Runnable)',
      allowing proper shutdown with a final task (see doc).
    - Fix (c) header, since it is a new file, changed to interface with df161c9fcc1bc90d03e374e0eb8148424d4c5577

DefaultEDTUtil:
    - Move EDT scope volatile states (shouldStop, edtTasks)
      into inner class EventDispatchThread (the actual EDT).
      Fetching those outer instance values from within the inner EDT instance
      results in 'old values', ie this memory didn't get updates/synced.

    - Give the thread a proper name:
        <invocation-thread-name>-<custom-name>-EDT-<start-sequence-number>
      This allows easy identification of the EDT incl. indication of the 'start' sequence number.

    - Added fail-fast Exceptions in case of a stopped EDT with remaining tasks.
      This should validate/test the runtime behavior.

    - Ensure isRunning(), invokeStop(..) and waitUntilStopped()
      only returns after the last task has been executed.

    - invokeImpl/EDT.run: Complete task-lock coverage incl. wait case.
      - The final task is blocked until the wait() state in invokeImpl is reached.
      - EDT executes the final task and notifies the lock owner - then exist
      - invokeImpl leaves the wait state.
      - See EDTUtil API doc ..

3 files changed, 206 insertions, 120 deletions

diff --git a/src/newt/classes/com/jogamp/newt/util/DefaultEDTUtil.java b/src/newt/classes/com/jogamp/newt/impl/DefaultEDTUtil.java
index 47c089f87..7a76c94fc 100644
--- a/src/newt/classes/com/jogamp/newt/util/DefaultEDTUtil.java
+++ b/src/newt/classes/com/jogamp/newt/impl/DefaultEDTUtil.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2009 Sun Microsystems, Inc. All Rights Reserved.
+ * Copyright (c) 2010 JogAmp Community. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -34,36 +35,38 @@
  * facility.
  */
 
-package com.jogamp.newt.util;
+package com.jogamp.newt.impl;
 
 import com.jogamp.common.util.RunnableTask;
 import com.jogamp.newt.Display;
-import com.jogamp.newt.impl.Debug;
+import com.jogamp.newt.util.EDTUtil;
 import java.util.*;
 
 public class DefaultEDTUtil implements EDTUtil {
     public static final boolean DEBUG = Debug.debug("EDT");
 
     private ThreadGroup threadGroup; 
-    private volatile boolean shouldStop = false;
     private EventDispatchThread edt = null;
     private Object edtLock = new Object(); // locking the EDT start/stop state
-    private ArrayList edtTasks = new ArrayList(); // one shot tasks
     private String name;
-    private Runnable pumpMessages;
+    int start_iter=0;
+    private Runnable dispatchMessages;
 
-    public DefaultEDTUtil(ThreadGroup tg, String name, Runnable pumpMessages) {
+    public DefaultEDTUtil(ThreadGroup tg, String name, Runnable dispatchMessages) {
         this.threadGroup = tg;
-        this.name=new String(Thread.currentThread().getName()+"-EDT-"+name);
-        this.pumpMessages=pumpMessages;
-        this.edt = new EventDispatchThread(threadGroup, name+"-EDT");
+        this.name=new String(Thread.currentThread().getName()+"-"+name+"-EDT-");
+        this.dispatchMessages=dispatchMessages;
+        this.edt = new EventDispatchThread(threadGroup, name);
     }
 
     public final void reset() {
         synchronized(edtLock) { 
             waitUntilStopped();
-            if(edtTasks.size()>0) {
-                throw new RuntimeException("Remaining EDTTasks: "+edtTasks.size());
+            if(edt.tasks.size()>0) {
+                throw new RuntimeException("Remaining EDTTasks: "+edt.tasks.size()+" - "+edt);
+            }
+            if(DEBUG) {
+                System.err.println(Thread.currentThread()+": EDT reset - edt: "+edt);
             }
             this.edt = new EventDispatchThread(threadGroup, name);
         }
@@ -72,24 +75,17 @@ public class DefaultEDTUtil implements EDTUtil {
     public final void start() {
         synchronized(edtLock) { 
             if(!edt.isRunning()) {
-                shouldStop = false;
-                edt.start();
-                if(DEBUG) {
-                    System.out.println(Thread.currentThread()+": EDT START");
+                if(edt.isAlive()) {
+                    throw new RuntimeException("EDT Thread.isAlive(): true, isRunning: "+edt.isRunning()+", edt: "+edt+", tasks: "+edt.tasks.size());
                 }
-            }
-        }
-    }
-
-    public final void stop() {
-        synchronized(edtLock) { 
-            if(edt.isRunning()) {
-                shouldStop = true;
+                start_iter++;
+                edt.setName(name+start_iter);
+                edt.shouldStop = false;
                 if(DEBUG) {
-                    System.out.println(Thread.currentThread()+": EDT signal STOP");
+                    System.err.println(Thread.currentThread()+": EDT START - edt: "+edt);
                 }
+                edt.start();
             }
-            edtLock.notifyAll();
         }
     }
 
@@ -98,36 +94,68 @@ public class DefaultEDTUtil implements EDTUtil {
     }
 
     public final boolean isRunning() {
-        return !shouldStop && edt.isRunning() ;
+        return edt.isRunning() ;
+    }
+
+    public final void invokeStop(Runnable task) {
+        invokeImpl(true, task, true);
+    }
+
+    public final void invoke(boolean wait, Runnable task) {
+        invokeImpl(wait, task, false);
     }
 
-    public void invoke(boolean wait, Runnable task) {
+    private final void invokeImpl(boolean wait, Runnable task, boolean stop) {
         if(task == null) {
-            return;
+            throw new RuntimeException("Null Runnable");
         }
         Throwable throwable = null;
         RunnableTask rTask = null;
         Object rTaskLock = new Object();
         synchronized(rTaskLock) { // lock the optional task execution
-            if( isCurrentThreadEDT() ) {
-                wait = false;
-                task.run();
-            } else {
-                synchronized(edtLock) { // lock the EDT status
+            synchronized(edtLock) { // lock the EDT status
+                if( edt.shouldStop ) {
+                    throw new RuntimeException("EDT about to stop: "+edt);
+                }
+                if( isCurrentThreadEDT() ) {
+                    if(stop) {
+                        edt.shouldStop = true;
+                        if(DEBUG) {
+                            System.err.println(Thread.currentThread()+": EDT signal STOP (edt) - edt: "+edt);
+                        }
+                    }
+                    if(!wait && edt.tasks.size()>0) {
+                        // append task to ensure proper sequence
+                        edt.tasks.add(rTask);
+                    } else {
+                        // wait or last task, execute now
+                        task.run();
+                    }
+                    wait = false; // running in same thread (EDT) -> no wait
+                    if(stop && edt.tasks.size()>0) {
+                        throw new RuntimeException("Remaining EDTTasks: "+edt.tasks.size()+" - "+edt);
+                    }
+                } else {
                     start(); // start if not started yet
-                    rTask = new RunnableTask(task, wait?rTaskLock:null, true);
-                    synchronized(edtTasks) {
-                        edtTasks.add(rTask);
-                        edtTasks.notifyAll();
+                    rTask = new RunnableTask(task,
+                                             wait ? rTaskLock : null,
+                                             wait /* catch Exceptions if waiting for result */);
+                    synchronized(edt.tasks) {
+                        if(stop) {
+                            edt.shouldStop = true;
+                            if(DEBUG) {
+                                System.err.println(Thread.currentThread()+": EDT signal STOP (!edt) - edt: "+edt);
+                            }
+                        }
+                        // append task ..
+                        edt.tasks.add(rTask);
+                        edt.tasks.notifyAll();
                     }
                 }
             }
-
-            // wait until task finished, if requested
-            // and no stop() call slipped through.
-            if( wait && isRunning() ) {
+            if( wait ) {
                 try {
-                    rTaskLock.wait();
+                    rTaskLock.wait(); // free lock, allow execution of rTask
                 } catch (InterruptedException ie) {
                     throwable = ie;
                 }
@@ -139,14 +167,17 @@ public class DefaultEDTUtil implements EDTUtil {
                 }
             }
         }
+        if(DEBUG && stop) {
+            System.err.println(Thread.currentThread()+": EDT signal STOP X edt: "+edt);
+        }
     }
 
     public void waitUntilIdle() {
         if(edt.isRunning() && edt != Thread.currentThread()) {
-            synchronized(edtTasks) {
-                while(edt.isRunning() && edtTasks.size()>0) {
+            synchronized(edt.tasks) {
+                while(edt.isRunning() && edt.tasks.size()>0) {
                     try {
-                        edtTasks.wait();
+                        edt.tasks.wait();
                     } catch (InterruptedException e) {
                         e.printStackTrace();
                     }
@@ -170,7 +201,9 @@ public class DefaultEDTUtil implements EDTUtil {
     }
 
     class EventDispatchThread extends Thread {
+        volatile boolean shouldStop = false;
         volatile boolean isRunning = false;
+        ArrayList tasks = new ArrayList(); // one shot tasks
 
         public EventDispatchThread(ThreadGroup tg, String name) {
             super(tg, name);
@@ -186,55 +219,61 @@ public class DefaultEDTUtil implements EDTUtil {
         }
 
         /** 
-         * Utilizing locking only on edtTasks and its execution,
+         * Utilizing locking only on tasks and its execution,
          * not for event dispatching.
          */
         public void run() {
             if(DEBUG) {
-                System.out.println(Thread.currentThread()+": EDT run() START");
+                System.err.println(getName()+": EDT run() START "+ getName());
             }
             try {
                 do {
                     // event dispatch
                     if(!shouldStop) {
-                        pumpMessages.run();
+                        dispatchMessages.run();
                     }
                     // wait and work on tasks
                     Runnable task = null;
-                    synchronized(edtTasks) {
+                    synchronized(tasks) {
                         // wait for tasks
-                        while(!shouldStop && edtTasks.size()==0) {
+                        if(!shouldStop && tasks.size()==0) {
                             try {
-                                edtTasks.wait(defaultEDTPollGranularity);
+                                tasks.wait(defaultEDTPollGranularity);
                             } catch (InterruptedException e) {
                                 e.printStackTrace();
                             }
                         }
                         // execute one task, if available
-                        if(edtTasks.size()>0) {
-                            task = (Runnable) edtTasks.remove(0);
-                            edtTasks.notifyAll();
+                        if(tasks.size()>0) {
+                            task = (Runnable) tasks.remove(0);
+                            tasks.notifyAll();
                         }
                     }
                     if(null!=task) {
                         task.run();
                     }
-                } while(!shouldStop || edtTasks.size()>0) ;
+                } while(!shouldStop) ;
             } catch (Throwable t) {
                 // handle errors ..
                 shouldStop = true;
-                throw new RuntimeException(t);
+                throw new RuntimeException(getName()+": EDT run() Error", t);
             } finally {
-                // check for tasks
-                // sync for waitUntilStopped()
+                if(DEBUG) {
+                    System.err.println(getName()+": EDT run() END "+ getName()); 
+                }
                 synchronized(edtLock) {
+                    synchronized(tasks) {
+                        if(tasks.size()>0) {
+                            throw new RuntimeException("Remaining EDTTasks: "+tasks.size()+" - "+edt);
+                        }
+                    }
                     isRunning = !shouldStop;
                     if(!isRunning) {
                         edtLock.notifyAll();
                     }
                 }
                 if(DEBUG) {
-                    System.out.println(Thread.currentThread()+": EDT run() EXIT");
+                    System.err.println(getName()+": EDT run() EXIT "+ getName());
                 }
             }
         }
diff --git a/src/newt/classes/com/jogamp/newt/util/EDTUtil.java b/src/newt/classes/com/jogamp/newt/util/EDTUtil.java
index 969da6c2d..d1a11a788 100644
--- a/src/newt/classes/com/jogamp/newt/util/EDTUtil.java
+++ b/src/newt/classes/com/jogamp/newt/util/EDTUtil.java
@@ -1,76 +1,114 @@
-/*
- * Copyright (c) 2009 Sun Microsystems, Inc. All Rights Reserved.
- * 
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are
- * met:
- * 
- * - Redistribution of source code must retain the above copyright
- *   notice, this list of conditions and the following disclaimer.
- * 
- * - Redistribution in binary form must reproduce the above copyright
- *   notice, this list of conditions and the following disclaimer in the
- *   documentation and/or other materials provided with the distribution.
- * 
- * Neither the name of Sun Microsystems, Inc. or the names of
- * contributors may be used to endorse or promote products derived from
- * this software without specific prior written permission.
- * 
- * This software is provided "AS IS," without a warranty of any kind. ALL
- * EXPRESS OR IMPLIED CONDITIONS, REPRESENTATIONS AND WARRANTIES,
- * INCLUDING ANY IMPLIED WARRANTY OF MERCHANTABILITY, FITNESS FOR A
- * PARTICULAR PURPOSE OR NON-INFRINGEMENT, ARE HEREBY EXCLUDED. SUN
- * MICROSYSTEMS, INC. ("SUN") AND ITS LICENSORS SHALL NOT BE LIABLE FOR
- * ANY DAMAGES SUFFERED BY LICENSEE AS A RESULT OF USING, MODIFYING OR
- * DISTRIBUTING THIS SOFTWARE OR ITS DERIVATIVES. IN NO EVENT WILL SUN OR
- * ITS LICENSORS BE LIABLE FOR ANY LOST REVENUE, PROFIT OR DATA, OR FOR
- * DIRECT, INDIRECT, SPECIAL, CONSEQUENTIAL, INCIDENTAL OR PUNITIVE
- * DAMAGES, HOWEVER CAUSED AND REGARDLESS OF THE THEORY OF LIABILITY,
- * ARISING OUT OF THE USE OF OR INABILITY TO USE THIS SOFTWARE, EVEN IF
- * SUN HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES.
- * 
- * You acknowledge that this software is not designed or intended for use
- * in the design, construction, operation or maintenance of any nuclear
- * facility.
+/**
+ * Copyright 2010 JogAmp Community. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are
+ * permitted provided that the following conditions are met:
+ *
+ *    1. Redistributions of source code must retain the above copyright notice, this list of
+ *       conditions and the following disclaimer.
+ *
+ *    2. Redistributions in binary form must reproduce the above copyright notice, this list
+ *       of conditions and the following disclaimer in the documentation and/or other materials
+ *       provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY JogAmp Community ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL JogAmp Community OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ * The views and conclusions contained in the software and documentation are those of the
+ * authors and should not be interpreted as representing official policies, either expressed
+ * or implied, of JogAmp Community.
  */
 
 package com.jogamp.newt.util;
 
-import com.jogamp.common.util.RunnableTask;
-import com.jogamp.newt.Display;
-import java.util.*;
-
+/**
+ * EDT stands for Event Dispatch Thread.
+ * <p>
+ * EDTUtil comprises the functionality of:
+ * <ul>
+ *   <li> Periodically issuing an event dispatch command on the EDT.</li>
+ *   <li> Ability to enqueue tasks, executed on the EDT.</li>
+ *   <li> Controlling the EDT, ie start and stop in a sane manner.</li>
+ * </ul>
+ * The EDT pattern is a common tool to comply with todays windowing toolkits,
+ * where the common denominator in regards to multithreading is to:
+ * <ul>
+ *   <li> Create a Window on one thread </li>
+ *   <li> Modify the Window within the same thread </li>
+ *   <li> Handle incoming events from within the same thread </li>
+ * </ul>
+ * Note: This is not true on MacOSX, where all these actions have to be
+ * performed by a unique, so called main thread.<br>
+ */
 public interface EDTUtil {
 
     public static final long defaultEDTPollGranularity = 10; // 10ms, 1/100s
 
+    /**
+     * Create a new EDT. One should invoke <code>reset()</code><br>
+     * after <code>invokeStop(..)</code> in case another <code>start()</code> or <code>invoke(..)</code>
+     * is expected.
+     *
+     * @see #start()
+     * @see #invoke(boolean, java.lang.Runnable)
+     * @see #invokeStop(java.lang.Runnable)
+     */
     public void reset();
 
+    /**
+     * Start the EDT
+     */
     public void start();
 
-    public void stop();
-
+    /**
+     * @return True if the current thread is the EDT thread
+     */
     public boolean isCurrentThreadEDT();
 
+    /**
+     * @return True if EDT is running
+     */
     public boolean isRunning();
 
     /** 
-     * Add task to the EDT task queue.
-     * Wait until execution is finished if wait is true.
-     * Shall start the thread if not running 
+     * Append the final task to the EDT task queue,
+     * signals EDT to stop and wait until stopped.<br>
+     * Due to the nature of this method:
+     * <ul>
+     *   <li>All previous queued tasks will be finished.</li>
+     *   <li>No new tasks are allowed, an Exception is thrown.</li>
+     *   <li>Can be issued from within EDT, ie from within an enqueued task.</li>
+     *   <li>{@link #reset()} may follow immediately, ie creating a new EDT</li>
+     * </ul>
+     */
+    public void invokeStop(Runnable finalTask);
+
+    /** 
+     * Append task to the EDT task queue.<br>
+     * Wait until execution is finished if <code>wait == true</code>.<br>
+     * Shall start the thread if not running.<br>
+     * Can be issued from within EDT, ie from within an enqueued task.<br>
+     *
+     * @throws RuntimeException in case EDT is stopped and not {@link #reset()}
      */
     public void invoke(boolean wait, Runnable task);
 
     /** 
-     * Wait until EDT task queue, filled via invoke, is empty.
-     * It is allowed that the last task is still in execution
-     * when this method returns. 
+     * Wait until the EDT task queue is empty.<br>
+     * The last task may still be in execution when this method returns.
      */
     public void waitUntilIdle();
 
     /**
-     * Wait until EDT task has stopped.
-     * stop is not exected here and should be beforehand.
+     * Wait until EDT task is stopped.<br>
+     * No <code>stop</code> action is performed, {@link #invokeStop(java.lang.Runnable)} should be used before.
      */
     public void waitUntilStopped();
 }
diff --git a/src/newt/classes/com/jogamp/newt/util/MainThread.java b/src/newt/classes/com/jogamp/newt/util/MainThread.java
index cb5c87a89..5b26cfbda 100644
--- a/src/newt/classes/com/jogamp/newt/util/MainThread.java
+++ b/src/newt/classes/com/jogamp/newt/util/MainThread.java
@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2009 Sun Microsystems, Inc. All Rights Reserved.
+ * Copyright (c) 2010 JogAmp Community. All rights reserved.
  * 
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
@@ -148,7 +149,10 @@ public class MainThread implements EDTUtil {
             if(DEBUG) System.err.println("MainAction.run(): "+Thread.currentThread().getName()+" user app fin");
 
             if ( useMainThread ) {
-                singletonMainThread.stop();
+                singletonMainThread.invokeStop(new Runnable() {
+                    public void run() {
+                        // nop
+                    }});
                 if(DEBUG) System.err.println("MainAction.run(): "+Thread.currentThread().getName()+" MainThread fin - stop");
                 System.exit(0);
             }
@@ -251,17 +255,6 @@ public class MainThread implements EDTUtil {
         // nop
     }
 
-    public void stop() {
-        if(DEBUG) System.err.println("MainThread.stop(): "+Thread.currentThread().getName()+" start");
-        synchronized(taskWorkerLock) { 
-            if(isRunning) {
-                shouldStop = true;
-            }
-            taskWorkerLock.notifyAll();
-        }
-        if(DEBUG) System.err.println("MainThread.stop(): "+Thread.currentThread().getName()+" end");
-    }
-
     public boolean isCurrentThreadEDT() {
         if(NativeWindowFactory.isAWTAvailable()) {
             initAWTReflection();
@@ -291,8 +284,15 @@ public class MainThread implements EDTUtil {
         }
     }
 
-    /** invokes the given Runnable */
+    public void invokeStop(Runnable r) {
+        invokeImpl(true, r, true);
+    }
+
     public void invoke(boolean wait, Runnable r) {
+        invokeImpl(wait, r, false);
+    }
+
+    private void invokeImpl(boolean wait, Runnable r, boolean stop) {
         if(r == null) {
             return;
         }
@@ -319,6 +319,7 @@ public class MainThread implements EDTUtil {
 
         // if this main thread is not being used or
         // if this is already the main thread .. just execute.
+        // FIXME: start if not started .. sync logic with DefaultEDTUtil!!!
         if( !isRunning() || mainThread == Thread.currentThread() ) {
             r.run();
             return;
@@ -330,6 +331,14 @@ public class MainThread implements EDTUtil {
         Throwable throwable = null;
         synchronized(lock) {
             invokeLater(rTask);
+            // FIXME ..
+            synchronized(taskWorkerLock) { 
+                if(isRunning) {
+                    shouldStop = true;
+                    if(DEBUG) System.err.println("MainThread.stop(): "+Thread.currentThread().getName()+" start");
+                }
+                taskWorkerLock.notifyAll();
+            }
             if( doWait ) {
                 try {
                     lock.wait();