Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix virtual thread pinning caused by synchronized blocks in EclipseLinkJpaDialect #33546

Conversation

chschu
Copy link

@chschu chschu commented Sep 17, 2024

This PR aims to fix virtual thread pinning that occurs with spring-orm and EclipseLink: The code inside the two synchronized blocks in org.springframework.orm.jpa.vendor.EclipseLinkJpaDialect potentially blocks when interacting with a database.

This is the (truncated) Stacktrace (Spring 6.1.12, OpenJDK 21.0.3+9, EclipseLink 4.0.4, PostgreSQL 42.7.4, Tomcat 10.1.26) as logged with -Djdk.tracePinnedThreads=full:

Thread[#61,ForkJoinPool-1-worker-2,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.parkNanos(VirtualThread.java:621)
    java.base/java.lang.System$2.parkVirtualThread(System.java:2652)
    java.base/jdk.internal.misc.VirtualThreads.park(VirtualThreads.java:67)
    java.base/java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:408)
    java.base/sun.nio.ch.Poller.pollIndirect(Poller.java:137)
    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.NioSocketImpl.park(NioSocketImpl.java:175)
    java.base/sun.nio.ch.NioSocketImpl.timedRead(NioSocketImpl.java:280)
    java.base/sun.nio.ch.NioSocketImpl.implRead(NioSocketImpl.java:304)
    java.base/sun.nio.ch.NioSocketImpl.read(NioSocketImpl.java:346)
    java.base/sun.nio.ch.NioSocketImpl$1.read(NioSocketImpl.java:796)
    java.base/java.net.Socket$SocketInputStream.read(Socket.java:1099)
    java.base/sun.security.ssl.SSLSocketInputRecord.read(SSLSocketInputRecord.java:489)
    java.base/sun.security.ssl.SSLSocketInputRecord.readHeader(SSLSocketInputRecord.java:483)
    java.base/sun.security.ssl.SSLSocketInputRecord.bytesInCompletePacket(SSLSocketInputRecord.java:70)
    java.base/sun.security.ssl.SSLSocketImpl.readApplicationRecord(SSLSocketImpl.java:1461)
    java.base/sun.security.ssl.SSLSocketImpl$AppInputStream.read(SSLSocketImpl.java:1066)
    org.postgresql.core.VisibleBufferedInputStream.readMore(VisibleBufferedInputStream.java:192)
    org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:159)
    org.postgresql.core.VisibleBufferedInputStream.ensureBytes(VisibleBufferedInputStream.java:144)
    org.postgresql.core.VisibleBufferedInputStream.read(VisibleBufferedInputStream.java:76)
    org.postgresql.core.PGStream.receiveChar(PGStream.java:476)
    org.postgresql.core.v3.QueryExecutorImpl.processResults(QueryExecutorImpl.java:2174)
    org.postgresql.core.v3.QueryExecutorImpl.execute(QueryExecutorImpl.java:372)
    org.postgresql.jdbc.PgStatement.executeInternal(PgStatement.java:517)
    org.postgresql.jdbc.PgStatement.execute(PgStatement.java:434)
    org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:356)
    org.postgresql.jdbc.PgStatement.executeCachedSql(PgStatement.java:341)
    org.postgresql.jdbc.PgStatement.executeWithFlags(PgStatement.java:317)
    org.postgresql.jdbc.PgStatement.execute(PgStatement.java:312)
    org.apache.tomcat.jdbc.pool.PooledConnection.validate(PooledConnection.java:576)
    org.apache.tomcat.jdbc.pool.PooledConnection.validate(PooledConnection.java:478)
    org.apache.tomcat.jdbc.pool.ConnectionPool.borrowConnection(ConnectionPool.java:847)
    org.apache.tomcat.jdbc.pool.ConnectionPool.borrowConnection(ConnectionPool.java:689)
    org.apache.tomcat.jdbc.pool.ConnectionPool.getConnection(ConnectionPool.java:199)
    org.apache.tomcat.jdbc.pool.DataSourceProxy.getConnection(DataSourceProxy.java:133)
    org.eclipse.persistence.sessions.JNDIConnector.connect(JNDIConnector.java:140)
    org.eclipse.persistence.sessions.DatasourceLogin.connectToDatasource(DatasourceLogin.java:174)
    org.eclipse.persistence.internal.databaseaccess.DatasourceAccessor.connectInternal(DatasourceAccessor.java:374)
    org.eclipse.persistence.internal.databaseaccess.DatabaseAccessor.connectInternal(DatabaseAccessor.java:318)
    org.eclipse.persistence.internal.databaseaccess.DatasourceAccessor.reconnect(DatasourceAccessor.java:617)
    org.eclipse.persistence.internal.databaseaccess.DatabaseAccessor.reconnect(DatabaseAccessor.java:1820)
    org.eclipse.persistence.internal.databaseaccess.DatasourceAccessor.incrementCallCount(DatasourceAccessor.java:345)
    org.eclipse.persistence.internal.databaseaccess.DatasourceAccessor.beginTransaction(DatasourceAccessor.java:269)
    org.eclipse.persistence.internal.sessions.AbstractSession.basicBeginTransaction(AbstractSession.java:742)
    org.eclipse.persistence.sessions.server.ClientSession.addWriteConnection(ClientSession.java:757)
    org.eclipse.persistence.sessions.server.ServerSession.acquireClientConnection(ServerSession.java:271)
    org.eclipse.persistence.sessions.server.ClientSession.getAccessor(ClientSession.java:391)
    org.eclipse.persistence.internal.sessions.UnitOfWorkImpl.getAccessor(UnitOfWorkImpl.java:1960)
    org.eclipse.persistence.internal.jpa.EntityManagerImpl.unwrap(EntityManagerImpl.java:2906)
    org.springframework.orm.jpa.vendor.EclipseLinkJpaDialect.beginTransaction(EclipseLinkJpaDialect.java:131) <== monitors:1
	<...>

Synchronizing on object instances that are part of a public API isn't a good idea in general, so using private locks in EclipseLinkJpaDialect also improves isolation.

The PR may be considered a breaking change if EclipseLink or some other code synchronizes on the DatabaseLogin instance.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 17, 2024
@sbrannen sbrannen added the in: data Issues in data modules (jdbc, orm, oxm, tx) label Sep 17, 2024
@sbrannen sbrannen changed the title Fix virtual thread pinning caused by synchronized blocks in EclipseLinkJpaDialect Fix virtual thread pinning caused by synchronized blocks in EclipseLinkJpaDialect Sep 17, 2024
@jhoeller jhoeller added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 26, 2024
@jhoeller jhoeller self-assigned this Sep 26, 2024
@jhoeller jhoeller added this to the 6.2.0-RC2 milestone Sep 26, 2024
@jhoeller
Copy link
Contributor

Good point! I'm addressing this slightly differently with an instance-level lock in EclipseLinkJpaDialect rather than managing Lock instances per DatabaseLogin since there is usually one EclipseLinkJpaDialect instance per persistence unit (at the same granularity as the DatabaseLogin). Thanks for the pull request, in any case!

@jhoeller jhoeller closed this in 594ed95 Sep 27, 2024
@chschu
Copy link
Author

chschu commented Sep 28, 2024

Thanks for having a look and for the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: data Issues in data modules (jdbc, orm, oxm, tx) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants