Skip to content

Commit

Permalink
Validation query not timing out on connections managed by
Browse files Browse the repository at this point in the history
PerUserPoolDataSource
  • Loading branch information
garydgregory committed Dec 15, 2024
1 parent a82b6db commit 4f051f8
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 56 deletions.
1 change: 1 addition & 0 deletions src/changes/changes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ The <action> type attribute can be add,update,fix,remove.
<release version="2.13.1" date="YYYY-MM-DD" description="This is a minor release, including bug fixes and enhancements.">
<!-- FIX -->
<action type="fix" issue="DBCP-597" dev="ggregory" due-to="Xiaotian Bai, Raju Gupta, Gary Gregory">Validation query not timing out on connections managed by SharedPoolDataSource.</action>
<action type="fix" issue="DBCP-597" dev="ggregory" due-to="Gary Gregory">Validation query not timing out on connections managed by PerUserPoolDataSource.</action>
<action type="fix" issue="DBCP-597" dev="ggregory" due-to="Gary Gregory">KeyedCPDSConnectionFactory.validateObject(UserPassKey, PooledObject) does ignores timeouts less than 1 second when there is no validation query.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Modernize tests to use JUnit 5 features.</action>
<!-- ADD -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,14 @@
package org.apache.commons.dbcp2.datasources;

import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.time.Duration;

import javax.sql.ConnectionEvent;
import javax.sql.ConnectionEventListener;
import javax.sql.ConnectionPoolDataSource;
import javax.sql.PooledConnection;

import org.apache.commons.dbcp2.Utils;
import org.apache.commons.pool2.ObjectPool;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.PooledObjectFactory;
Expand Down Expand Up @@ -280,57 +277,4 @@ public synchronized String toString() {
builder.append("]");
return builder.toString();
}

@Override
public boolean validateObject(final PooledObject<PooledConnectionAndInfo> pooledObject) {
try {
validateLifetime(pooledObject);
} catch (final Exception e) {
return false;
}
boolean valid = false;
final PooledConnection pconn = pooledObject.getObject().getPooledConnection();
Connection conn = null;
validatingSet.add(pconn);
if (null == validationQuery) {
Duration timeoutDuration = validationQueryTimeoutDuration;
if (timeoutDuration.isNegative()) {
timeoutDuration = Duration.ZERO;
}
try {
conn = pconn.getConnection();
valid = conn.isValid((int) timeoutDuration.getSeconds());
} catch (final SQLException e) {
valid = false;
} finally {
Utils.closeQuietly((AutoCloseable) conn);
validatingSet.remove(pconn);
}
} else {
Statement stmt = null;
ResultSet rset = null;
// logical Connection from the PooledConnection must be closed
// before another one can be requested and closing it will
// generate an event. Keep track so we know not to return
// the PooledConnection
validatingSet.add(pconn);
try {
conn = pconn.getConnection();
stmt = conn.createStatement();
rset = stmt.executeQuery(validationQuery);
valid = rset.next();
if (rollbackAfterValidation) {
conn.rollback();
}
} catch (final Exception e) {
valid = false;
} finally {
Utils.closeQuietly((AutoCloseable) rset);
Utils.closeQuietly((AutoCloseable) stmt);
Utils.closeQuietly((AutoCloseable) conn);
validatingSet.remove(pconn);
}
}
return valid;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.time.Duration;
import java.util.HashMap;
import java.util.Map;
import java.util.NoSuchElementException;

import javax.sql.DataSource;

Expand Down Expand Up @@ -167,6 +168,26 @@ public void testClosingWithUserName() throws Exception {
}
}

/**
* Tests https://issues.apache.org/jira/browse/DBCP-597
*/
@Test
public void testDbcp597() throws SQLException {
PerUserPoolDataSource tds = (PerUserPoolDataSource) ds;
tds.setDefaultTestOnBorrow(true);
tds.setValidationQuery("SELECT 1");
// The tester statement throws a SQLTimeoutException when the timeout is > 0 and < 5.
tds.setValidationQueryTimeout(Duration.ofSeconds(1));
// The SQLTimeoutException is lost for now
SQLException e = assertThrows(SQLException.class, tds::getConnection);
assertEquals(NoSuchElementException.class, e.getCause().getClass());
// timeout > 0 and < 1
tds.setValidationQueryTimeout(Duration.ofMillis(999));
// The SQLTimeoutException is lost for now
e = assertThrows(SQLException.class, tds::getConnection);
assertEquals(NoSuchElementException.class, e.getCause().getClass());
}

// see issue https://issues.apache.org/bugzilla/show_bug.cgi?id=23843
@Test
public void testDefaultUser1() throws Exception {
Expand Down

0 comments on commit 4f051f8

Please sign in to comment.