Skip to content

Commit

Permalink
Avoid getTargetConnection call on transaction-aware Connection close
Browse files Browse the repository at this point in the history
Closes gh-34484

(cherry picked from commit c64dae3)
  • Loading branch information
jhoeller committed Feb 28, 2025
1 parent 0d60f26 commit 1e31cd6
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -439,7 +439,11 @@ private static boolean connectionEquals(ConnectionHolder conHolder, Connection p
public static Connection getTargetConnection(Connection con) {
Connection conToUse = con;
while (conToUse instanceof ConnectionProxy connectionProxy) {
conToUse = connectionProxy.getTargetConnection();
Connection targetCon = connectionProxy.getTargetConnection();
if (targetCon == conToUse) {
break;
}
conToUse = targetCon;
}
return conToUse;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2023 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -210,13 +210,23 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
sb.append('[').append(this.target).append(']');
}
else {
sb.append(" from DataSource [").append(this.targetDataSource).append(']');
sb.append("from DataSource [").append(this.targetDataSource).append(']');
}
return sb.toString();
}
case "close" -> {
// Handle close method: only close if not within a transaction.
DataSourceUtils.doReleaseConnection(this.target, this.targetDataSource);
if (this.target != null) {
ConnectionHolder conHolder = (ConnectionHolder)
TransactionSynchronizationManager.getResource(this.targetDataSource);
if (conHolder != null && conHolder.hasConnection() && conHolder.getConnection() == this.target) {
// It's the transactional Connection: Don't close it.
conHolder.released();
}
else {
DataSourceUtils.doCloseConnection(this.target, this.targetDataSource);
}
}
this.closed = true;
return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -72,7 +72,7 @@ public class DataSourceTransactionManagerTests {

protected DataSource ds = mock();

protected Connection con = mock();
protected ConnectionProxy con = mock();

protected DataSourceTransactionManager tm;

Expand All @@ -81,6 +81,7 @@ public class DataSourceTransactionManagerTests {
void setup() throws Exception {
tm = createTransactionManager(ds);
given(ds.getConnection()).willReturn(con);
given(con.getTargetConnection()).willThrow(new UnsupportedOperationException());
}

protected DataSourceTransactionManager createTransactionManager(DataSource ds) {
Expand Down Expand Up @@ -1074,9 +1075,9 @@ protected void doInTransactionWithoutResult(TransactionStatus status) {
Connection tCon = dsProxy.getConnection();
tCon.getWarnings();
tCon.clearWarnings();
assertThat(((ConnectionProxy) dsProxy.getConnection()).getTargetConnection()).isEqualTo(con);
assertThat(((ConnectionProxy) tCon).getTargetConnection()).isEqualTo(con);
// should be ignored
dsProxy.getConnection().close();
tCon.close();
}
catch (SQLException ex) {
throw new UncategorizedSQLException("", "", ex);
Expand Down Expand Up @@ -1110,9 +1111,9 @@ protected void doInTransactionWithoutResult(TransactionStatus status) {
Connection tCon = dsProxy.getConnection();
assertThatExceptionOfType(SQLException.class).isThrownBy(tCon::getWarnings);
tCon.clearWarnings();
assertThat(((ConnectionProxy) dsProxy.getConnection()).getTargetConnection()).isEqualTo(con);
assertThat(((ConnectionProxy) tCon).getTargetConnection()).isEqualTo(con);
// should be ignored
dsProxy.getConnection().close();
tCon.close();
}
catch (SQLException ex) {
throw new UncategorizedSQLException("", "", ex);
Expand All @@ -1128,6 +1129,42 @@ protected void doInTransactionWithoutResult(TransactionStatus status) {
verify(con).close();
}

@Test
void testTransactionAwareDataSourceProxyWithEarlyConnection() throws Exception {
given(ds.getConnection()).willReturn(mock(Connection.class), con);
given(con.getAutoCommit()).willReturn(true);
given(con.getWarnings()).willThrow(new SQLException());

TransactionAwareDataSourceProxy dsProxy = new TransactionAwareDataSourceProxy(ds);
dsProxy.setLazyTransactionalConnections(false);
Connection tCon = dsProxy.getConnection();

TransactionTemplate tt = new TransactionTemplate(tm);
assertThat(TransactionSynchronizationManager.hasResource(ds)).isFalse();
tt.execute(new TransactionCallbackWithoutResult() {
@Override
protected void doInTransactionWithoutResult(TransactionStatus status) {
// something transactional
assertThat(DataSourceUtils.getConnection(ds)).isEqualTo(con);
try {
// should close the early Connection obtained before the transaction
tCon.close();
}
catch (SQLException ex) {
throw new UncategorizedSQLException("", "", ex);
}
}
});

assertThat(TransactionSynchronizationManager.hasResource(ds)).isFalse();

InOrder ordered = inOrder(con);
ordered.verify(con).setAutoCommit(false);
ordered.verify(con).commit();
ordered.verify(con).setAutoCommit(true);
verify(con).close();
}

@Test
void testTransactionAwareDataSourceProxyWithSuspension() throws Exception {
given(con.getAutoCommit()).willReturn(true);
Expand Down

0 comments on commit 1e31cd6

Please sign in to comment.