Fix: Avoid list modification while iterator in use (ConcurrentModificationException) - 2 - Use volatile barrier

2 files changed, 134 insertions, 19 deletions

diff --git a/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java b/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
index e8aee977d..53fc34181 100644
--- a/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
+++ b/src/jogl/classes/com/jogamp/opengl/impl/GLDrawableHelper.java
@@ -51,7 +51,7 @@ public class GLDrawableHelper {
   private static final boolean VERBOSE = Debug.verbose();
   private Object listenersLock = new Object();
   private List listeners = new ArrayList();
-  private Iterator listenersIter = null; // avoid java.util.ConcurrentModificationException
+  private volatile boolean listenersIter = false; // avoid java.util.ConcurrentModificationException
   private Set listenersToBeInit = new HashSet();
   private boolean autoSwapBufferMode = true;
   private Object glRunnablesLock = new Object();
@@ -66,14 +66,15 @@ public class GLDrawableHelper {
     sb.append("GLAnimatorControl: "+animatorCtrl+", ");
     synchronized(listenersLock) {
         sb.append("GLEventListeners num "+listeners.size()+" [");
-        for (listenersIter = listeners.iterator(); listenersIter.hasNext(); ) {
-          Object l = listenersIter.next();
+        listenersIter = true;
+        for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
+          Object l = iter.next();
           sb.append(l);
           sb.append("[init ");
           sb.append( !listenersToBeInit.contains(l) );
           sb.append("], ");
         }
-        listenersIter = null;
+        listenersIter = false;
     }
     sb.append("]");
     return sb.toString();
@@ -89,7 +90,7 @@ public class GLDrawableHelper {
             index = listeners.size();
         }
         listenersToBeInit.add(listener);
-        if(null == listenersIter) {
+        if(!listenersIter) {
             // fast path
             listeners.add(index, listener);
         } else {
@@ -103,7 +104,7 @@ public class GLDrawableHelper {
   
   public void removeGLEventListener(GLEventListener listener) {
     synchronized(listenersLock) {
-        if(null == listenersIter) {
+        if(!listenersIter) {
             // fast path
             listeners.remove(listener);
         } else {
@@ -118,12 +119,13 @@ public class GLDrawableHelper {
 
   public void dispose(GLAutoDrawable drawable) {
     synchronized(listenersLock) {
-        for (listenersIter = listeners.iterator(); listenersIter.hasNext(); ) {
-          GLEventListener listener = (GLEventListener) listenersIter.next() ;
+        listenersIter = true;
+        for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
+          GLEventListener listener = (GLEventListener) iter.next() ;
           listener.dispose(drawable);
           listenersToBeInit.add(listener);
         }
-        listenersIter = null;
+        listenersIter = false;
     }
   }
 
@@ -140,26 +142,28 @@ public class GLDrawableHelper {
 
   public void init(GLAutoDrawable drawable) {
     synchronized(listenersLock) {
-        for (listenersIter = listeners.iterator(); listenersIter.hasNext(); ) {
-          GLEventListener listener = (GLEventListener) listenersIter.next() ;
+        listenersIter = true;
+        for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
+          GLEventListener listener = (GLEventListener) iter.next() ;
           if ( ! init( listener, drawable, false ) ) {
             throw new GLException("GLEventListener "+listener+" already initialized: "+drawable);
           }
         }
-        listenersIter = null;
+        listenersIter = false;
     }
   }
 
   public void display(GLAutoDrawable drawable) {
     synchronized(listenersLock) {
-        for (listenersIter = listeners.iterator(); listenersIter.hasNext(); ) {
-          GLEventListener listener = (GLEventListener) listenersIter.next() ;
+        listenersIter = true;
+        for (Iterator iter = listeners.iterator(); iter.hasNext(); ) {
+          GLEventListener listener = (GLEventListener) iter.next() ;
           // GLEventListener may need to be init, 
           // in case this one is added after the realization of the GLAutoDrawable
           init( listener, drawable, true ) ; 
           listener.display(drawable);
         }
-        listenersIter = null;
+        listenersIter = false;
     }
     execGLRunnables(drawable);
   }
@@ -174,11 +178,12 @@ public class GLDrawableHelper {
 
   public void reshape(GLAutoDrawable drawable, int x, int y, int width, int height) {
     synchronized(listenersLock) {
+        listenersIter = true;
         int i=0;
-        for (listenersIter = listeners.iterator(); listenersIter.hasNext(); i++) {
-          reshape((GLEventListener) listenersIter.next(), drawable, x, y, width, height, 0==i);
+        for (Iterator iter = listeners.iterator(); iter.hasNext(); i++) {
+          reshape((GLEventListener) iter.next(), drawable, x, y, width, height, 0==i);
         }
-        listenersIter = null;
+        listenersIter = false;
     }
   }
 
@@ -277,7 +282,7 @@ public class GLDrawableHelper {
                        Runnable  initAction) {
     if(null==context) {
         if (DEBUG) {
-            Exception e = new GLException(Thread.currentThread().getName()+" GLDrawableHelper " + this + ".invokeGL(): NULL GLContext");
+            Exception e = new GLException(Thread.currentThread().getName()+"Info: GLDrawableHelper " + this + ".invokeGL(): NULL GLContext");
             e.printStackTrace();
         }
         return;
diff --git a/src/junit/com/jogamp/test/junit/newt/TestEventSourceNotNEWTBug.java b/src/junit/com/jogamp/test/junit/newt/TestEventSourceNotNEWTBug.java
new file mode 100644
index 000000000..7e596eb8c
--- /dev/null
+++ b/src/junit/com/jogamp/test/junit/newt/TestEventSourceNotNEWTBug.java
@@ -0,0 +1,110 @@
+/**
+ * Copyright 2010 JogAmp Community. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification, are
+ * permitted provided that the following conditions are met:
+ * 
+ *    1. Redistributions of source code must retain the above copyright notice, this list of
+ *       conditions and the following disclaimer.
+ * 
+ *    2. Redistributions in binary form must reproduce the above copyright notice, this list
+ *       of conditions and the following disclaimer in the documentation and/or other materials
+ *       provided with the distribution.
+ * 
+ * THIS SOFTWARE IS PROVIDED BY JogAmp Community ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
+ * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL JogAmp Community OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ * 
+ * The views and conclusions contained in the software and documentation are those of the
+ * authors and should not be interpreted as representing official policies, either expressed
+ * or implied, of JogAmp Community.
+ */
+ 
+package com.jogamp.test.junit.newt;
+
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.After;
+import org.junit.AfterClass;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import javax.media.opengl.GLCapabilities;
+import javax.media.opengl.GLProfile;
+import javax.swing.JFrame;
+import javax.swing.SwingUtilities;
+import javax.swing.WindowConstants;
+
+import com.jogamp.newt.awt.NewtCanvasAWT;
+import com.jogamp.newt.opengl.GLWindow;
+import com.jogamp.test.junit.jogl.demos.gl2.gears.Gears;
+import com.jogamp.test.junit.util.*;
+
+/**
+ * This simple program will throw a {@link RuntimeException} when the application is closed.
+ */
+public class TestEventSourceNotNEWTBug extends UITestCase {
+
+    @BeforeClass
+    public static void initClass() {
+        GLProfile.initSingleton(true);
+    }
+
+    @Test
+    public void testEventSourceNotNewtBug() throws InterruptedException {
+        JFrame jf = new JFrame();
+
+        jf.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
+
+        final GLCapabilities caps = new GLCapabilities(GLProfile.getDefault());
+        final GLWindow glWindow = GLWindow.create(caps);
+        final NewtCanvasAWT canvas = new NewtCanvasAWT(glWindow);
+        jf.getContentPane().add(canvas);
+
+        // The following line isn't event necessary to see the problem.
+        glWindow.addGLEventListener(new Gears());
+
+        final JFrame f_jf = jf;
+
+        SwingUtilities.invokeLater(new Runnable() {
+            public void run() {
+                f_jf.setSize(800, 600);
+                f_jf.setVisible(true);
+            }
+        });
+
+        Thread.sleep(500);
+
+        SwingUtilities.invokeLater(new Runnable() {
+            public void run() {
+                f_jf.dispose();
+            }
+        });
+        glWindow.destroy(true);
+    }
+
+    public static void main(String args[]) throws IOException {
+        String tstname = TestEventSourceNotNEWTBug.class.getName();
+        /*
+        org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(new String[] {
+            tstname,
+            "filtertrace=true",
+            "haltOnError=false",
+            "haltOnFailure=false",
+            "showoutput=true",
+            "outputtoformatters=true",
+            "logfailedtests=true",
+            "logtestlistenerevents=true",
+            "formatter=org.apache.tools.ant.taskdefs.optional.junit.PlainJUnitResultFormatter",
+            "formatter=org.apache.tools.ant.taskdefs.optional.junit.XMLJUnitResultFormatter,TEST-"+tstname+".xml" } ); */
+        org.junit.runner.JUnitCore.main(tstname);
+    }
+}