Fix Bug 527: Creating a context w/ shared context, while the latter is in use (threading)

Bug 527 describes the situation where wglShareLists() fails (throws exception)
while the shared context is in use by another thread (some Animator).

It was reported by Jerome Jouvie.

The exception happens not everytime, but at least around 20% on manual tests
I have performed on the Windows platform.

The context in question are all JOGL's GL2, which natively where bound to an OpenGL 3.0 profile.
The WGL_ARB_create_context spec says http://www.opengl.org/registry/specs/ARB/wgl_create_context.txt:
+++
    Future versions of OpenGL may only support being added to a share
    group at context creation time. Specifying such a version of a
    context as either the <hglrc1> or <hglrc2> arguments to
    wglShareLists will fail. wglShareLists will return FALSE, and
    GetLastError will return ERROR_INCOMPATIBLE_DEVICE_CONTEXTS_ARB.
+++
Hence the 1st patch was to remove 'wglShareLists()' usage in case
we use the new WGL_ARB_create_context context creation method.
Even though this is a desired change, and works in general, it didn't fix the issue.

It seems that the shared context, which is passed @ new context creation,
cannot be used while it is in use itself in another thread.

This conclusion leads to the actual fix, ie. locking the shared context
while creating the new context which shares it.
Manual tests using this patch could not reproduce this issue (40 attempts).

Test: TestSharedContextListNEWT2

9 files changed, 68 insertions, 55 deletions

diff --git a/src/jogl/classes/jogamp/opengl/GLContextImpl.java b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
index fd34d8ac4..9611a1651 100644
--- a/src/jogl/classes/jogamp/opengl/GLContextImpl.java
+++ b/src/jogl/classes/jogamp/opengl/GLContextImpl.java
@@ -458,7 +458,17 @@ public abstract class GLContextImpl extends GLContext {
           if (!isCreated()) {
             GLProfile.initProfiles(
                     getGLDrawable().getNativeSurface().getGraphicsConfiguration().getScreen().getDevice());
-            newCreated = createImpl(); // may throws exception if fails!
+            final GLContextImpl shareWith = (GLContextImpl) GLContextShareSet.getShareContext(this);
+            if (null != shareWith) {
+                shareWith.getDrawableImpl().lockSurface();
+            }
+            try {
+                newCreated = createImpl(shareWith); // may throws exception if fails!
+            } finally {
+                if (null != shareWith) {
+                    shareWith.getDrawableImpl().unlockSurface();
+                }                
+            }
             if (DEBUG) {
                 if(newCreated) {
                     System.err.println(getThreadName() + ": !!! Create GL context OK: " + toHexString(contextHandle) + " for " + getClass().getName());
@@ -485,7 +495,7 @@ public abstract class GLContextImpl extends GLContext {
     }
   }
   protected abstract void makeCurrentImpl(boolean newCreatedContext) throws GLException;
-  protected abstract boolean createImpl() throws GLException ;
+  protected abstract boolean createImpl(GLContextImpl sharedWith) throws GLException ;
 
   /** 
    * Platform dependent but harmonized implementation of the <code>ARB_create_context</code>
diff --git a/src/jogl/classes/jogamp/opengl/egl/EGLContext.java b/src/jogl/classes/jogamp/opengl/egl/EGLContext.java
index 7c72e0d29..6af35e3aa 100644
--- a/src/jogl/classes/jogamp/opengl/egl/EGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/egl/EGLContext.java
@@ -35,17 +35,20 @@
 
 package jogamp.opengl.egl;
 
-import javax.media.opengl.*;
+import java.nio.ByteBuffer;
+import java.util.Map;
 
-import jogamp.opengl.*;
+import javax.media.nativewindow.AbstractGraphicsConfiguration;
+import javax.media.nativewindow.AbstractGraphicsDevice;
+import javax.media.opengl.GLContext;
+import javax.media.opengl.GLException;
+import javax.media.opengl.GLProfile;
+
+import jogamp.opengl.GLContextImpl;
+import jogamp.opengl.GLDrawableImpl;
 
 import com.jogamp.gluegen.runtime.ProcAddressTable;
 import com.jogamp.gluegen.runtime.opengl.GLProcAddressResolver;
-import java.nio.*;
-import java.util.*;
-
-import javax.media.nativewindow.AbstractGraphicsConfiguration;
-import javax.media.nativewindow.AbstractGraphicsDevice;
 
 public abstract class EGLContext extends GLContextImpl {
     private boolean eglQueryStringInitialized;
@@ -135,12 +138,12 @@ public abstract class EGLContext extends GLContextImpl {
         // FIXME
     }
 
-    protected boolean createImpl() throws GLException {
+    protected boolean createImpl(GLContextImpl shareWith) throws GLException {
         long eglDisplay = ((EGLDrawable)drawable).getDisplay();
         EGLGraphicsConfiguration config = ((EGLDrawable)drawable).getGraphicsConfiguration();
         GLProfile glProfile = drawable.getGLProfile();
         long eglConfig = config.getNativeConfig();
-        long shareWith = EGL.EGL_NO_CONTEXT;
+        long shareWithHandle = EGL.EGL_NO_CONTEXT;
 
         if (eglDisplay == 0) {
             throw new GLException("Error: attempted to create an OpenGL context without a display connection");
@@ -160,10 +163,9 @@ public abstract class EGLContext extends GLContextImpl {
             }
         }
 
-        EGLContext other = (EGLContext) GLContextShareSet.getShareContext(this);
-        if (other != null) {
-            shareWith = other.getHandle();
-            if (shareWith == 0) {
+        if (shareWith != null) {
+            shareWithHandle = shareWith.getHandle();
+            if (shareWithHandle == 0) {
                 throw new GLException("GLContextShareSet returned an invalid OpenGL context");
             }
         }
@@ -179,10 +181,10 @@ public abstract class EGLContext extends GLContextImpl {
         } else {
             throw new GLException("Error creating OpenGL context - invalid GLProfile: "+glProfile);
         }
-        contextHandle = EGL.eglCreateContext(eglDisplay, eglConfig, shareWith, contextAttrs, 0);
+        contextHandle = EGL.eglCreateContext(eglDisplay, eglConfig, shareWithHandle, contextAttrs, 0);
         if (contextHandle == 0) {
             throw new GLException("Error creating OpenGL context: eglDisplay "+toHexString(eglDisplay)+
-                                  ", eglConfig "+config+", "+glProfile+", shareWith "+toHexString(shareWith)+", error "+toHexString(EGL.eglGetError()));
+                                  ", eglConfig "+config+", "+glProfile+", shareWith "+toHexString(shareWithHandle)+", error "+toHexString(EGL.eglGetError()));
         }
         if (DEBUG) {
             System.err.println(getThreadName() + ": !!! Created OpenGL context 0x" +
@@ -190,7 +192,7 @@ public abstract class EGLContext extends GLContextImpl {
                                ",\n\twrite surface 0x" + Long.toHexString(drawable.getHandle()) +
                                ",\n\tread  surface 0x" + Long.toHexString(drawableRead.getHandle())+
                                ",\n\t"+this+
-                               ",\n\tsharing with 0x" + Long.toHexString(shareWith));
+                               ",\n\tsharing with 0x" + Long.toHexString(shareWithHandle));
         }
         if (!EGL.eglMakeCurrent(((EGLDrawable)drawable).getDisplay(),
                                 drawable.getHandle(),
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
index d5ce6a196..d063c3a7b 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXCGLContext.java
@@ -54,7 +54,6 @@ import javax.media.opengl.GLException;
 import javax.media.opengl.GLProfile;
 
 import jogamp.opengl.GLContextImpl;
-import jogamp.opengl.GLContextShareSet;
 import jogamp.opengl.GLDrawableImpl;
 import jogamp.opengl.GLGraphicsConfigurationUtil;
 import jogamp.opengl.macosx.cgl.MacOSXCGLDrawable.GLBackendType;
@@ -204,35 +203,35 @@ public abstract class MacOSXCGLContext extends GLContextImpl
     return false;
   }
 
-  protected long createImplPreset() throws GLException {
-    MacOSXCGLContext other = (MacOSXCGLContext) GLContextShareSet.getShareContext(this);
+  protected long createImplPreset(GLContextImpl shareWith) throws GLException {
     long share = 0;
-    if (other != null) {
+    if (shareWith != null) {
       // Change our OpenGL mode to match that of any share context before we create ourselves
-      setOpenGLMode(other.getOpenGLMode());
-      share = other.getHandle();
+      setOpenGLMode(((MacOSXCGLContext)shareWith).getOpenGLMode());
+      share = shareWith.getHandle();
       if (share == 0) {
         throw new GLException("GLContextShareSet returned a NULL OpenGL context");
       }
     }
+    
     MacOSXCGLGraphicsConfiguration config = (MacOSXCGLGraphicsConfiguration) drawable.getNativeSurface().getGraphicsConfiguration();
     GLCapabilitiesImmutable capabilitiesChosen = (GLCapabilitiesImmutable) config.getChosenCapabilities();
-    if (capabilitiesChosen.getPbufferFloatingPointBuffers() &&
-        !isTigerOrLater) {
+    if (capabilitiesChosen.getPbufferFloatingPointBuffers() && !isTigerOrLater) {
        throw new GLException("Floating-point pbuffers supported only on OS X 10.4 or later");
     }
     GLProfile glp = capabilitiesChosen.getGLProfile();
     if(glp.isGLES1() || glp.isGLES2() || glp.isGL4() || glp.isGL3() && !isLionOrLater) {
         throw new GLException("OpenGL profile not supported on MacOSX "+Platform.getOSVersionNumber()+": "+glp);
     }
+    
     if (DEBUG) {
       System.err.println("!!! Share context is " + toHexString(share) + " for " + this);
     }
-    return share;
+    return share;      
   }
-    
-  protected boolean createImpl() throws GLException {
-    long share = createImplPreset();
+  
+  protected boolean createImpl(GLContextImpl shareWith) throws GLException {
+    long share = createImplPreset(shareWith);
     contextHandle = createContextARB(share, true);
     return 0 != contextHandle;
   }
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java
index ba384fc71..2b24ed378 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXExternalCGLContext.java
@@ -50,6 +50,7 @@ import javax.media.opengl.GLDrawableFactory;
 import javax.media.opengl.GLException;
 
 import jogamp.nativewindow.WrappedSurface;
+import jogamp.opengl.GLContextImpl;
 import jogamp.opengl.GLContextShareSet;
 import jogamp.opengl.macosx.cgl.MacOSXCGLDrawable.GLBackendType;
 
@@ -117,7 +118,7 @@ public class MacOSXExternalCGLContext extends MacOSXCGLContext {
     return new MacOSXExternalCGLContext(new Drawable(factory, ns), isNSContext, contextHandle);
   }
 
-  protected boolean createImpl() throws GLException {
+  protected boolean createImpl(GLContextImpl shareWith) throws GLException {
       return true;
   }
 
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java
index 4fe6fa484..6de94085f 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/MacOSXOnscreenCGLContext.java
@@ -42,6 +42,8 @@ package jogamp.opengl.macosx.cgl;
 import javax.media.opengl.GLContext;
 import javax.media.opengl.GLException;
 
+import jogamp.opengl.GLContextImpl;
+
 public class MacOSXOnscreenCGLContext extends MacOSXCGLContext {
 
   public MacOSXOnscreenCGLContext(MacOSXOnscreenCGLDrawable drawable,
@@ -72,8 +74,8 @@ public class MacOSXOnscreenCGLContext extends MacOSXCGLContext {
   }
   
   @Override
-  protected boolean createImpl() {
-    boolean res = super.createImpl();
+  protected boolean createImpl(GLContextImpl sharedWith) {
+    boolean res = super.createImpl(sharedWith);
     lastWidth = -1; 
     lastHeight = -1;    
     if(res && isNSContext()) {
diff --git a/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java b/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java
index ae58f6811..a9376fb34 100644
--- a/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/macosx/cgl/awt/MacOSXJava2DCGLContext.java
@@ -44,6 +44,7 @@ import java.awt.Graphics;
 import javax.media.opengl.GLContext;
 import javax.media.opengl.GLException;
 
+import jogamp.opengl.GLContextImpl;
 import jogamp.opengl.awt.Java2D;
 import jogamp.opengl.awt.Java2DGLContext;
 import jogamp.opengl.macosx.cgl.MacOSXCGLContext;
@@ -79,8 +80,8 @@ public class MacOSXJava2DCGLContext extends MacOSXCGLContext implements Java2DGL
     }            
   }
 
-  protected boolean createImpl() {
-    long share = createImplPreset();
+  protected boolean createImpl(GLContextImpl shareWith) {
+    long share = createImplPreset(shareWith);
     
     long ctx = Java2D.createOGLContextOnSurface(graphics, share);
     if (ctx == 0) {
diff --git a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java
index 90b7fa949..300b5c5c7 100644
--- a/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java
+++ b/src/jogl/classes/jogamp/opengl/windows/wgl/WindowsWGLContext.java
@@ -55,7 +55,6 @@ import com.jogamp.gluegen.runtime.ProcAddressTable;
 import com.jogamp.gluegen.runtime.opengl.GLProcAddressResolver;
 import jogamp.nativewindow.windows.GDI;
 import jogamp.opengl.GLContextImpl;
-import jogamp.opengl.GLContextShareSet;
 import jogamp.opengl.GLDrawableImpl;
 
 public class WindowsWGLContext extends GLContextImpl {
@@ -257,20 +256,19 @@ public class WindowsWGLContext extends GLContextImpl {
    * Creates and initializes an appropriate OpenGL context. Should only be
    * called by {@link #makeCurrentImpl()}.
    */
-  protected boolean createImpl() {
-    WindowsWGLDrawableFactory factory = (WindowsWGLDrawableFactory)drawable.getFactoryImpl();
+  protected boolean createImpl(GLContextImpl shareWith) {
     AbstractGraphicsConfiguration config = drawable.getNativeSurface().getGraphicsConfiguration();
     AbstractGraphicsDevice device = config.getScreen().getDevice();
+    WindowsWGLDrawableFactory factory = (WindowsWGLDrawableFactory)drawable.getFactoryImpl();
     WindowsWGLContext sharedContext = (WindowsWGLContext) factory.getOrCreateSharedContextImpl(device);
     GLCapabilitiesImmutable glCaps = drawable.getChosenGLCapabilities();
 
     isGLReadDrawableAvailable(); // trigger setup wglGLReadDrawableAvailable
 
     // Windows can set up sharing of display lists after creation time
-    WindowsWGLContext other = (WindowsWGLContext) GLContextShareSet.getShareContext(this);
     long share = 0;
-    if (other != null) {
-      share = other.getHandle();
+    if (null != shareWith) {
+      share = shareWith.getHandle();
       if (share == 0) {
         throw new GLException("GLContextShareSet returned an invalid OpenGL context");
       }
@@ -347,17 +345,19 @@ public class WindowsWGLContext extends GLContextImpl {
             WGL.wglDeleteContext(contextHandle);
             throw new GLException("Error making old context current: 0x" + toHexString(contextHandle) + ", werr: " + GDI.GetLastError());
         }
+        if(0!=share) {
+            // Only utilize the classic GDI 'wglShareLists' shared context method 
+            // for traditional non ARB context.
+            if (!WGL.wglShareLists(share, contextHandle)) {
+                throw new GLException("wglShareLists(" + toHexString(share) +
+                                      ", " + toHexString(contextHandle) + ") failed: werr " + GDI.GetLastError());
+            }
+        }
         if (DEBUG) {
             System.err.println(getThreadName() + ": createImpl: OK (old) share "+share);
         }
     }
 
-    if(0!=share) {
-        if (!WGL.wglShareLists(share, contextHandle)) {
-            throw new GLException("wglShareLists(" + toHexString(share) +
-                                  ", " + toHexString(contextHandle) + ") failed: werr " + GDI.GetLastError());
-        }
-    }
     return true;
   }
   
diff --git a/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java b/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java
index e79c8b66c..70881a040 100644
--- a/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java
+++ b/src/jogl/classes/jogamp/opengl/x11/glx/X11ExternalGLXContext.java
@@ -95,7 +95,7 @@ public class X11ExternalGLXContext extends X11GLXContext {
     return new X11ExternalGLXContext(new Drawable(factory, ns), ctx);
   }
 
-  protected boolean createImpl() {
+  protected boolean createImpl(GLContextImpl shareWith) {
       return true;
   }
 
diff --git a/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java b/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java
index 6214c98e7..d3507cbaa 100644
--- a/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java
+++ b/src/jogl/classes/jogamp/opengl/x11/glx/X11GLXContext.java
@@ -56,7 +56,6 @@ import javax.media.opengl.GLProfile;
 import jogamp.nativewindow.x11.X11Lib;
 import jogamp.nativewindow.x11.X11Util;
 import jogamp.opengl.GLContextImpl;
-import jogamp.opengl.GLContextShareSet;
 import jogamp.opengl.GLDrawableImpl;
 
 import com.jogamp.common.nio.Buffers;
@@ -262,17 +261,17 @@ public abstract class X11GLXContext extends GLContextImpl {
     return ctx;
   }
 
-  protected boolean createImpl() {
+  protected boolean createImpl(GLContextImpl shareWith) {
       // covers the whole context creation loop incl createContextARBImpl and destroyContextARBImpl
       X11Util.setX11ErrorHandler(true, true);
       try {
-          return createImplRaw();
+          return createImplRaw(shareWith);
       } finally {
           X11Util.setX11ErrorHandler(false, false);
       }
   }
 
-  private boolean createImplRaw() {
+  private boolean createImplRaw(GLContextImpl shareWith) {
     boolean direct = true; // try direct always
     isDirect = false; // fall back
 
@@ -282,10 +281,9 @@ public abstract class X11GLXContext extends GLContextImpl {
     X11GLXContext sharedContext = (X11GLXContext) factory.getOrCreateSharedContextImpl(device);
     long display = device.getHandle();
 
-    X11GLXContext other = (X11GLXContext) GLContextShareSet.getShareContext(this);
     long share = 0;
-    if (other != null) {
-      share = other.getHandle();
+    if (shareWith != null) {
+      share = shareWith.getHandle();
       if (share == 0) {
         throw new GLException("GLContextShareSet returned an invalid OpenGL context");
       }