#7065 ConcurrentModificationException in AbstractWebApplicationContext.endTransaction() and startTransaction()

svn changeset:20545/svn branch:6.7

2 files changed, 95 insertions, 11 deletions

diff --git a/src/com/vaadin/terminal/gwt/server/AbstractWebApplicationContext.java b/src/com/vaadin/terminal/gwt/server/AbstractWebApplicationContext.java
index 02218f4bc2..752e4c4760 100644
--- a/src/com/vaadin/terminal/gwt/server/AbstractWebApplicationContext.java
+++ b/src/com/vaadin/terminal/gwt/server/AbstractWebApplicationContext.java
@@ -7,6 +7,7 @@ import java.io.PrintWriter;
 import java.io.Serializable;
 import java.io.StringWriter;
 import java.net.URL;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -60,10 +61,12 @@ public abstract class AbstractWebApplicationContext implements
      *            the HTTP or portlet request that triggered the transaction.
      */
     protected void startTransaction(Application application, Object request) {
+        ArrayList<TransactionListener> currentListeners;
         synchronized (listeners) {
-            for (TransactionListener listener : listeners) {
-                listener.transactionStart(application, request);
-            }
+            currentListeners = new ArrayList<TransactionListener>(listeners);
+        }
+        for (TransactionListener listener : currentListeners) {
+            listener.transactionStart(application, request);
         }
     }
 
@@ -78,16 +81,19 @@ public abstract class AbstractWebApplicationContext implements
     protected void endTransaction(Application application, Object request) {
         LinkedList<Exception> exceptions = null;
 
+        ArrayList<TransactionListener> currentListeners;
         synchronized (listeners) {
-            for (TransactionListener listener : listeners) {
-                try {
-                    listener.transactionEnd(application, request);
-                } catch (final RuntimeException t) {
-                    if (exceptions == null) {
-                        exceptions = new LinkedList<Exception>();
-                    }
-                    exceptions.add(t);
+            currentListeners = new ArrayList<TransactionListener>(listeners);
+        }
+
+        for (TransactionListener listener : currentListeners) {
+            try {
+                listener.transactionEnd(application, request);
+            } catch (final RuntimeException t) {
+                if (exceptions == null) {
+                    exceptions = new LinkedList<Exception>();
                 }
+                exceptions.add(t);
             }
         }
 
diff --git a/tests/src/com/vaadin/tests/applicationcontext/RemoveTransactionListener.java b/tests/src/com/vaadin/tests/applicationcontext/RemoveTransactionListener.java
new file mode 100644
index 0000000000..8723e23a2a
--- /dev/null
+++ b/tests/src/com/vaadin/tests/applicationcontext/RemoveTransactionListener.java
@@ -0,0 +1,78 @@
+package com.vaadin.tests.applicationcontext;
+
+import com.vaadin.Application;
+import com.vaadin.service.ApplicationContext;
+import com.vaadin.service.ApplicationContext.TransactionListener;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.tests.util.Log;
+
+public class RemoveTransactionListener extends TestBase {
+
+    private final Log log = new Log(10);
+
+    @Override
+    protected void setup() {
+        // Add one listener that will remove itself from within transactionEnd
+        getMainWindow().getApplication().getContext()
+                .addTransactionListener(new TransactionListener() {
+                    public void transactionStart(Application application,
+                            Object transactionData) {
+                    }
+
+                    public void transactionEnd(Application application,
+                            Object transactionData) {
+                        removeListener(this);
+                        log.log("Listener removed in transactionEnd");
+                    }
+                });
+
+        // Add one listener that will remove itself from within transactionStart
+        getMainWindow().getApplication().getContext()
+                .addTransactionListener(new TransactionListener() {
+                    public void transactionStart(Application application,
+                            Object transactionData) {
+                        removeListener(this);
+                        log.log("Listener removed in transactionStart");
+                    }
+
+                    public void transactionEnd(Application application,
+                            Object transactionData) {
+                    }
+                });
+
+        // Add one listener to verify that all listeners are called, as thrown
+        // ConcurrentModificationException causes subsequent listeners to be
+        // ignored
+        getMainWindow().getApplication().getContext()
+                .addTransactionListener(new TransactionListener() {
+                    public void transactionStart(Application application,
+                            Object transactionData) {
+                        log.log("transactionStart from last listener");
+                    }
+
+                    public void transactionEnd(Application application,
+                            Object transactionData) {
+                        log.log("transactionEnd from last listener");
+                    }
+                });
+
+        addComponent(log);
+    }
+
+    private void removeListener(TransactionListener l) {
+        ApplicationContext context = getMainWindow().getApplication()
+                .getContext();
+        context.removeTransactionListener(l);
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Tests that a transaction listener can be removed from within the listener.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(7065);
+    }
+
+}