Skip to content

Commit 24df35c

Browse files
committed
Do not keep target connection after failed settings
Includes aligned setReadOnly exception suppression. Closes gh-35980 (cherry picked from commit ab33000)
1 parent 3b1fa36 commit 24df35c

File tree

2 files changed

+63
-44
lines changed

2 files changed

+63
-44
lines changed

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

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -202,24 +202,10 @@ static Integer prepareConnectionForTransaction(Connection con, int isolationLeve
202202
boolean debugEnabled = logger.isDebugEnabled();
203203
// Set read-only flag.
204204
if (setReadOnly) {
205-
try {
206-
if (debugEnabled) {
207-
logger.debug("Setting JDBC Connection [" + con + "] read-only");
208-
}
209-
con.setReadOnly(true);
210-
}
211-
catch (SQLException | RuntimeException ex) {
212-
Throwable exToCheck = ex;
213-
while (exToCheck != null) {
214-
if (exToCheck.getClass().getSimpleName().contains("Timeout")) {
215-
// Assume it's a connection timeout that would otherwise get lost: for example, from JDBC 4.0
216-
throw ex;
217-
}
218-
exToCheck = exToCheck.getCause();
219-
}
220-
// "read-only not supported" SQLException -> ignore, it's just a hint anyway
221-
logger.debug("Could not set JDBC Connection read-only", ex);
205+
if (debugEnabled) {
206+
logger.debug("Setting JDBC Connection [" + con + "] read-only");
222207
}
208+
setReadOnlyIfPossible(con);
223209
}
224210

225211
// Apply specific isolation level, if any.
@@ -238,6 +224,31 @@ static Integer prepareConnectionForTransaction(Connection con, int isolationLeve
238224
return previousIsolationLevel;
239225
}
240226

227+
/**
228+
* Apply the read-only hint to the given Connection,
229+
* suppressing exceptions other than timeout-related ones.
230+
* @param con the Connection to prepare
231+
* @throws SQLException in case of a timeout exception
232+
* @since 6.2.15
233+
*/
234+
static void setReadOnlyIfPossible(Connection con) throws SQLException {
235+
try {
236+
con.setReadOnly(true);
237+
}
238+
catch (SQLException | RuntimeException ex) {
239+
Throwable exToCheck = ex;
240+
while (exToCheck != null) {
241+
if (exToCheck.getClass().getSimpleName().contains("Timeout")) {
242+
// Assume it's a connection timeout that would otherwise get lost: for example, from JDBC 4.0
243+
throw ex;
244+
}
245+
exToCheck = exToCheck.getCause();
246+
}
247+
// "read-only not supported" SQLException -> ignore, it's just a hint anyway
248+
logger.debug("Could not set JDBC Connection read-only", ex);
249+
}
250+
}
251+
241252
/**
242253
* Reset the given Connection after a transaction,
243254
* regarding read-only flag and isolation level.

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

Lines changed: 35 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -471,48 +471,56 @@ private boolean hasTargetConnection() {
471471
/**
472472
* Return the target Connection, fetching it and initializing it if necessary.
473473
*/
474-
private Connection getTargetConnection(Method operation) throws SQLException {
475-
if (this.target == null) {
476-
// No target Connection held -> fetch one.
474+
private Connection getTargetConnection(Method operation) throws Throwable {
475+
Connection target = this.target;
476+
if (target != null) {
477+
// Target Connection already held -> return it.
477478
if (logger.isTraceEnabled()) {
478-
logger.trace("Connecting to database for operation '" + operation.getName() + "'");
479+
logger.trace("Using existing database connection for operation '" + operation.getName() + "'");
479480
}
481+
return target;
482+
}
480483

481-
// Fetch physical Connection from DataSource.
482-
DataSource dataSource = getDataSourceToUse();
483-
this.target = (this.username != null ? dataSource.getConnection(this.username, this.password) :
484-
dataSource.getConnection());
485-
if (this.target == null) {
486-
throw new IllegalStateException("DataSource returned null from getConnection(): " + dataSource);
487-
}
484+
// No target Connection held -> fetch one.
485+
if (logger.isTraceEnabled()) {
486+
logger.trace("Connecting to database for operation '" + operation.getName() + "'");
487+
}
488+
489+
// Fetch physical Connection from DataSource.
490+
DataSource dataSource = getDataSourceToUse();
491+
target = (this.username != null ? dataSource.getConnection(this.username, this.password) :
492+
dataSource.getConnection());
493+
if (target == null) {
494+
throw new IllegalStateException("DataSource returned null from getConnection(): " + dataSource);
495+
}
488496

489-
// Apply kept transaction settings, if any.
497+
// Apply kept transaction settings, if any.
498+
try {
490499
if (this.readOnly && readOnlyDataSource == null) {
491-
try {
492-
this.target.setReadOnly(true);
493-
}
494-
catch (Exception ex) {
495-
// "read-only not supported" -> ignore, it's just a hint anyway
496-
logger.debug("Could not set JDBC Connection read-only", ex);
497-
}
500+
DataSourceUtils.setReadOnlyIfPossible(target);
498501
}
499502
if (this.transactionIsolation != null &&
500503
!this.transactionIsolation.equals(defaultTransactionIsolation())) {
501-
this.target.setTransactionIsolation(this.transactionIsolation);
504+
target.setTransactionIsolation(this.transactionIsolation);
502505
}
503506
if (this.autoCommit != null && this.autoCommit != defaultAutoCommit()) {
504-
this.target.setAutoCommit(this.autoCommit);
507+
target.setAutoCommit(this.autoCommit);
505508
}
506509
}
507-
508-
else {
509-
// Target Connection already held -> return it.
510-
if (logger.isTraceEnabled()) {
511-
logger.trace("Using existing database connection for operation '" + operation.getName() + "'");
510+
catch (Throwable settingsEx) {
511+
logger.debug("Failed to apply transaction settings to JDBC Connection", settingsEx);
512+
// Close Connection and do not set it as target.
513+
try {
514+
target.close();
515+
}
516+
catch (Throwable closeEx) {
517+
logger.debug("Could not close JDBC Connection after failed settings", closeEx);
512518
}
519+
throw settingsEx;
513520
}
514521

515-
return this.target;
522+
this.target = target;
523+
return target;
516524
}
517525

518526
private DataSource getDataSourceToUse() {

0 commit comments

Comments
 (0)