Do not keep target connection after failed settings

Includes aligned setReadOnly exception suppression.

Closes gh-35980

Author: jhoeller

spring-jdbc/src/main/java/org/springframework/jdbc/datasource/DataSourceUtils.java

@@ -200,24 +200,10 @@ private static Connection fetchConnection(DataSource dataSource) throws SQLExcep
 		boolean debugEnabled = logger.isDebugEnabled();
 		// Set read-only flag.
 		if (setReadOnly) {
-			try {
-				if (debugEnabled) {
-					logger.debug("Setting JDBC Connection [" + con + "] read-only");
-				}
-				con.setReadOnly(true);
-			}
-			catch (SQLException | RuntimeException ex) {
-				Throwable exToCheck = ex;
-				while (exToCheck != null) {
-					if (exToCheck.getClass().getSimpleName().contains("Timeout")) {
-						// Assume it's a connection timeout that would otherwise get lost: for example, from JDBC 4.0
-						throw ex;
-					}
-					exToCheck = exToCheck.getCause();
-				}
-				// "read-only not supported" SQLException -> ignore, it's just a hint anyway
-				logger.debug("Could not set JDBC Connection read-only", ex);
+			if (debugEnabled) {
+				logger.debug("Setting JDBC Connection [" + con + "] read-only");
 			}
+			setReadOnlyIfPossible(con);
 		}
 
 		// Apply specific isolation level, if any.
@@ -236,6 +222,31 @@ private static Connection fetchConnection(DataSource dataSource) throws SQLExcep
 		return previousIsolationLevel;
 	}
 
+	/**
+	 * Apply the read-only hint to the given Connection,
+	 * suppressing exceptions other than timeout-related ones.
+	 * @param con the Connection to prepare
+	 * @throws SQLException in case of a timeout exception
+	 * @since 6.2.15
+	 */
+	static void setReadOnlyIfPossible(Connection con) throws SQLException {
+		try {
+			con.setReadOnly(true);
+		}
+		catch (SQLException | RuntimeException ex) {
+			Throwable exToCheck = ex;
+			while (exToCheck != null) {
+				if (exToCheck.getClass().getSimpleName().contains("Timeout")) {
+					// Assume it's a connection timeout that would otherwise get lost: for example, from JDBC 4.0
+					throw ex;
+				}
+				exToCheck = exToCheck.getCause();
+			}
+			// "read-only not supported" SQLException -> ignore, it's just a hint anyway
+			logger.debug("Could not set JDBC Connection read-only", ex);
+		}
+	}
+
 	/**
 	 * Reset the given Connection after a transaction,
 	 * regarding read-only flag and isolation level.

spring-jdbc/src/main/java/org/springframework/jdbc/datasource/LazyConnectionDataSourceProxy.java

@@ -460,48 +460,56 @@ private boolean hasTargetConnection() {
 		/**
 		 * Return the target Connection, fetching it and initializing it if necessary.
 		 */
-		private Connection getTargetConnection(Method operation) throws SQLException {
-			if (this.target == null) {
-				// No target Connection held -> fetch one.
+		private Connection getTargetConnection(Method operation) throws Throwable {
+			Connection target = this.target;
+			if (target != null) {
+				// Target Connection already held -> return it.
 				if (logger.isTraceEnabled()) {
-					logger.trace("Connecting to database for operation '" + operation.getName() + "'");
+					logger.trace("Using existing database connection for operation '" + operation.getName() + "'");
 				}
+				return target;
+			}
 
-				// Fetch physical Connection from DataSource.
-				DataSource dataSource = getDataSourceToUse();
-				this.target = (this.username != null ? dataSource.getConnection(this.username, this.password) :
-						dataSource.getConnection());
-				if (this.target == null) {
-					throw new IllegalStateException("DataSource returned null from getConnection(): " + dataSource);
-				}
+			// No target Connection held -> fetch one.
+			if (logger.isTraceEnabled()) {
+				logger.trace("Connecting to database for operation '" + operation.getName() + "'");
+			}
+
+			// Fetch physical Connection from DataSource.
+			DataSource dataSource = getDataSourceToUse();
+			target = (this.username != null ? dataSource.getConnection(this.username, this.password) :
+					dataSource.getConnection());
+			if (target == null) {
+				throw new IllegalStateException("DataSource returned null from getConnection(): " + dataSource);
+			}
 
-				// Apply kept transaction settings, if any.
+			// Apply kept transaction settings, if any.
+			try {
 				if (this.readOnly && readOnlyDataSource == null) {
-					try {
-						this.target.setReadOnly(true);
-					}
-					catch (Exception ex) {
-						// "read-only not supported" -> ignore, it's just a hint anyway
-						logger.debug("Could not set JDBC Connection read-only", ex);
-					}
+					DataSourceUtils.setReadOnlyIfPossible(target);
 				}
 				if (this.transactionIsolation != null &&
 						!this.transactionIsolation.equals(defaultTransactionIsolation())) {
-					this.target.setTransactionIsolation(this.transactionIsolation);
+					target.setTransactionIsolation(this.transactionIsolation);
 				}
 				if (this.autoCommit != null && this.autoCommit != defaultAutoCommit()) {
-					this.target.setAutoCommit(this.autoCommit);
+					target.setAutoCommit(this.autoCommit);
 				}
 			}
-
-			else {
-				// Target Connection already held -> return it.
-				if (logger.isTraceEnabled()) {
-					logger.trace("Using existing database connection for operation '" + operation.getName() + "'");
+			catch (Throwable settingsEx) {
+				logger.debug("Failed to apply transaction settings to JDBC Connection", settingsEx);
+				// Close Connection and do not set it as target.
+				try {
+					target.close();
+				}
+				catch (Throwable closeEx) {
+					logger.debug("Could not close JDBC Connection after failed settings", closeEx);
 				}
+				throw settingsEx;
 			}
 
-			return this.target;
+			this.target = target;
+			return target;
 		}
 
 		private DataSource getDataSourceToUse() {