You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by ka...@apache.org on 2008/05/28 16:22:43 UTC
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/
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");
+ }
+ }
+
}