This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 768f346f07972d7bb33c7f09bd5fdaba0b2bf7dd Author: opencode <[email protected]> AuthorDate: Sat Oct 31 07:32:17 2026 +0100 Scope the tracked-list promise to closes the handle sees StatementFinalizer's class javadoc promised the tracked list stays "bounded by the statements that are genuinely still open". The removal that keeps it bounded fires when a close() passes through the handle this interceptor returns, and a close that reaches the statement by another route is not one the mechanism can observe: a close performed on the driver handle that unwrap() hands back never passes through the proxy, and an outer interceptor can swallow the close it forwards. Such an entry - and, with trace on, the creation stack it holds - then simply survives until the connection return reclaims the list, where the renewed close meets a statement already closed as a harmless no-op. That is not a leak: the retention is bounded by the borrow, not by the life of the statement. Nothing in the mechanism can honestly promise more, so the claim is scoped to what it guarantees: the list is bounded by the statements that are genuinely still open and closed through that handle, with the out-of-band routes named. A new test pins the documented posture from both sides - a close through the unwrapped driver handle leaves the entry until the return reclaims it, while a close through a result set's getStatement() handle, which the always-on statement facade routes back through the pool, retires the entry the moment it lands. --- .../jdbc/pool/interceptor/StatementFinalizer.java | 9 ++-- .../pool/interceptor/TestStatementFinalizer.java | 50 +++++++++++++++++++++- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java index 55d217c7db..dd4bafa1a7 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementFinalizer.java @@ -41,9 +41,12 @@ import org.apache.tomcat.jdbc.pool.PooledConnection; * exactly the case the interceptor exists for, so finalisation must not depend on that statement still being reachable * when the connection is returned. Statements that the application does close are forgotten by the returned proxy at * the moment <code>close()</code> completes, keeping the tracked list bounded by the statements that are genuinely - * still open. (An interceptor placed outside this one may swallow the statement's <code>close()</code>, e.g. a - * statement cache recycling instead of closing; such entries are simply reclaimed at the connection return, as they - * were before.) + * still open and closed through that handle. (The removal cannot observe a close that reaches the statement by + * another route: an interceptor placed outside this one may swallow the statement's <code>close()</code>, e.g. a + * statement cache recycling instead of closing, and a statement the application closes through the driver handle + * that <code>unwrap</code> hands back never passes through this proxy; such entries are simply reclaimed at the + * connection return, where the renewed close meets a statement already closed as a harmless no-op, as they were + * before.) * <p> * Tracked statements are handed to the application as proxies implementing the standard JDBC statement interfaces, as * is the statement facade the pool always installs in front of this interceptor and as are the proxies of the other diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java index 2c35237099..2be6079dc4 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementFinalizer.java @@ -37,8 +37,8 @@ import org.apache.tomcat.jdbc.pool.DataSource; * What {@code StatementFinalizer} owes the application from the other * side of the coin: statements the application <em>did</em> close must be * forgotten the moment they are closed (the tracked list is bounded by - * genuinely open statements, not by everything the borrow ever created), - * and statements it <em>did not</em> close must survive garbage collection + * genuinely open statements closed through its handle, not by everything + * the borrow ever created), and statements it <em>did not</em> close must survive garbage collection * so they can still be closed when the connection is returned. What it owes * on top of that is a handle that answers {@code equals} and * {@code hashCode} on itself, like every other statement handle the pool @@ -278,6 +278,52 @@ class TestStatementFinalizer extends JdbcPoolBaseTest { } } + @Test + void aStatementClosedThroughTheUnwrappedDriverHandleLingersUntilTheReturn() throws Exception { + DataSource ds = finalizerPool(); + ds.createPool(); + + Connection pooled = ds.getConnection(); + StatementFinalizer finalizer = interceptorOf(pooled, StatementFinalizer.class); + Assertions.assertNotNull(finalizer); + + // The result set back door stays on the handled route: the facade + // the pool always installs answers getStatement() with the pooled + // handle, so closing through it still passes through the tracking + // proxy, and the entry is forgotten right there. + Statement viaResultSet = pooled.createStatement(); + try (ResultSet rs = viaResultSet.executeQuery("SELECT 1")) { + rs.next(); + rs.getStatement().close(); + } + Assertions.assertEquals(0, tracked(finalizer), + "a close through a result set's handle must still be noticed"); + + // The unwrap route leaves the handled path: the driver's own + // statement takes the close, and the tracking proxy - the one place + // the entry is removed - never sees it. The entry cannot observe a + // close performed through another handle, so the tracked list is + // bounded only by the statements closed through the handle this + // interceptor returns, and this one lingers until the return. + Statement viaUnwrap = pooled.createStatement(); + Statement driver = driverStatement(viaUnwrap); + driver.close(); + Assertions.assertTrue(driver.isClosed(), + "precondition: the driver's own statement is closed"); + Assertions.assertEquals(1, tracked(finalizer), + "a close that bypassed the handle must not be noticed as removal"); + + // The return reclaims the stale entry, and the renewed close attempt + // there meets a statement that is already closed: a harmless no-op + // that must neither fail the return nor leave the entry behind. + Statement trackedStatement = trackedStatement(trackedEntries(finalizer).get(0)); + pooled.close(); + Assertions.assertEquals(0, tracked(finalizer), + "the connection return must clear the stale entry"); + Assertions.assertTrue(trackedStatement.isClosed(), + "the statement behind the stale entry is closed all the same"); + } + @Test void twoTrackingHandlesOnOneStatementDoNotCompareEqual() throws Exception { DataSource ds = finalizerPool(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
