Bug 877 Concurrency Discussion: Update doc MultiThreading.txt, volatile field usage; GLDrawableImpl: Make read-only fields final.

7 files changed, 43 insertions, 8 deletions

diff --git a/doc/Implementation/MultiThreading.txt b/doc/Implementation/MultiThreading.txt
index fb586e1ec..272204d94 100644
--- a/doc/Implementation/MultiThreading.txt
+++ b/doc/Implementation/MultiThreading.txt
@@ -1,4 +1,9 @@
 
+Don't miss 
+    Current 'violations' of synchronization/memory-barriers
+    using explicit locking:
+below ..
+
 Locking
 =========
 
@@ -92,3 +97,29 @@ Summary:
         - swapBuffersImpl
             - [Surface i/o] - if not locked already
 
++++
+
+Complying with synchronization/memory-barriers
+using explicit locking:
+
+- NativeSurface's handle access
+
+Current 'violations' of synchronization/memory-barriers
+using explicit locking:
+
+- Using 'volatile' to avoid locking for read-only access
+  with rare write access
+
+    - GLAutoDrawable's drawable reference
+
+    - GLAutoDrawable's context reference
+        Since 7f7a23dd0ddf106e6f0c69fc2a05ff92ac56200e
+
+    - GLContext's contextHandle
+        Since 7f7a23dd0ddf106e6f0c69fc2a05ff92ac56200e
+
+    - GLDrawableImpl's realized
+        Used for Pre-locking queries, whether drawable is realized
+
+- Misc 'volatile' usage:
+
diff --git a/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java b/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java
index 1268e9569..cad780a89 100644
--- a/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java
+++ b/src/jogl/classes/com/jogamp/opengl/swt/GLCanvas.java
@@ -109,7 +109,7 @@ public class GLCanvas extends Canvas implements GLAutoDrawable, GLSharedContextS
 
    private volatile Rectangle clientArea;
    private volatile GLDrawableImpl drawable; // volatile: avoid locking for read-only access
-   private volatile GLContextImpl context;
+   private volatile GLContextImpl context; // volatile: avoid locking for read-only access
 
    /* Native window surface */
    private final boolean useX11GTK;
diff --git a/src/jogl/classes/javax/media/opengl/GLContext.java b/src/jogl/classes/javax/media/opengl/GLContext.java
index 5c6c7073a..deb1d7587 100644
--- a/src/jogl/classes/javax/media/opengl/GLContext.java
+++ b/src/jogl/classes/javax/media/opengl/GLContext.java
@@ -207,7 +207,7 @@ public abstract class GLContext {
   protected final RecursiveLock lock = LockFactory.createRecursiveLock();
 
   /** The underlying native OpenGL context */
-  protected volatile long contextHandle;
+  protected volatile long contextHandle; // volatile: avoid locking for read-only access
 
   protected GLContext() {
       resetStates(true);
diff --git a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
index 439a5bd30..01d6a6738 100644
--- a/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
+++ b/src/jogl/classes/javax/media/opengl/awt/GLCanvas.java
@@ -163,7 +163,7 @@ public class GLCanvas extends Canvas implements AWTGLAutoDrawable, WindowClosing
   private AWTGraphicsConfiguration awtConfig;
   private volatile GLDrawableImpl drawable; // volatile: avoid locking for read-only access
   private volatile JAWTWindow jawtWindow; // the JAWTWindow presentation of this AWT Canvas, bound to the 'drawable' lifecycle
-  private volatile GLContextImpl context;
+  private volatile GLContextImpl context; // volatile: avoid locking for read-only access
   private volatile boolean sendReshape = false; // volatile: maybe written by EDT w/o locking
 
   // copy of the cstr args, mainly for recreation
diff --git a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
index 8b1467268..fdacc5bc4 100644
--- a/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
+++ b/src/jogl/classes/javax/media/opengl/awt/GLJPanel.java
@@ -1295,12 +1295,12 @@ public class GLJPanel extends JPanel implements AWTGLAutoDrawable, WindowClosing
     protected IntBuffer readBackIntsForCPUVFlip;
 
     // Implementation using software rendering
-    private volatile GLDrawableImpl offscreenDrawable;
+    private volatile GLDrawableImpl offscreenDrawable; // volatile: avoid locking for read-only access
     private boolean offscreenIsFBO;
     private FBObject fboFlipped;
     private GLSLTextureRaster glslTextureRaster;
 
-    private volatile GLContextImpl offscreenContext;
+    private volatile GLContextImpl offscreenContext; // volatile: avoid locking for read-only access
     private boolean flipVertical;
 
     // For saving/restoring of OpenGL state during ReadPixels
diff --git a/src/jogl/classes/jogamp/opengl/GLContextImpl.java b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
index 4e5465906..a15352c52 100644
--- a/src/jogl/classes/jogamp/opengl/GLContextImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
@@ -650,6 +650,10 @@ public abstract class GLContextImpl extends GLContext {
         final GLContextImpl shareWith = (GLContextImpl) GLContextShareSet.getCreatedShare(this);
         if (null != shareWith) {
             shareWith.getDrawableImpl().lockSurface();
+            // FIXME:
+            // Contemplate whether we shall 'fail' creating this context
+            // if 0==shareWith.getHandle(), since at this point
+            // both context are locked and current values are available.
         }
         final boolean created;
         try {
diff --git a/src/jogl/classes/jogamp/opengl/GLDrawableImpl.java b/src/jogl/classes/jogamp/opengl/GLDrawableImpl.java
index a79a3cf25..94d39a4ab 100644
--- a/src/jogl/classes/jogamp/opengl/GLDrawableImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLDrawableImpl.java
@@ -344,9 +344,9 @@ public abstract class GLDrawableImpl implements GLDrawable {
 
   protected static String getThreadName() { return Thread.currentThread().getName(); }
 
-  protected GLDrawableFactory factory;
-  protected NativeSurface surface;
-  protected GLCapabilitiesImmutable requestedCapabilities;
+  protected final GLDrawableFactory factory;
+  protected final NativeSurface surface;
+  protected final GLCapabilitiesImmutable requestedCapabilities;
 
   // Indicates whether the surface (if an onscreen context) has been
   // realized. Plausibly, before the surface is realized the JAWT