From 76444dce2b678a7f6769564abac4f8a73f414609 Mon Sep 17 00:00:00 2001 From: Sven Gothel Date: Sat, 26 Feb 2011 21:43:20 +0100 Subject: Clean/Fix: Threading Code - Remove unsafe double checked locking - Annotate safe double checked locking (volatile) - use 'static final' if possible --- .../classes/com/jogamp/opengl/JoglVersion.java | 2 +- src/jogl/classes/jogamp/opengl/GLWorkerThread.java | 7 ++-- src/jogl/classes/jogamp/opengl/ThreadingImpl.java | 8 +--- .../jogamp/opengl/awt/AWTThreadingPlugin.java | 8 +--- .../jogamp/nativewindow/NativeWindowVersion.java | 2 +- .../nativewindow/awt/AWTWindowClosingProtocol.java | 5 ++- .../classes/jogamp/nativewindow/jawt/JAWTUtil.java | 46 +++++++++++++++------- .../classes/jogamp/nativewindow/x11/X11Util.java | 8 ++-- src/newt/classes/com/jogamp/newt/NewtVersion.java | 2 +- .../classes/com/jogamp/newt/util/MainThread.java | 37 ++++++++--------- src/newt/classes/jogamp/newt/DefaultEDTUtil.java | 45 +++++++++++---------- 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.
+ * 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.
*/ 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/**/ pumpMessageDisplayMap = new HashMap(); + private static final Map/**/ 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()); -- cgit v1.2.3