Fixed Issue 109: Two animator threads on two Frames crash

Fixed Issue 124: SIGSEGV on Fedora Core 3 / NVIDIA

Stability problems have been reported on all three of JOGL's major
supported platforms (Windows, Linux and Mac OS X) whose root cause is
multithreading-related bugs in vendors' OpenGL drivers.

On Windows, the most recent version of NVidia's drivers (66.93) when
run on a GeForce 6800 causes a blue screen of death upon exit from the
simplest JOGL demo (Gears).

On Linux, JVM crashes have been reported upon exit of certain test
cases on NVidia hardware. The same test case causes a report of an
unexpected async reply from Xlib on ATI hardware (at the last time of
testing -- I no longer have access to a Linux machine with ATI
hardware because all of their currently available drivers crash the X
server on my machine).

On Mac OS X, low-level warnings from Cocoa are printed in
multithreaded JOGL situations which can lead to JVM crashes.

All of these problems' root cause is that the current OpenGL drivers
on the market, or supporting software built on them, were not designed
to be used in a multithreaded fashion. Most C programs which use
OpenGL are either single-threaded or perform all of their OpenGL work
from a single thread. JOGL was originally designed to support OpenGL
rendering from arbitrary threads and appropriate synchronization was
introduced into the library to handle this. However, we have had to
scale back this support as stability issues have been encountered.

The most recent round of reported bugs, including the PC crash upon
exit of the JOGL demos, is serious enough that we must take drastic
measures. Support was introduced in earlier JOGL releases to move all
of the OpenGL work performed by JOGL and users' code via the
GLEventListener on to the AWT event dispatch thread. It turns out that
doing this works around all of the above reported bugs. In JOGL 1.1
b08 and 1.1 b09, code changes were made to make this single-threaded
support more correct; this checkin includes another small set of such
changes, including some to the GLPbuffer implementation, and changes
the default of the flag controlling this support to true.

-Djogl.1thread=true is now the default. -Djogl.1thread=auto restores
the behavior of previous releases, which was to enable the
single-threaded mode only with ATI cards. This auto-detection
mechanism was not robust enough and adding cases for the crashes above
was not feasible. -Djogl.1thread=false disables the single-threaded
workaround. Older synonyms for this system property,
JOGL_SINGLE_THREADED_WORKAROUND and ATI_WORKAROUND, remain in the
source base for the time being. Changing the value of any of these
system properties is not recommended.

The expected performance impact of these changes is minimal. In
earlier JOGL releases it appeared that the overhead of making a
context current and releasing it each frame was very significant and a
key differentiator in being able to match C performance. More recent
tests seem to indicate that this is no longer the case, at least with
current hardware. Regardless, we must achieve stability in order for
the library to be useful and this seems to be the best means of
achieving that goal.

We believe that the compatibility impact of these changes for existing
JOGL applications will also be minimal. For correctly-written JOGL
applications, the only visible change in behavior should be that the
values of thread-local variables accessed through the
java.lang.ThreadLocal class may change since the actual thread on
which the GLEventListener's callbacks will be executed may have
changed. Multithreaded JOGL applications performing complex
inter-thread synchronization may see subtle differences in behavior.
We hope that such applications and the developers writing them will be
able to handle this change in behavior without much trouble.

We will continue to work with graphics card vendors to improve the
stability of their OpenGL drivers. Until that happens, we believe this
change will yield the best possible improvement in stability and
portability for applications using JOGL.


git-svn-id: file:///usr/local/projects/SUN/JOGL/git-svn/svn-server-sync/jogl/trunk@241 232f8b59-042b-4e1e-8c03-345bb8c30851

3 files changed, 78 insertions, 15 deletions

diff --git a/src/net/java/games/jogl/GLCanvas.java b/src/net/java/games/jogl/GLCanvas.java
index 827184dac..17aed5bea 100644
--- a/src/net/java/games/jogl/GLCanvas.java
+++ b/src/net/java/games/jogl/GLCanvas.java
@@ -72,9 +72,9 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   }
   
   public void display() {
-    withSingleThreadedWorkaroundDo(displayOnEventDispatchThreadAction,
-                                   displayAction,
-                                   false);
+    maybeDoSingleThreadedWorkaround(displayOnEventDispatchThreadAction,
+                                    displayAction,
+                                    false);
   }
 
   /** Overridden from Canvas; calls {@link #display}. Should not be
@@ -122,7 +122,7 @@ public final class GLCanvas extends Canvas implements GLDrawable {
           context.invokeGL(reshapeRunnable, true, initAction);
         }
       };
-    withSingleThreadedWorkaroundDo(reshapeOnEDTRunnable, reshapeRunnable, true);
+    maybeDoSingleThreadedWorkaround(reshapeOnEDTRunnable, reshapeRunnable, true);
   }
 
   /** Overridden from Canvas to prevent Java2D's clearing of the
@@ -184,7 +184,7 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   }
 
   public void swapBuffers() {
-    withSingleThreadedWorkaroundDo(swapBuffersOnEventDispatchThreadAction, swapBuffersAction, false);
+    maybeDoSingleThreadedWorkaround(swapBuffersOnEventDispatchThreadAction, swapBuffersAction, false);
   }
 
   public boolean canCreateOffscreenDrawable() {
@@ -205,9 +205,9 @@ public final class GLCanvas extends Canvas implements GLDrawable {
   // Internals only below this point
   //
 
-  private void withSingleThreadedWorkaroundDo(Runnable eventDispatchThreadAction,
-                                              Runnable invokeGLAction,
-                                              boolean  isReshape) {
+  private void maybeDoSingleThreadedWorkaround(Runnable eventDispatchThreadAction,
+                                               Runnable invokeGLAction,
+                                               boolean  isReshape) {
     if (SingleThreadedWorkaround.doWorkaround() && !EventQueue.isDispatchThread()) {
       try {
         // Reshape events must not block on the event queue due to the
diff --git a/src/net/java/games/jogl/impl/GLPbufferImpl.java b/src/net/java/games/jogl/impl/GLPbufferImpl.java
index 03d161fdd..be0f23398 100644
--- a/src/net/java/games/jogl/impl/GLPbufferImpl.java
+++ b/src/net/java/games/jogl/impl/GLPbufferImpl.java
@@ -40,6 +40,7 @@
 package net.java.games.jogl.impl;
 
 import java.awt.Dimension;
+import java.awt.EventQueue;
 import java.awt.event.*;
 import java.beans.PropertyChangeListener;
 
@@ -61,7 +62,9 @@ public class GLPbufferImpl implements GLPbuffer {
   }
 
   public void display() {
-    context.invokeGL(displayAction, false, initAction);
+    maybeDoSingleThreadedWorkaround(displayOnEventDispatchThreadAction,
+                                    displayAction,
+                                    false);
   }
 
   public void setSize(int width, int height) {
@@ -107,7 +110,7 @@ public class GLPbufferImpl implements GLPbuffer {
   }
   
   void willSetRenderingThread() {
-    context.willSetRenderingThread();
+    // Not supported for pbuffers
   }
 
   public void setRenderingThread(Thread currentThreadOrNull) throws GLException {
@@ -135,7 +138,7 @@ public class GLPbufferImpl implements GLPbuffer {
   }
 
   public void swapBuffers() {
-    context.invokeGL(swapBuffersAction, false, initAction);
+    maybeDoSingleThreadedWorkaround(swapBuffersOnEventDispatchThreadAction, swapBuffersAction, false);
   }
 
   public boolean canCreateOffscreenDrawable() {
@@ -149,10 +152,14 @@ public class GLPbufferImpl implements GLPbuffer {
   }
 
   public void bindTexture() {
+    // Doesn't make much sense to try to do this on the event dispatch
+    // thread given that it has to be called while the context is current
     context.bindPbufferToTexture();
   }
 
   public void releaseTexture() {
+    // Doesn't make much sense to try to do this on the event dispatch
+    // thread given that it has to be called while the context is current
     context.releasePbufferFromTexture();
   }
 
@@ -204,6 +211,30 @@ public class GLPbufferImpl implements GLPbuffer {
   // Internals only below this point
   //
 
+  private void maybeDoSingleThreadedWorkaround(Runnable eventDispatchThreadAction,
+                                               Runnable invokeGLAction,
+                                               boolean  isReshape) {
+    if (SingleThreadedWorkaround.doWorkaround() && !EventQueue.isDispatchThread()) {
+      try {
+        // Reshape events must not block on the event queue due to the
+        // possibility of deadlocks during initial component creation.
+        // This solution is not optimal, because it changes the
+        // semantics of reshape() to have some of the processing being
+        // done asynchronously, but at least it preserves the
+        // semantics of the single-threaded workaround.
+        if (!isReshape) {
+          EventQueue.invokeAndWait(eventDispatchThreadAction);
+        } else {
+          EventQueue.invokeLater(eventDispatchThreadAction);
+        }
+      } catch (Exception e) {
+        throw new GLException(e);
+      }
+    } else {
+      context.invokeGL(invokeGLAction, isReshape, initAction);
+    }
+  }
+
   class InitAction implements Runnable {
     public void run() {
       drawableHelper.init(GLPbufferImpl.this);
@@ -225,4 +256,22 @@ public class GLPbufferImpl implements GLPbuffer {
     }
   }
   private SwapBuffersAction swapBuffersAction = new SwapBuffersAction();
+
+  // Workaround for ATI driver bugs related to multithreading issues
+  // like simultaneous rendering via Animators to canvases that are
+  // being resized on the AWT event dispatch thread
+  class DisplayOnEventDispatchThreadAction implements Runnable {
+    public void run() {
+      context.invokeGL(displayAction, false, initAction);
+    }
+  }
+  private DisplayOnEventDispatchThreadAction displayOnEventDispatchThreadAction =
+    new DisplayOnEventDispatchThreadAction();
+  class SwapBuffersOnEventDispatchThreadAction implements Runnable {
+    public void run() {
+      context.invokeGL(swapBuffersAction, false, initAction);
+    }
+  }
+  private SwapBuffersOnEventDispatchThreadAction swapBuffersOnEventDispatchThreadAction =
+    new SwapBuffersOnEventDispatchThreadAction();
 }
diff --git a/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java b/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java
index eb7d8f09d..7b9fa46c4 100755
--- a/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java
+++ b/src/net/java/games/jogl/impl/SingleThreadedWorkaround.java
@@ -45,10 +45,20 @@ import java.security.PrivilegedAction;
 /** Encapsulates the workaround of running all display operations on
     the AWT event queue thread for the purposes of working around
     problems seen primarily on ATI cards when rendering into a surface
-    that is simultaneously being resized by the event queue thread */
+    that is simultaneously being resized by the event queue thread.
+    <p>
+
+    As of JOGL 1.1 b10, this property defaults to true. Problems have
+    been seen on Windows, Linux and Mac OS X platforms that are solved
+    by switching all OpenGL work to a single thread, which this
+    workaround provides. The forthcoming JSR-231 work will rethink how
+    such a mechanism is implemented, but the core result of needing to
+    perform all OpenGL work on a single thread for best compatibility
+    will remain.
+*/
 
 public class SingleThreadedWorkaround {
-  private static boolean singleThreadedWorkaround = false;
+  private static boolean singleThreadedWorkaround = true;
   // If the user specified the workaround's system property (either
   // true or false), don't let the automatic detection have any effect
   private static boolean systemPropertySpecified = false;
@@ -56,12 +66,16 @@ public class SingleThreadedWorkaround {
   static {
     AccessController.doPrivileged(new PrivilegedAction() {
         public Object run() {
-          String workaround = System.getProperty("JOGL_SINGLE_THREADED_WORKAROUND");
+          String workaround = System.getProperty("jogl.1thread");
           if (workaround == null) {
             // Old system property (for compatibility)
+            workaround = System.getProperty("JOGL_SINGLE_THREADED_WORKAROUND");
+          }
+          if (workaround == null) {
+            // Older system property (for compatibility)
             workaround = System.getProperty("ATI_WORKAROUND");
           }
-          if (workaround != null) {
+          if (workaround != null && (!workaround.equals("auto"))) {
             systemPropertySpecified = true;
             singleThreadedWorkaround = Boolean.valueOf(workaround).booleanValue();
           }