From 6292dc816af736e94026a8fe59d1e7e654873cc3 Mon Sep 17 00:00:00 2001 From: Alex Panchenko Date: Thu, 16 Jan 2020 10:53:21 +0200 Subject: [PATCH 1/2] tomcat-jdbc check if returned connection is closed --- .../tomcat/jdbc/pool/ConnectionPool.java | 6 +++++ .../tomcat/jdbc/test/TestValidation.java | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java index 99a9face827b..4d1aabf08bff 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ConnectionPool.java @@ -911,6 +911,12 @@ protected boolean terminateTransaction(PooledConnection con) { protected boolean shouldClose(PooledConnection con, int action) { if (con.getConnectionVersion() < getPoolVersion()) return true; if (con.isDiscarded()) return true; + try { + if (con.isClosed()) return true; + } catch (SQLException e) { + log.warn("Unable to check if connection is closed", e); + return true; + } if (isClosed()) return true; if (!con.validate(action)) return true; if (!terminateTransaction(con)) return true; diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java index a557b82e2c7e..61955d5d8246 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java @@ -19,6 +19,7 @@ import java.sql.Connection; import java.sql.DriverManager; import java.sql.DriverPropertyInfo; +import java.sql.ResultSet; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; import java.sql.Savepoint; @@ -141,6 +142,29 @@ public void testOnConnectValidationWithValidationSQLDoesNotOccurWhenDisabled() t Assert.assertFalse("No transaction must be running after connection is obtained", getMock(cxn).isRunningTransaction()); } + @Test + public void returnClosedConnection() throws SQLException { + datasource.setDriverClassName("org.h2.Driver"); + datasource.setUrl("jdbc:h2:~/.h2/test;QUERY_TIMEOUT=0;DB_CLOSE_ON_EXIT=FALSE"); + Assert.assertFalse(datasource.getPoolProperties().isTestOnBorrow()); + Assert.assertFalse(datasource.getPoolProperties().isTestOnReturn()); + Assert.assertFalse(datasource.getPoolProperties().isTestWhileIdle()); + try (Connection connection = datasource.getConnection()) { + // this gets the real connection and closes it to simulate close by driver because of I/O error + final Connection realConnection = connection.getMetaData().getConnection(); + Assert.assertNotSame(connection, realConnection); + realConnection.close(); + } + try (Connection connection = datasource.getConnection()) { + try (Statement statement = connection.createStatement()) { + try (ResultSet resultSet = statement.executeQuery("select 1")) { + Assert.assertTrue(resultSet.next()); + Assert.assertEquals(1, resultSet.getInt(1)); + } + } + } + } + @Test public void testOnConnectValidationSuccessWithValidationQueryAndAutoCommitEnabled() throws SQLException { checkOnConnectValidationWithOutcome(ValidationOutcome.SUCCESS, WITHVALIDATIONQUERY, WITHAUTOCOMMIT); From 4a5420f127661cebb6dc48671a67716a589f0bd0 Mon Sep 17 00:00:00 2001 From: Alex Panchenko <440271+panchenko@users.noreply.github.com> Date: Sat, 18 Oct 2025 14:13:45 +0200 Subject: [PATCH 2/2] test review --- .../tomcat/jdbc/test/TestValidation.java | 26 +++++++++---------- .../tomcat/jdbc/test/driver/Connection.java | 4 ++- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java index f1b351e1eebc..28e3c08cb8f8 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/TestValidation.java @@ -19,7 +19,6 @@ import java.sql.Connection; import java.sql.DriverManager; import java.sql.DriverPropertyInfo; -import java.sql.ResultSet; import java.sql.SQLException; import java.sql.SQLFeatureNotSupportedException; import java.sql.Savepoint; @@ -29,6 +28,7 @@ import javax.sql.PooledConnection; +import org.apache.tomcat.jdbc.pool.ConnectionPool; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -144,25 +144,23 @@ public void testOnConnectValidationWithValidationSQLDoesNotOccurWhenDisabled() t @Test public void returnClosedConnection() throws SQLException { - datasource.setDriverClassName("org.h2.Driver"); - datasource.setUrl("jdbc:h2:~/.h2/test;QUERY_TIMEOUT=0;DB_CLOSE_ON_EXIT=FALSE"); + ConnectionPool pool = datasource.createPool(); + pool.resetStats(); Assert.assertFalse(datasource.getPoolProperties().isTestOnBorrow()); Assert.assertFalse(datasource.getPoolProperties().isTestOnReturn()); Assert.assertFalse(datasource.getPoolProperties().isTestWhileIdle()); try (Connection connection = datasource.getConnection()) { - // this gets the real connection and closes it to simulate close by driver because of I/O error - final Connection realConnection = connection.getMetaData().getConnection(); + Assert.assertEquals("size", 1, pool.getSize()); + Connection realConnection = ((PooledConnection) connection).getConnection(); Assert.assertNotSame(connection, realConnection); realConnection.close(); + Assert.assertTrue(realConnection.isClosed()); + Assert.assertFalse(connection.isClosed()); } - try (Connection connection = datasource.getConnection()) { - try (Statement statement = connection.createStatement()) { - try (ResultSet resultSet = statement.executeQuery("select 1")) { - Assert.assertTrue(resultSet.next()); - Assert.assertEquals(1, resultSet.getInt(1)); - } - } - } + Assert.assertEquals("borrowed", 1, pool.getBorrowedCount()); + Assert.assertEquals("returned", 1, pool.getReturnedCount()); + Assert.assertEquals("released", 1, pool.getReleasedCount()); + Assert.assertEquals("size", 0, pool.getSize()); } @Test @@ -668,4 +666,4 @@ public boolean execute(String sql) throws SQLException { } } -} \ No newline at end of file +} diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java index 55ca60acff0e..afa577052bf8 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/test/driver/Connection.java @@ -38,6 +38,7 @@ public class Connection implements java.sql.Connection { Properties info; + private boolean closed = false; public Connection(Properties info) { this.info = info; @@ -57,6 +58,7 @@ public void clearWarnings() throws SQLException { @Override public void close() throws SQLException { + closed = true; Driver.disconnectCount.incrementAndGet(); } @@ -156,7 +158,7 @@ public SQLWarning getWarnings() throws SQLException { @Override public boolean isClosed() throws SQLException { - return false; + return closed; } @Override