From c64dae36237e9ab41737c4b72977db879f0356ef Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 28 Feb 2025 14:12:51 +0100 Subject: [PATCH] Avoid getTargetConnection call on transaction-aware Connection close Closes gh-34484 --- .../jdbc/datasource/DataSourceUtils.java | 8 ++- .../TransactionAwareDataSourceProxy.java | 16 ++++-- .../DataSourceTransactionManagerTests.java | 49 ++++++++++++++++--- 3 files changed, 62 insertions(+), 11 deletions(-) diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java index ad99f2e2d55d..8daa652a3d15 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java @@ -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. @@ -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; } diff --git a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java index d9cfce3885e8..3945c896eeea 100644 --- a/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java +++ b/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/TransactionAwareDataSourceProxy.java @@ -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. @@ -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; } diff --git a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java index 81cc8142e1c8..9849784d8b28 100644 --- a/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java +++ b/spring-jdbc/src/test/java/org/springframework/jdbc/datasource/DataSourceTransactionManagerTests.java @@ -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. @@ -72,7 +72,7 @@ public class DataSourceTransactionManagerTests { protected DataSource ds = mock(); - protected Connection con = mock(); + protected ConnectionProxy con = mock(); protected DataSourceTransactionManager tm; @@ -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) { @@ -1073,9 +1074,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); @@ -1109,9 +1110,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); @@ -1127,6 +1128,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);