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.3k stars 339 forks source link

Virtual thread pinning on c3p0 getConnection with java 21 (loom) #174

Closed alex-kormukhin closed 6 months ago

alex-kormukhin commented 8 months ago

c3p0-loom v. 0.10.0

We use java 21 virtual threads and c3p0 connectionCustomizer's with blocking operation (onCheckOut for example for Oracle EBR support). In such case virtual thread is pinned because connectionCustomizer's methods synchronized on synchronized (inUseLockFetcher.getInUseLock(resc)) block.

image

If we start our applications with -Djdk.tracePinnedThreads=full java option, we can see diagnostic like this: Thread[#82,ForkJoinPool-1-worker-5,5,CarrierThreads] java.base/java.lang.VirtualThread$VThreadContinuation.onPinned(VirtualThread.java:183) java.base/jdk.internal.vm.Continuation.onPinned0(Continuation.java:393) java.base/java.lang.VirtualThread.park(VirtualThread.java:582) java.base/java.lang.System$2.parkVirtualThread(System.java:2643) java.base/jdk.internal.misc.VirtualThreads.park(VirtualThreads.java:54) java.base/java.util.concurrent.locks.LockSupport.park(LockSupport.java:369) java.base/sun.nio.ch.Poller.pollIndirect(Poller.java:139) java.base/sun.nio.ch.Poller.poll(Poller.java:102) java.base/sun.nio.ch.Poller.poll(Poller.java:87) java.base/sun.nio.ch.SelChImpl.park(SelChImpl.java:88) java.base/sun.nio.ch.SelChImpl.park(SelChImpl.java:116) java.base/sun.nio.ch.SocketChannelImpl.read(SocketChannelImpl.java:428) oracle.net.nt.TimeoutSocketChannel.read(TimeoutSocketChannel.java:417) oracle.net.ns.NSProtocolNIO.doSocketRead(NSProtocolNIO.java:1136) oracle.net.ns.NIOPacket.readHeader(NIOPacket.java:268) oracle.net.ns.NIOPacket.readPacketFromSocketChannel(NIOPacket.java:200) oracle.net.ns.NIOPacket.readFromSocketChannel(NIOPacket.java:142) oracle.net.ns.NIOPacket.readFromSocketChannel(NIOPacket.java:115) oracle.net.ns.NIONSDataChannel.readDataFromSocketChannel(NIONSDataChannel.java:98) oracle.jdbc.driver.T4CMAREngineNIO.prepareForUnmarshall(T4CMAREngineNIO.java:834) oracle.jdbc.driver.T4CMAREngineNIO.unmarshalUB1(T4CMAREngineNIO.java:487) oracle.jdbc.driver.T4CTTIfun.receive(T4CTTIfun.java:623) oracle.jdbc.driver.T4CTTIfun.doRPC(T4CTTIfun.java:299) oracle.jdbc.driver.T4C8Oall.doOALL(T4C8Oall.java:512) oracle.jdbc.driver.T4CStatement.doOall8(T4CStatement.java:123) oracle.jdbc.driver.T4CStatement.executeForDescribe(T4CStatement.java:969) oracle.jdbc.driver.OracleStatement.prepareDefineBufferAndExecute(OracleStatement.java:1271) oracle.jdbc.driver.OracleStatement.executeMaybeDescribe(OracleStatement.java:1149) oracle.jdbc.driver.OracleStatement.executeSQLSelect(OracleStatement.java:1661) oracle.jdbc.driver.OracleStatement.doExecuteWithTimeout(OracleStatement.java:1470) oracle.jdbc.driver.OracleStatement.executeQuery(OracleStatement.java:2055) oracle.jdbc.driver.OracleStatementWrapper.executeQuery(OracleStatementWrapper.java:394) some.org.common.db.oracle.ebr.c3p0.internal.EbrSupportBase.queryConnectionEdition(EbrSupportBase.java:136) some.org.common.db.oracle.ebr.c3p0.internal.EbrSupportBase.actualizeConnectionEdition(EbrSupportBase.java:113) some.org.common.db.oracle.ebr.c3p0.internal.C3P0EbrConnectionCustomizer$Customizer.onCheckOut(C3P0EbrConnectionCustomizer.java:61) some.org.common.db.pool.c3p0.internal.C3P0ConnectionCustomizersBridge$Impl.lambda$7(C3P0ConnectionCustomizersBridge.java:216) some.org.common.db.pool.c3p0.internal.C3P0ConnectionCustomizersBridge$Impl.onCheckOut(C3P0ConnectionCustomizersBridge.java:186) some.org.common.db.pool.c3p0.internal.C3P0ConnectionCustomizersBridge$GlobalConnectionCustomizer.call(C3P0ConnectionCustomizersBridge.java:139) some.org.common.db.pool.c3p0.internal.C3P0ConnectionCustomizersBridge$GlobalConnectionCustomizer.onCheckOut(C3P0ConnectionCustomizersBridge.java:114) com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool$1PooledConnectionResourcePoolManager.refurbishResourceOnCheckout(C3P0PooledConnectionPool.java:455) <== monitors:1 com.mchange.v2.resourcepool.BasicResourcePool.attemptRefurbishResourceOnCheckout(BasicResourcePool.java:1709) com.mchange.v2.resourcepool.BasicResourcePool.checkoutResource(BasicResourcePool.java:494) com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool.checkoutAndMarkConnectionInUse(C3P0PooledConnectionPool.java:798) com.mchange.v2.c3p0.impl.C3P0PooledConnectionPool.checkoutPooledConnection(C3P0PooledConnectionPool.java:724) com.mchange.v2.c3p0.impl.AbstractPoolBackedDataSource.getConnection(AbstractPoolBackedDataSource.java:105) some.org.common.db.utils.DataSourceDelegateWrapper.getConnection(DataSourceDelegateWrapper.java:54) ...

Problem marked with "<== monitors:1".

Wold be nice to replace synchronized() to ReentrantLock for virtual thread support.

tuckerje commented 7 months ago

Also seeing the same in the generated code at com.mchange.v2.c3p0.impl.NewProxyConnection.commit(NewProxyConnection.java:981)

swaldman commented 7 months ago

Hi,

Thanks for this. (And sorry for the slow response. Somehow I'm not reliably seeing issue notifications.) I'll look into replacing locking for in-use Connection tracking with ReentrantLock.

swaldman commented 7 months ago

@tuckerje This is a bit of a deeper issue — Operations at the Connection level should be Thread safe. Traditionally that was done by synchronizing access to all Connection methods, which is what NewProxyConnection does. To remedy this, there are a couple of choices:

  1. Use ReentrantLock or similar to control Connection-level access to proxies, replicating the current arrangement
  2. Try to use more fine-grained locking and reply upon the (not always perfect, in my experience) thread-safety of wrapped Connections.

I'll try 1 first, 'cuz it shouldn't be so hard, and Connection-level operations tend not to be performance sensitive or highly contended. (Indeed, most applications adopt a thread-per-Connection model. c3p0 perhaps overdoes a requirement from early versions of the JDBC spec that may later have been dropped.)

swaldman commented 6 months ago

@alex-kormukhin @tuckerje Both of the pins you have pointed to should be fixed in the current 0.10.1-SNAPSHOT. Thank you for pointing them out. I'd like c3p0 to play well with loom virtual threads, so I appreciate the help.

If you want to try out the latest snapshot, it's published on my local repository https://www.mchange.com/repository/ as "com.mchange:c3p0:0.10.1-SNAPSHOT" ( or download the SNAPSHOT jar file directly from https://www.mchange.com/repository/com/mchange/c3p0/0.10.1-SNAPSHOT/c3p0-0.10.1-SNAPSHOT.jar )

(Please don't permanently depend upon SNAPSHOT releases. They are for testing only, and frequently ovewritten.)

swaldman commented 6 months ago

Hi! c3p0-0.10.1 is now a release. This issue should be addressed in that version. I'll close it for now. If anything goes wrong, please feel free to submit a new issue or reopen this one. Thanks!