Clean/Fix: Threading Code

- Remove unsafe double checked locking
- Annotate safe double checked locking (volatile)
- use 'static final' if possible

11 files changed, 90 insertions, 80 deletions

diff --git a/src/jogl/classes/com/jogamp/opengl/JoglVersion.java b/src/jogl/classes/com/jogamp/opengl/JoglVersion.java
index ef1ecd7e7..34108bce6 100644
--- a/src/jogl/classes/com/jogamp/opengl/JoglVersion.java
+++ b/src/jogl/classes/com/jogamp/opengl/JoglVersion.java
@@ -46,7 +46,7 @@ public class JoglVersion extends JogampVersion {
     }
 
     public static JoglVersion getInstance() {
-        if(null == jogampCommonVersionInfo) {
+        if(null == jogampCommonVersionInfo) { // volatile: ok
             synchronized(JoglVersion.class) {
                 if( null == jogampCommonVersionInfo ) {
                     final String packageName = "javax.media.opengl";
diff --git a/src/jogl/classes/jogamp/opengl/GLWorkerThread.java b/src/jogl/classes/jogamp/opengl/GLWorkerThread.java
index e57cbe0bc..ac9655fbb 100644
--- a/src/jogl/classes/jogamp/opengl/GLWorkerThread.java
+++ b/src/jogl/classes/jogamp/opengl/GLWorkerThread.java
@@ -40,7 +40,6 @@
 package jogamp.opengl;
 
 import java.lang.reflect.InvocationTargetException;
-import java.security.*;
 import java.util.*;
 import javax.media.opengl.*;
 
@@ -69,9 +68,11 @@ public class GLWorkerThread {
   
   /** Should only be called by Threading class if creation of the
       GLWorkerThread was requested via the opengl.1thread system
-      property. */
+      property. <br>
+   * Start the GLWorkerThread iff not started yet!
+   */
   public static void start() {
-    if (!started) {
+    if (!started) { // volatile: ok
       synchronized (GLWorkerThread.class) {
         if (!started) {
           lock = new Object();
diff --git a/src/jogl/classes/jogamp/opengl/ThreadingImpl.java b/src/jogl/classes/jogamp/opengl/ThreadingImpl.java
index d63699aad..67a950185 100644
--- a/src/jogl/classes/jogamp/opengl/ThreadingImpl.java
+++ b/src/jogl/classes/jogamp/opengl/ThreadingImpl.java
@@ -203,13 +203,7 @@ public class ThreadingImpl {
                 throw new InternalError();
 
             case WORKER:
-                if (!GLWorkerThread.isStarted()) {
-                    synchronized (GLWorkerThread.class) {
-                        if (!GLWorkerThread.isStarted()) {
-                            GLWorkerThread.start();
-                        }
-                    }
-                }
+                GLWorkerThread.start(); // singleton start via volatile-dbl-checked-locking
                 try {
                     GLWorkerThread.invokeAndWait(r);
                 } catch (InvocationTargetException e) {
diff --git a/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java b/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java
index a681c5b8f..dd493f5ee 100644
--- a/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java
+++ b/src/jogl/classes/jogamp/opengl/awt/AWTThreadingPlugin.java
@@ -103,13 +103,7 @@ public class AWTThreadingPlugin implements ThreadingPlugin {
         break;
 
       case ThreadingImpl.WORKER:
-        if (!GLWorkerThread.isStarted()) {
-          synchronized (GLWorkerThread.class) {
-            if (!GLWorkerThread.isStarted()) {
-              GLWorkerThread.start();
-            }
-          }
-        }
+        GLWorkerThread.start(); // singleton start via volatile-dbl-checked-locking
         try {
           GLWorkerThread.invokeAndWait(r);
         } catch (InvocationTargetException e) {
diff --git a/src/nativewindow/classes/com/jogamp/nativewindow/NativeWindowVersion.java b/src/nativewindow/classes/com/jogamp/nativewindow/NativeWindowVersion.java
index ccb2176fc..38bd70a90 100644
--- a/src/nativewindow/classes/com/jogamp/nativewindow/NativeWindowVersion.java
+++ b/src/nativewindow/classes/com/jogamp/nativewindow/NativeWindowVersion.java
@@ -42,7 +42,7 @@ public class NativeWindowVersion extends JogampVersion {
     }
 
     public static NativeWindowVersion getInstance() {
-        if(null == jogampCommonVersionInfo) {
+        if(null == jogampCommonVersionInfo) { // volatile: ok
             synchronized(NativeWindowVersion.class) {
                 if( null == jogampCommonVersionInfo ) {
                     final String packageName = "javax.media.nativewindow";
diff --git a/src/nativewindow/classes/javax/media/nativewindow/awt/AWTWindowClosingProtocol.java b/src/nativewindow/classes/javax/media/nativewindow/awt/AWTWindowClosingProtocol.java
index 7112f944d..e7db942e4 100644
--- a/src/nativewindow/classes/javax/media/nativewindow/awt/AWTWindowClosingProtocol.java
+++ b/src/nativewindow/classes/javax/media/nativewindow/awt/AWTWindowClosingProtocol.java
@@ -51,6 +51,7 @@ public class AWTWindowClosingProtocol implements WindowClosingProtocol {
   }
 
   class WindowClosingAdapter extends WindowAdapter {
+    @Override
     public void windowClosing(WindowEvent e) {
       int op = AWTWindowClosingProtocol.this.getDefaultCloseOperation();
 
@@ -80,7 +81,7 @@ public class AWTWindowClosingProtocol implements WindowClosingProtocol {
    * @return
    */
   public final boolean addClosingListenerOneShot() {
-    if(!closingListenerSet) {
+    if(!closingListenerSet) { // volatile: ok
       synchronized(closingListenerLock) {
         if(!closingListenerSet) {
             closingListenerSet=addClosingListenerImpl();
@@ -92,7 +93,7 @@ public class AWTWindowClosingProtocol implements WindowClosingProtocol {
   }
 
   public final boolean removeClosingListener() {
-    if(closingListenerSet) {
+    if(closingListenerSet) { // volatile: ok
       synchronized(closingListenerLock) {
         if(closingListenerSet) {
             Window w = AWTMisc.getWindow(comp);
diff --git a/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java b/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java
index d5811d9c5..80dd5c402 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/jawt/JAWTUtil.java
@@ -60,13 +60,23 @@ public class JAWTUtil {
   private static final Method isQueueFlusherThread;
   private static final boolean j2dExist;
 
-  private static Class   sunToolkitClass;
-  private static Method  sunToolkitAWTLockMethod;
-  private static Method  sunToolkitAWTUnlockMethod;
-  private static boolean hasSunToolkitAWTLock;
+  private static final Class   sunToolkitClass;
+  private static final Method  sunToolkitAWTLockMethod;
+  private static final Method  sunToolkitAWTUnlockMethod;
+  private static final boolean hasSunToolkitAWTLock;
 
   private static final JAWTToolkitLock jawtToolkitLock;
 
+  private static class PrivilegedDataBlob1 {
+    PrivilegedDataBlob1() {
+        ok = false;
+    }  
+    Class   sunToolkitClass;
+    Method  sunToolkitAWTLockMethod;
+    Method  sunToolkitAWTUnlockMethod;
+    boolean ok;
+  }
+  
   static {
     JAWTJNILibLoader.loadAWTImpl();
     JAWTJNILibLoader.loadNativeWindow("awt");
@@ -87,22 +97,28 @@ public class JAWTUtil {
     isQueueFlusherThread = m;
     j2dExist = ok;
 
-    AccessController.doPrivileged(new PrivilegedAction() {
+    PrivilegedDataBlob1 pdb1 = (PrivilegedDataBlob1) AccessController.doPrivileged(new PrivilegedAction() {        
         public Object run() {
-            try {
-                sunToolkitClass = Class.forName("sun.awt.SunToolkit");
-                sunToolkitAWTLockMethod = sunToolkitClass.getDeclaredMethod("awtLock", new Class[]{});
-                sunToolkitAWTLockMethod.setAccessible(true);
-                sunToolkitAWTUnlockMethod = sunToolkitClass.getDeclaredMethod("awtUnlock", new Class[]{});
-                sunToolkitAWTUnlockMethod.setAccessible(true);
+            PrivilegedDataBlob1 d = new PrivilegedDataBlob1();
+            try {                
+                d.sunToolkitClass = Class.forName("sun.awt.SunToolkit");
+                d.sunToolkitAWTLockMethod = sunToolkitClass.getDeclaredMethod("awtLock", new Class[]{});
+                d.sunToolkitAWTLockMethod.setAccessible(true);
+                d.sunToolkitAWTUnlockMethod = sunToolkitClass.getDeclaredMethod("awtUnlock", new Class[]{});
+                d.sunToolkitAWTUnlockMethod.setAccessible(true);
+                d.ok=true;
             } catch (Exception e) {
                 // Either not a Sun JDK or the interfaces have changed since 1.4.2 / 1.5
             }
-            return null;
+            return d;
         }
     });
+    sunToolkitClass = pdb1.sunToolkitClass;
+    sunToolkitAWTLockMethod = pdb1.sunToolkitAWTLockMethod;
+    sunToolkitAWTUnlockMethod = pdb1.sunToolkitAWTUnlockMethod;
+    
     boolean _hasSunToolkitAWTLock = false;
-    if (null != sunToolkitAWTLockMethod && null != sunToolkitAWTUnlockMethod) {
+    if ( pdb1.ok ) {
         try {
             sunToolkitAWTLockMethod.invoke(null, (Object[])null);
             sunToolkitAWTUnlockMethod.invoke(null, (Object[])null);
@@ -152,11 +168,11 @@ public class JAWTUtil {
   }
 
 
-  public static final boolean hasJava2D() {
+  public static boolean hasJava2D() {
     return j2dExist;
   }
 
-  public static final boolean isJava2DQueueFlusherThread() {
+  public static boolean isJava2DQueueFlusherThread() {
     boolean b = false;
     if(j2dExist) {
         try {
diff --git a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
index 4098e101b..052a9934e 100644
--- a/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
+++ b/src/nativewindow/classes/jogamp/nativewindow/x11/X11Util.java
@@ -54,7 +54,7 @@ public class X11Util {
     private static final boolean DEBUG = Debug.debug("X11Util");
     private static final boolean TRACE_DISPLAY_LIFECYCLE = Debug.getBooleanProperty("nativewindow.debug.X11Util.TraceDisplayLifecycle", true, AccessController.getContext());
 
-    private static String nullDisplayName = null;
+    private static volatile String nullDisplayName = null;
     private static boolean isFirstX11ActionOnProcess = false;
     private static boolean isInit = false;
 
@@ -117,7 +117,7 @@ public class X11Util {
     }
 
     public static String getNullDisplayName() {
-        if(null==nullDisplayName) {
+        if(null==nullDisplayName) { // volatile: ok
             synchronized(X11Util.class) {
                 if(null==nullDisplayName) {
                     NativeWindowFactory.getDefaultToolkitLock().lock();
@@ -174,17 +174,19 @@ public class X11Util {
 
         public final Throwable getCreationStack() { return creationStack; }
 
+        @Override
         public Object clone() throws CloneNotSupportedException {
           return super.clone();
         }
 
+        @Override
         public String toString() {
             return "NamedX11Display["+name+", 0x"+Long.toHexString(handle)+", refCount "+refCount+", unCloseable "+unCloseable+"]";
         }
     }
 
     /** Returns the number of unclosed X11 Displays.
-      * @param realXCloseAndPendingDisplays if true, {@link #closePendingDisplayConnections()} is called.
+     * @param realXCloseOpenAndPendingDisplays if true, {@link #closePendingDisplayConnections()} is called.
       */
     public static int shutdown(boolean realXCloseOpenAndPendingDisplays, boolean verbose) {
         int num=0;
diff --git a/src/newt/classes/com/jogamp/newt/NewtVersion.java b/src/newt/classes/com/jogamp/newt/NewtVersion.java
index a38dca34c..961ffdf6a 100644
--- a/src/newt/classes/com/jogamp/newt/NewtVersion.java
+++ b/src/newt/classes/com/jogamp/newt/NewtVersion.java
@@ -43,7 +43,7 @@ public class NewtVersion extends JogampVersion {
     }
 
     public static NewtVersion getInstance() {
-        if(null == jogampCommonVersionInfo) {
+        if(null == jogampCommonVersionInfo) { // volatile: ok
             synchronized(NewtVersion.class) {
                 if( null == jogampCommonVersionInfo ) {
                     final String packageName = "com.jogamp.newt";
diff --git a/src/newt/classes/com/jogamp/newt/util/MainThread.java b/src/newt/classes/com/jogamp/newt/util/MainThread.java
index 9f66fefe6..8bb725b99 100644
--- a/src/newt/classes/com/jogamp/newt/util/MainThread.java
+++ b/src/newt/classes/com/jogamp/newt/util/MainThread.java
@@ -91,25 +91,25 @@ import jogamp.newt.awt.AWTEDTUtil;
  * Which starts 4 threads, each with a window and OpenGL rendering.<br>
  */
 public class MainThread implements EDTUtil {
-    private static AccessControlContext localACC = AccessController.getContext();
+    private static final AccessControlContext localACC = AccessController.getContext();
     public static final boolean  MAIN_THREAD_CRITERIA = ( !NativeWindowFactory.isAWTAvailable() &&
                                                            NativeWindowFactory.TYPE_MACOSX.equals(NativeWindowFactory.getNativeWindowType(false)) 
                                                         ) || Debug.getBooleanProperty("newt.MainThread.force", true, localACC);
 
     protected static final boolean DEBUG = Debug.debug("MainThread");
 
-    private static MainThread singletonMainThread = new MainThread(); // one singleton MainThread
+    private static final MainThread singletonMainThread = new MainThread(); // one singleton MainThread
 
     private static boolean isExit=false;
     private static volatile boolean isRunning=false;
-    private static Object taskWorkerLock=new Object();
+    private static final Object taskWorkerLock=new Object();
     private static boolean shouldStop;
     private static ArrayList tasks;
     private static Thread mainThread;
 
     private static Timer pumpMessagesTimer=null;
     private static TimerTask pumpMessagesTimerTask=null;
-    private static Map/*<Display, Runnable>*/ pumpMessageDisplayMap = new HashMap();
+    private static final Map/*<Display, Runnable>*/ pumpMessageDisplayMap = new HashMap();
 
     private static boolean useMainThread = false;
 
@@ -124,6 +124,7 @@ public class MainThread implements EDTUtil {
             this.mainClassArgs=mainClassArgs;
         }
 
+        @Override
         public void run() {
             if ( useMainThread ) {
                 // we have to start first to provide the service ..
@@ -205,7 +206,7 @@ public class MainThread implements EDTUtil {
         }
     }
 
-    public static final MainThread getSingleton() {
+    public static MainThread getSingleton() {
         return singletonMainThread;
     }
 
@@ -219,24 +220,20 @@ public class MainThread implements EDTUtil {
         if ( useMainThread ) {
             return; // error ?
         }
-        if(null == pumpMessagesTimer) {
-            synchronized (MainThread.class) {
-                if(null == pumpMessagesTimer) {
-                    pumpMessagesTimer = new Timer();
-                    pumpMessagesTimerTask = new TimerTask() {
-                        public void run() {
-                            synchronized(pumpMessageDisplayMap) {
-                                for(Iterator i = pumpMessageDisplayMap.values().iterator(); i.hasNext(); ) {
-                                    ((Runnable) i.next()).run();
-                                }
+        synchronized (pumpMessageDisplayMap) {
+            if(null == pumpMessagesTimer) {
+                pumpMessagesTimer = new Timer();
+                pumpMessagesTimerTask = new TimerTask() {
+                    public void run() {
+                        synchronized(pumpMessageDisplayMap) {
+                            for(Iterator i = pumpMessageDisplayMap.values().iterator(); i.hasNext(); ) {
+                                ((Runnable) i.next()).run();
                             }
                         }
-                    };
-                    pumpMessagesTimer.scheduleAtFixedRate(pumpMessagesTimerTask, 0, defaultEDTPollGranularity);
-                }
+                    }
+                };
+                pumpMessagesTimer.scheduleAtFixedRate(pumpMessagesTimerTask, 0, defaultEDTPollGranularity);
             }
-        }
-        synchronized(pumpMessageDisplayMap) {
             pumpMessageDisplayMap.put(dpy, pumpMessage);
         }
     }
diff --git a/src/newt/classes/jogamp/newt/DefaultEDTUtil.java b/src/newt/classes/jogamp/newt/DefaultEDTUtil.java
index 016906581..3b14f30fc 100644
--- a/src/newt/classes/jogamp/newt/DefaultEDTUtil.java
+++ b/src/newt/classes/jogamp/newt/DefaultEDTUtil.java
@@ -54,7 +54,7 @@ public class DefaultEDTUtil implements EDTUtil {
 
     public DefaultEDTUtil(ThreadGroup tg, String name, Runnable dispatchMessages) {
         this.threadGroup = tg;
-        this.name=new String(Thread.currentThread().getName()+"-"+name+"-EDT-");
+        this.name=Thread.currentThread().getName()+"-"+name+"-EDT-";
         this.dispatchMessages=dispatchMessages;
         this.edt = new EventDispatchThread(threadGroup, name);
         this.edt.setDaemon(true); // don't stop JVM from shutdown ..
@@ -113,7 +113,7 @@ public class DefaultEDTUtil implements EDTUtil {
         invokeImpl(wait, task, false);
     }
 
-    private final void invokeImpl(boolean wait, Runnable task, boolean stop) {
+    private void invokeImpl(boolean wait, Runnable task, boolean stop) {
         if(task == null) {
             throw new RuntimeException("Null Runnable");
         }
@@ -192,30 +192,33 @@ public class DefaultEDTUtil implements EDTUtil {
     }
 
     final public void waitUntilIdle() {
-        if(edt.isRunning() && edt != Thread.currentThread()) {
-            synchronized(edt.tasks) {
-                while(edt.isRunning() && edt.tasks.size()>0) {
-                    try {
-                        edt.tasks.notifyAll();
-                        edt.tasks.wait();
-                    } catch (InterruptedException e) {
-                        e.printStackTrace();
-                    }
+        final EventDispatchThread _edt;
+        synchronized(edtLock) {
+            _edt = edt;
+        }
+        if(!_edt.isRunning() || _edt == Thread.currentThread()) {
+            return;
+        }
+        synchronized(_edt.tasks) {
+            while(_edt.isRunning() && _edt.tasks.size()>0) {
+                try {
+                    _edt.tasks.notifyAll();
+                    _edt.tasks.wait();
+                } catch (InterruptedException e) {
+                    e.printStackTrace();
                 }
             }
         }
     }
 
     final public void waitUntilStopped() {
-        if(edt.isRunning() && edt != Thread.currentThread() ) {
-            synchronized(edtLock) {
-                if(edt.isRunning() && edt != Thread.currentThread() ) {
-                    while(edt.isRunning()) {
-                        try {
-                            edtLock.wait();
-                        } catch (InterruptedException e) {
-                            e.printStackTrace();
-                        }
+        synchronized(edtLock) {
+            if(edt.isRunning() && edt != Thread.currentThread() ) {
+                while(edt.isRunning()) {
+                    try {
+                        edtLock.wait();
+                    } catch (InterruptedException e) {
+                        e.printStackTrace();
                     }
                 }
             }
@@ -235,6 +238,7 @@ public class DefaultEDTUtil implements EDTUtil {
             return isRunning;
         }
 
+        @Override
         final public void start() throws IllegalThreadStateException {
             isRunning = true;
             super.start();
@@ -244,6 +248,7 @@ public class DefaultEDTUtil implements EDTUtil {
          * Utilizing locking only on tasks and its execution,
          * not for event dispatching.
          */
+        @Override
         final public void run() {
             if(DEBUG) {
                 System.err.println(getName()+": EDT run() START "+ getName());