swaldman / c3p0

a mature, highly concurrent JDBC Connection pooling library, with support for caching and reuse of PreparedStatements.
http://www.mchange.com/projects/c3p0
Other
1.28k stars 338 forks source link

java.util.ConcurrentModificationException on stmtSet iterator.Next() call in GooGooStatementCache.checkinAll #164

Closed wvkehoe closed 6 months ago

wvkehoe commented 1 year ago

C3P0 Version: c3p0-0.9.5.5.jar:0.9.5.5 The following stack trace shows that our application is encountering a java.util.ConcurrentModificationException when hibernate attempts to close a connection managed by a C3PO connection pool:

Caused by: java.util.ConcurrentModificationException at java.util.HashMap$HashIterator.nextNode(HashMap.java:1469) ~[?:1.8.0_342] at java.util.HashMap$KeyIterator.next(HashMap.java:1493) ~[?:1.8.0_342] at com.mchange.v2.c3p0.stmt.GooGooStatementCache.checkinAll(GooGooStatementCache.java:322) ~[c3p0-0.9.5.5.jar:0.9.5.5] at com.mchange.v2.c3p0.impl.NewPooledConnection.checkinAllCachedStatements(NewPooledConnection.java:772) ~[c3p0-0.9.5.5.jar:0.9.5.5] at com.mchange.v2.c3p0.impl.NewPooledConnection.markClosedProxyConnection(NewPooledConnection.java:402) ~[c3p0-0.9.5.5.jar:0.9.5.5] at com.mchange.v2.c3p0.impl.NewProxyConnection.close(NewProxyConnection.java:87) ~[c3p0-0.9.5.5.jar:0.9.5.5] at org.hibernate.c3p0.internal.C3P0ConnectionProvider.closeConnection(C3P0ConnectionProvider.java:84) ~[hibernate-c3p0-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.internal.NonContextualJdbcConnectionAccess.releaseConnection(NonContextualJdbcConnectionAccess.java:46) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.resource.jdbc.internal.LogicalConnectionManagedImpl.releaseConnection(LogicalConnectionManagedImpl.java:196) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.resource.jdbc.internal.LogicalConnectionManagedImpl.afterTransaction(LogicalConnectionManagedImpl.java:162) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.engine.jdbc.internal.JdbcCoordinatorImpl.afterTransaction(JdbcCoordinatorImpl.java:288) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.internal.SessionImpl.afterOperation(SessionImpl.java:579) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.internal.SessionImpl.list(SessionImpl.java:1543) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.query.internal.AbstractProducedQuery.doList(AbstractProducedQuery.java:1561) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] at org.hibernate.query.internal.AbstractProducedQuery.list(AbstractProducedQuery.java:1529) ~[hibernate-core-5.4.2.Final.jar:5.4.2.Final] ... 72 more

While, at this juncture, we do not have evidence of that this scenario is actually occurring, it at least appears that the method-level "synchronized" protection for checkinAll() at https://github.com/swaldman/c3p0/blob/c3p0-0.9.5.5/src/java/com/mchange/v2/c3p0/stmt/GooGooStatementCache.java#L312 coupled with the block-level synchronized protection in closeAll() at https://github.com/swaldman/c3p0/blob/c3p0-0.9.5.5/src/java/com/mchange/v2/c3p0/stmt/GooGooStatementCache.java#L362 does not protect against the possibility of concurrent calls to both checkInAll() and closeAll(). Two such concurrent calls on the same GooGooStatementCache instance might attempt to access the same statementSet resulting in the ConcurrentModificationException.

Have we somehow mis-configured C3PO? Are we using an unsupported combination of hibernate and C3PO versions? Or is this just a defect in C3PO?

Here are our c3p0 and hibernate settings:

` 10

  <property name="hibernate.c3p0.max_statements">50</property>

  <property name="hibernate.c3p0.min_size">0</property>

  <property name="hibernate.c3p0.acquire_increment">1</property>

  <property name="hibernate.max_fetch_depth">5</property>

  <property name="hibernate.generate_statistics">false</property>

  <property name="hibernate.c3p0.acquireRetryAttempts">5</property>

  <property name="hibernate.c3p0.checkoutTimeout">10000</property>`
swaldman commented 6 months ago

Hi!

Thanks for this!

As of 0.10.0 (should be a release within the next day or two), we make defensive copies before iterating in both methods, so I hope you won't have this problem going forward.