db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r660959 - in /db/derby/code/trunk/java: client/org/apache/derby/client/ engine/org/apache/derby/jdbc/ testing/org/apache/derbyTesting/functionTests/tests/jdbc4/
Date Wed, 28 May 2008 14:22:43 GMT
Author: kahatlen
Date: Wed May 28 07:22:42 2008
New Revision: 660959

URL: http://svn.apache.org/viewvc?rev=660959&view=rev
Log:
DERBY-3401 (partial) Fixed ConcurrentModificationException if a
statement event listener adds or removes a listener on the same pooled
connection.

Modified:
    db/derby/code/trunk/java/client/org/apache/derby/client/ClientPooledConnection40.java
    db/derby/code/trunk/java/client/org/apache/derby/client/ClientXAConnection40.java
    db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection40.java
    db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection40.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/StatementEventsTest.java

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/ClientPooledConnection40.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/ClientPooledConnection40.java?rev=660959&r1=660958&r2=660959&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/ClientPooledConnection40.java
(original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/ClientPooledConnection40.java
Wed May 28 07:22:42 2008
@@ -23,7 +23,7 @@
 
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
-import java.util.ArrayList;
+import java.util.concurrent.CopyOnWriteArrayList;
 import javax.sql.StatementEventListener;
 import javax.sql.StatementEvent;
 import org.apache.derby.jdbc.ClientBaseDataSource;
@@ -38,10 +38,14 @@
 
 public class ClientPooledConnection40 extends ClientPooledConnection {
     
-    /** List of statement event listeners. */
-    //@GuardedBy("this")
-    private final ArrayList<StatementEventListener> statementEventListeners = 
-             new ArrayList<StatementEventListener>();
+    /**
+     * List of statement event listeners. The list is copied on each write,
+     * ensuring that it can be safely iterated over even if other threads or
+     * the listeners fired in the same thread add or remove listeners.
+     */
+    private final CopyOnWriteArrayList<StatementEventListener>
+            statementEventListeners =
+                    new CopyOnWriteArrayList<StatementEventListener>();
 
     public ClientPooledConnection40(ClientBaseDataSource ds,
         org.apache.derby.client.am.LogWriter logWriter,
@@ -73,7 +77,7 @@
      *                  interface and wants to be notified of Statement closed or 
      *                  or Statement error occurred events
      */
-    public synchronized void addStatementEventListener(StatementEventListener listener){
+    public void addStatementEventListener(StatementEventListener listener) {
         if (logWriter_ != null) {
             logWriter_.traceEntry(this, "addStatementEventListener", listener);
         }
@@ -89,7 +93,7 @@
      * @param listener The previously registered event listener that needs to be
      *                 removed from the list of components
      */
-    public synchronized void removeStatementEventListener(StatementEventListener listener){
+    public void removeStatementEventListener(StatementEventListener listener) {
         if (logWriter_ != null) {
             logWriter_.traceEntry(this, "removeConnectionEventListener", listener);
         }
@@ -104,7 +108,7 @@
      * @param statement The PreparedStatement that was closed
      *
      */
-    public synchronized void onStatementClose(PreparedStatement statement) {
+    public void onStatementClose(PreparedStatement statement) {
         if (!statementEventListeners.isEmpty()) {
             StatementEvent event = new StatementEvent(this,statement);
             for (StatementEventListener l : statementEventListeners) {
@@ -123,9 +127,8 @@
      *                  caused the invalidation of the PreparedStatements
      *
      */
-    public synchronized void onStatementErrorOccurred(
-                                                    PreparedStatement statement,
-                                                    SQLException sqle) {
+    public void onStatementErrorOccurred(PreparedStatement statement,
+                                         SQLException sqle) {
         if (!statementEventListeners.isEmpty()) {
             StatementEvent event = new StatementEvent(this,statement,sqle);
             for (StatementEventListener l : statementEventListeners) {

Modified: db/derby/code/trunk/java/client/org/apache/derby/client/ClientXAConnection40.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/client/org/apache/derby/client/ClientXAConnection40.java?rev=660959&r1=660958&r2=660959&view=diff
==============================================================================
--- db/derby/code/trunk/java/client/org/apache/derby/client/ClientXAConnection40.java (original)
+++ db/derby/code/trunk/java/client/org/apache/derby/client/ClientXAConnection40.java Wed
May 28 07:22:42 2008
@@ -23,14 +23,9 @@
 
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
-import java.util.Enumeration;
-import java.util.Vector;
+import java.util.concurrent.CopyOnWriteArrayList;
 import javax.sql.StatementEvent;
 import javax.sql.StatementEventListener;
-import org.apache.derby.client.am.SqlException;
-import org.apache.derby.client.net.NetLogWriter;
-import org.apache.derby.client.net.NetXAConnection;
-import org.apache.derby.jdbc.ClientDataSource;
 import org.apache.derby.jdbc.ClientXADataSource;
 
 /**
@@ -38,10 +33,14 @@
  */ 
 public class ClientXAConnection40 extends ClientXAConnection {
     
-    //using generics to avoid casting problems
-     protected final Vector<StatementEventListener> statementEventListeners = 
-             new Vector<StatementEventListener>();
-
+    /**
+     * List of statement event listeners. The list is copied on each write,
+     * ensuring that it can be safely iterated over even if other threads or
+     * the listeners fired in the same thread add or remove listeners.
+     */
+    private final CopyOnWriteArrayList<StatementEventListener>
+            statementEventListeners =
+                     new CopyOnWriteArrayList<StatementEventListener>();
     
     /**
      * Constructor for ClientXAConnection40.
@@ -73,7 +72,7 @@
         if (logWriter_ != null) {
             logWriter_.traceEntry(this, "removeConnectionEventListener", listener);
         }
-        statementEventListeners.removeElement(listener);
+        statementEventListeners.remove(listener);
     }
     
     /**
@@ -92,7 +91,7 @@
         if (logWriter_ != null) {
             logWriter_.traceEntry(this, "addStatementEventListener", listener);
         }
-        statementEventListeners.addElement(listener);
+        statementEventListeners.add(listener);
     }
     
     /**
@@ -103,12 +102,8 @@
     public void onStatementClose(PreparedStatement statement) {
         if (!statementEventListeners.isEmpty()) {
             StatementEvent event = new StatementEvent(this,statement);
-            //synchronized block on statementEventListeners to make it thread
-            //safe
-            synchronized(statementEventListeners) {
-                for (StatementEventListener l : statementEventListeners) {
-                    l.statementClosed(event);
-                }
+            for (StatementEventListener l : statementEventListeners) {
+                l.statementClosed(event);
             }
         }
     }
@@ -127,12 +122,8 @@
                     SQLException sqle) {
         if (!statementEventListeners.isEmpty()) {
             StatementEvent event = new StatementEvent(this,statement,sqle);
-            //synchronized block on statementEventListeners to make it thread
-            //safe
-            synchronized(statementEventListeners) {
-                for (StatementEventListener l : statementEventListeners) {
-                    l.statementErrorOccurred(event);
-                }
+            for (StatementEventListener l : statementEventListeners) {
+                l.statementErrorOccurred(event);
             }
         }
     }   

Modified: db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection40.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection40.java?rev=660959&r1=660958&r2=660959&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection40.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedPooledConnection40.java Wed
May 28 07:22:42 2008
@@ -21,11 +21,9 @@
 
 package org.apache.derby.jdbc;
 
-import java.sql.Connection;
 import java.sql.SQLException;
-import java.util.Enumeration;
-import java.util.Vector;
 import java.sql.PreparedStatement;
+import java.util.concurrent.CopyOnWriteArrayList;
 import javax.sql.StatementEvent;
 import javax.sql.StatementEventListener;
 
@@ -41,11 +39,15 @@
 
  */
 class EmbedPooledConnection40 extends EmbedPooledConnection {
-    
-    //using generics to avoid casting problems
-    protected final Vector<StatementEventListener> statementEventListeners =
-            new Vector<StatementEventListener>();
-    
+
+    /**
+     * List of statement event listeners. The list is copied on each write,
+     * ensuring that it can be safely iterated over even if other threads or
+     * the listeners fired in the same thread add or remove listeners.
+     */
+    private final CopyOnWriteArrayList<StatementEventListener>
+            statementEventListeners =
+                    new CopyOnWriteArrayList<StatementEventListener>();
 
     EmbedPooledConnection40 (ReferenceableDataSource ds, String user, 
                  String password, boolean requestPassword) throws SQLException {
@@ -66,7 +68,7 @@
     public void removeStatementEventListener(StatementEventListener listener) {
         if (listener == null)
             return;
-        statementEventListeners.removeElement(listener);
+        statementEventListeners.remove(listener);
     }
 
     /**
@@ -89,7 +91,7 @@
             return;
         if (listener == null)
             return;
-        statementEventListeners.addElement(listener);
+        statementEventListeners.add(listener);
     }
     
     /**
@@ -100,12 +102,8 @@
     public void onStatementClose(PreparedStatement statement) {
         if (!statementEventListeners.isEmpty()){
             StatementEvent event = new StatementEvent(this,statement);
-            //synchronized block on statementEventListeners to make it thread
-            //safe
-            synchronized(statementEventListeners) {
-                for (StatementEventListener l : statementEventListeners) {
-                    l.statementClosed(event);
-                }
+            for (StatementEventListener l : statementEventListeners) {
+                l.statementClosed(event);
             }
         }
     }
@@ -119,12 +117,8 @@
     public void onStatementErrorOccurred(PreparedStatement statement,SQLException sqle) {
         if (!statementEventListeners.isEmpty()){
             StatementEvent event = new StatementEvent(this,statement,sqle);
-            //synchronized block on statementEventListeners to make it thread
-            //safe
-            synchronized(statementEventListeners) {
-                for (StatementEventListener l : statementEventListeners){
-                    l.statementErrorOccurred(event);
-                }
+            for (StatementEventListener l : statementEventListeners) {
+                l.statementErrorOccurred(event);
             }
         }
     }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection40.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection40.java?rev=660959&r1=660958&r2=660959&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection40.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/jdbc/EmbedXAConnection40.java Wed May
28 07:22:42 2008
@@ -21,9 +21,9 @@
 
 package org.apache.derby.jdbc;
 
-import java.util.Vector;
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
+import java.util.concurrent.CopyOnWriteArrayList;
 import javax.sql.StatementEvent;
 import javax.sql.StatementEventListener;
 import javax.sql.XAConnection;
@@ -35,9 +35,14 @@
 final class EmbedXAConnection40 extends EmbedXAConnection
         implements XAConnection {
     
-    //using generics to avoid casting problems
-    protected final Vector<StatementEventListener> statementEventListeners =
-            new Vector<StatementEventListener>();
+    /**
+     * List of statement event listeners. The list is copied on each write,
+     * ensuring that it can be safely iterated over even if other threads or
+     * the listeners fired in the same thread add or remove listeners.
+     */
+    private final CopyOnWriteArrayList<StatementEventListener>
+            statementEventListeners =
+                    new CopyOnWriteArrayList<StatementEventListener>();
     
     /**
      * Creates EmbedXAConnection40.
@@ -67,7 +72,7 @@
     public void removeStatementEventListener(StatementEventListener listener) {
         if (listener == null)
             return;
-        statementEventListeners.removeElement(listener);
+        statementEventListeners.remove(listener);
     }
 
     /**
@@ -90,7 +95,7 @@
             return;
         if (listener == null)
             return;
-        statementEventListeners.addElement(listener);
+        statementEventListeners.add(listener);
     }
     
     /**
@@ -101,12 +106,8 @@
     public void onStatementClose(PreparedStatement statement) {
         if (!statementEventListeners.isEmpty()){
             StatementEvent event = new StatementEvent(this,statement);
-            //synchronized block on statementEventListeners to make it thread
-            //safe
-            synchronized(statementEventListeners) {
-                for (StatementEventListener l : statementEventListeners) {
-                    l.statementClosed(event);
-                }
+            for (StatementEventListener l : statementEventListeners) {
+                l.statementClosed(event);
             }
         }
     }
@@ -120,12 +121,8 @@
     public void onStatementErrorOccurred(PreparedStatement statement,SQLException sqle) {
         if (!statementEventListeners.isEmpty()){
             StatementEvent event = new StatementEvent(this,statement,sqle);
-            //synchronized block on statementEventListeners to make it thread
-            //safe
-            synchronized(statementEventListeners) {
-                for (StatementEventListener l : statementEventListeners){
-                    l.statementErrorOccurred(event);
-                }
+            for (StatementEventListener l : statementEventListeners) {
+                l.statementErrorOccurred(event);
             }
         }
     }

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/StatementEventsTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/StatementEventsTest.java?rev=660959&r1=660958&r2=660959&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/StatementEventsTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/jdbc4/StatementEventsTest.java
Wed May 28 07:22:42 2008
@@ -22,6 +22,7 @@
 package org.apache.derbyTesting.functionTests.tests.jdbc4;
 
 import java.sql.*;
+import java.util.Arrays;
 import javax.sql.*;
 import junit.framework.*;
 
@@ -273,4 +274,221 @@
             assertEquals("Incorrect error count.", 1, errorCount);
         }
     }
+
+    /**
+     * Test that removing a listener from a listener works. (DERBY-3401)
+     */
+    public void testRemoveListenerFromListener() throws SQLException {
+
+        // First element is number of times the close listeners below have
+        // been triggered, second element is number of times the error
+        // listeners have been triggered.
+        final int[] counters = new int[2];
+
+        // Add some listeners that remove themselves
+        for (int i = 0; i < 5; i++) {
+            StatementEventListener close = new StatementEventListener() {
+
+                public void statementClosed(StatementEvent event) {
+                    pooledConnection.removeStatementEventListener(this);
+                    counters[0]++;
+                }
+
+                public void statementErrorOccurred(StatementEvent event) {
+                }
+            };
+            pooledConnection.addStatementEventListener(close);
+
+            StatementEventListener error = new StatementEventListener() {
+
+                public void statementClosed(StatementEvent event) {
+                }
+
+                public void statementErrorOccurred(StatementEvent event) {
+                    pooledConnection.removeStatementEventListener(this);
+                    counters[1]++;
+                }
+            };
+            pooledConnection.addStatementEventListener(error);
+        }
+
+        // Generate close event twice. The close listeners remove themselves
+        // in the first iteration, so no updates of the counters are expected
+        // in the second iteration.
+        for (int i = 0; i < 2; i++) {
+            prepare("VALUES (1)").close();
+            assertEquals("unexpected number of close events", 5, counters[0]);
+            assertEquals("unexpected number of error events", 0, counters[1]);
+        }
+
+        // reset counters
+        Arrays.fill(counters, 0);
+
+        // Generate error event twice. Only expect counters to be updated in
+        // the first iteration since the listeners remove themselves.
+        for (int i = 0; i < 2; i++) {
+            PreparedStatement ps = prepare("VALUES (1)");
+            connection.close();
+            try {
+                ps.execute();
+                fail("Execute on closed connection should fail");
+            } catch (SQLNonTransientConnectionException e) {
+                assertSQLState("08003", e);
+            }
+            assertEquals("unexpected number of close events", 0, counters[0]);
+            assertEquals("unexpected number of error events", 5, counters[1]);
+            connection = pooledConnection.getConnection();
+        }
+
+        // The listeners that are automatically added for all test cases have
+        // been active all the time.
+        assertEquals("Incorrect error count", 2, errorCount);
+        // Embedded doesn't receive close events when the connection is
+        // closed, whereas the client driver does. This is therefore an
+        // expected difference.
+        if (usingEmbedded()) {
+            assertEquals("Incorrect close count", 2, closedCount);
+        } else if (usingDerbyNetClient()) {
+            assertEquals("Incorrect close count", 4, closedCount);
+        } else {
+            fail("unknown framework");
+        }
+    }
+
+    /**
+     * Test that adding a listener from a listener works. (DERBY-3401)
+     */
+    public void testAddListenerFromListener() throws SQLException {
+
+        // First element is number of times the close listeners below have
+        // been triggered, second element is number of times the error
+        // listeners have been triggered. Third element is the number of
+        // times listeners added by close listeners have been triggered,
+        // fourth element is the number of times listeners added by error
+        // listeners have been triggered.
+        final int[] counters = new int[4];
+
+        // Add some listeners that add another listener
+        for (int i = 0; i < 5; i++) {
+            StatementEventListener close = new StatementEventListener() {
+
+                public void statementClosed(StatementEvent event) {
+                    counters[0]++;
+                    pooledConnection.addStatementEventListener(
+                            new StatementEventListener() {
+                        public void statementClosed(StatementEvent e) {
+                            counters[2]++;
+                        }
+                        public void statementErrorOccurred(StatementEvent e) {
+                            counters[2]++;
+                        }
+                    });
+                }
+
+                public void statementErrorOccurred(StatementEvent event) {
+                }
+            };
+
+            pooledConnection.addStatementEventListener(close);
+
+            StatementEventListener error = new StatementEventListener() {
+
+                public void statementClosed(StatementEvent event) {
+                }
+
+                public void statementErrorOccurred(StatementEvent event) {
+                    counters[1]++;
+                    pooledConnection.addStatementEventListener(
+                            new StatementEventListener() {
+                        public void statementClosed(StatementEvent e) {
+                            counters[3]++;
+                        }
+                        public void statementErrorOccurred(StatementEvent e) {
+                            counters[3]++;
+                        }
+                    });
+                }
+            };
+
+            pooledConnection.addStatementEventListener(error);
+        }
+
+        // Generate close event
+        prepare("VALUES (1)").close();
+        assertEquals("unexpected number of close events", 5, counters[0]);
+        assertEquals("unexpected number of error events", 0, counters[1]);
+        assertEquals("unexpected number of added close listeners triggered",
+                     0, counters[2]);
+        assertEquals("unexpected number of added error listeners triggered",
+                     0, counters[3]);
+
+        // Generate another close event
+        prepare("VALUES (1)").close();
+        assertEquals("unexpected number of close events", 10, counters[0]);
+        assertEquals("unexpected number of error events", 0, counters[1]);
+        assertEquals("unexpected number of added close listeners triggered",
+                     5, counters[2]);
+        assertEquals("unexpected number of added error listeners triggered",
+                     0, counters[3]);
+
+        // Generate a statement that doesn't work
+        PreparedStatement ps = prepare("VALUES (1)");
+        connection.close();
+        // reset counters
+        Arrays.fill(counters, 0);
+
+        // Generate an error event
+        try {
+            ps.execute();
+            fail("Execute on closed connection should fail");
+        } catch (SQLNonTransientConnectionException e) {
+            assertSQLState("08003", e);
+        }
+
+        assertEquals("unexpected number of close events", 0, counters[0]);
+        assertEquals("unexpected number of error events", 5, counters[1]);
+        // difference between embedded and client because client gets
+        // statement-closed event when the connection is closed, whereas
+        // embedded doesn't
+        assertEquals("unexpected number of added close listeners triggered",
+                     usingEmbedded() ? 10 : 15, counters[2]);
+        assertEquals("unexpected number of added error listeners triggered",
+                     0, counters[3]);
+
+        // reset counters
+        Arrays.fill(counters, 0);
+
+        // Generate another error event, now with more listeners active
+        try {
+            ps.execute();
+            fail("Execute on closed connection should fail");
+        } catch (SQLNonTransientConnectionException e) {
+            assertSQLState("08003", e);
+        }
+
+        assertEquals("unexpected number of close events", 0, counters[0]);
+        assertEquals("unexpected number of error events", 5, counters[1]);
+        // difference between embedded and client because client gets
+        // statement-closed event when the connection is closed, whereas
+        // embedded doesn't
+        assertEquals("unexpected number of added close listeners triggered",
+                     usingEmbedded() ? 10 : 15, counters[2]);
+        assertEquals("unexpected number of added error listeners triggered",
+                     5, counters[3]);
+
+        // The listeners that are automatically added for all test cases have
+        // been active all the time.
+        assertEquals("Incorrect error count", 2, errorCount);
+        // Embedded doesn't receive close events when the connection is
+        // closed, whereas the client driver does. This is therefore an
+        // expected difference.
+        if (usingEmbedded()) {
+            assertEquals("Incorrect close count", 2, closedCount);
+        } else if (usingDerbyNetClient()) {
+            assertEquals("Incorrect close count", 3, closedCount);
+        } else {
+            fail("unknown framework");
+        }
+    }
+
 }



Mime
View raw message