This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 10.1.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4868db0e08e93c72ced0e334097256ecf43ec101 Author: opencode <[email protected]> AuthorDate: Fri Nov 6 09:59:10 2026 +0100 Stop exposing the internal pooled connection through Wrapper ProxyConnection advertised and returned the pool-internal PooledConnection class through the public java.sql.Wrapper contract, giving any connection holder access to credentials, the physical session and lifecycle mutators. Refuse the internal type through isWrapperFor and unwrap, while keeping the standard javax.sql.PooledConnection handle available. Tests now reach the internal object through the ProxyConnection chain. --- .../apache/tomcat/jdbc/pool/ProxyConnection.java | 199 +++------------------ .../org/apache/tomcat/jdbc/JdbcPoolBaseTest.java | 10 ++ .../jdbc/pool/TestIdleValidationHeadOfLine.java | 4 +- .../pool/TestMBeanDelegationWhenDisconnected.java | 2 +- .../tomcat/jdbc/pool/TestTestAllIdleClaim.java | 10 +- .../pool/TestThrowingDisconnectedInterceptor.java | 6 +- .../pool/TestThrowingDriverOnClaimedPaths.java | 4 +- .../jdbc/pool/TestThrowingResetInterceptor.java | 6 +- .../apache/tomcat/jdbc/pool/TestValidation.java | 2 +- .../tomcat/jdbc/pool/TestWrapperContract.java | 54 +++--- .../TestResetAbandonedTimerMBeanIdle.java | 2 +- .../TestStatementCacheCloseAfterDisposal.java | 2 +- .../TestStatementCacheInterceptorOrder.java | 2 +- .../TestStatementCachePurgeCloseFailure.java | 4 +- .../interceptor/TestStatementCacheRacingClose.java | 2 +- .../interceptor/TestStatementCacheRacingPark.java | 4 +- .../interceptor/TestStatementCacheReconnect.java | 6 +- .../TestStatementCacheStaleEntryResurrection.java | 4 +- .../pool/interceptor/TestStatementFinalizer.java | 2 +- .../jmx/TestDataSourceRegistrationVsClose.java | 2 +- 20 files changed, 92 insertions(+), 235 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java index 5eb6099a21..e3f4b68c23 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/ProxyConnection.java @@ -26,86 +26,34 @@ import javax.sql.XAConnection; import org.apache.tomcat.jdbc.pool.util.MessageFactory; /** - * A ProxyConnection object is the bottom most interceptor that wraps an object of type {@link PooledConnection}. The - * ProxyConnection intercepts, among the calls it forwards, the following methods: - * <ul> - * <li>{@link java.sql.Connection#close()} - returns the connection to the pool. May be called multiple times.</li> - * <li>{@link java.sql.Connection#abort(java.util.concurrent.Executor)} - delegates the abort to the physical - * connection and then ends the borrow: the session is taken out of service and the slot is freed. On a released - * handle it is the specification's no-op; on a handle whose session the pool discarded while borrowed it skips the - * driver and ends the borrow quietly.</li> - * <li>{@link java.lang.Object#toString()} - returns a custom string for this object</li> - * <li>{@link javax.sql.PooledConnection#getConnection()} - while the borrow is live, returns the underlying - * connection; a handle that reports itself closed - one whose session the pool discarded while it was still - * borrowed, which its borrower has already closed, or whose borrow ended before a call parked in the interceptor - * chain reached this interceptor while the session meanwhile went to the next borrower - answers it with the - * closed-connection failure rather than handing out the session the pool is retiring or re-served, or the null - * that session's field holds once disconnected.</li> - * <li>{@link java.lang.Object#equals(Object)} and {@link java.lang.Object#hashCode()} - answered with the identity of - * the connection handle, so that the {@code Object} contract holds even when no facade sits in front of this - * interceptor</li> - * </ul> - * By default method names are compared with {@link String#equals(Object)}; - * {@link PoolConfiguration#setUseEquals(boolean)} called with a <code>false</code> argument switches to - * reference comparison. + * The bottom-most interceptor, wrapping a {@link PooledConnection}. It intercepts {@link java.sql.Connection#close()}, + * {@link java.sql.Connection#abort(java.util.concurrent.Executor)}, {@link Object#toString()}, + * {@link javax.sql.PooledConnection#getConnection()}, {@link Object#equals(Object)} and {@link Object#hashCode()}. + * Method names are compared with {@link String#equals(Object)} unless {@link PoolConfiguration#setUseEquals(boolean)} + * selects reference comparison. * <p> - * The connection handle the pool hands out implements {@link javax.sql.PooledConnection} (and - * {@link javax.sql.XAConnection} when the pool is backed by an {@link javax.sql.XADataSource}). The pool keeps no - * connection or statement event listeners of its own, so the four listener methods of - * {@code javax.sql.PooledConnection} - {@code addConnectionEventListener}, {@code removeConnectionEventListener}, - * {@code addStatementEventListener} and {@code removeStatementEventListener} - are answered by the XA-family - * dispatch in {@link #invoke(Object, Method, Object[])}: where the handle is itself an {@code XAConnection} they - * reach the XA connection the driver created, whose implementation decides what registering them means; where it - * is not, the call has no dispatch target and each of the four fails with an {@link SQLException} naming the - * method. That refusal is deliberate: the specification has the pool manager register through these methods to be - * told to deactivate the handle and reuse or retire the physical session - decisions this pool makes and carries - * out itself, with a new handle per borrow - and accepting a registration it could not notify faithfully would - * only hide from the caller that no notification will ever arrive. The refusal also stands on a pool backed by a - * {@link javax.sql.ConnectionPoolDataSource}, which holds the driver's own pooled connection for its session - * management and does not route these calls to it. A handle that reports itself closed answers these calls with the closed-connection - * failure every dispatched call meets in that state; the two boolean liveness probes are the exception, reporting - * the state rather than failing - {@code isClosed} true and {@code isValid} false. Since the interface declares no - * checked exception, either failure reaches the caller as an {@link java.lang.reflect.UndeclaredThrowableException} - * carrying it. + * The connection handle implements {@link javax.sql.PooledConnection} (and {@link javax.sql.XAConnection} for an XA + * pool). The pool manages session reuse itself, so the four listener methods are delegated to the driver's XA + * connection where one exists, and otherwise fail with an {@link SQLException} naming the method rather than accepting + * a notification the pool could never deliver faithfully. */ public class ProxyConnection extends JdbcInterceptor { /** - * The underlying pooled connection. Volatile because the writes that end or re-arm a borrow - the seal a - * concurrent {@link java.sql.Connection#close() close} or - * {@link java.sql.Connection#abort(java.util.concurrent.Executor) abort} makes from another thread, and the - * {@link #reset(ConnectionPool, PooledConnection)} of a pool walk - have no synchronization on the reader's - * side, and the guard in {@link #invoke(Object, Method, Object[])} consults this field to decide whether the - * handle can still serve. A seal seen here ends a borrow that was in progress; what it cannot show is a borrow - * that already ended and was followed by a clean recycle, which re-arms this field with the very same pooled - * connection object - the fence for that state is the borrow handle, documented on {@link #borrowHandle}. + * The underlying pooled connection. Volatile because writes that end or re-arm a borrow have no synchronization on + * the reader's side. */ protected volatile PooledConnection connection = null; /** - * The connection handle the pool handed to the borrow this interceptor is currently armed for, or - * <code>null</code> once the borrow has ended: the close and abort branches of - * {@link #invoke(Object, Method, Object[])} seal it where they end a borrow, and - * {@link #reset(ConnectionPool, PooledConnection)} clears it and holds it clear until - * {@link ConnectionPool#setupConnection(PooledConnection)} records the next borrow's handle. This interceptor is - * cached on the pooled connection and a clean recycle re-arms it with the same {@link PooledConnection} object - * the ended borrow held, so neither the session field nor the discarded flag can tell a call of the current - * borrow from one that entered the interceptor chain before the borrow ended and descends only after the - * session went to the next borrower - every state those reads see then is the next borrow's live state. The - * handle is the per-borrow identity the call still carries: the pool builds a new handle for every borrow and - * the receiver travels down the chain unchanged, so a call arriving on any other handle belongs to a borrow - * that is over, and {@link #invoke(Object, Method, Object[])} answers it the way an ended borrow answers, - * whatever state the session itself is in. + * The connection handle the current borrow was served under, or {@code null} once the borrow has ended. It is the + * per-borrow identity that distinguishes a call of the current borrow from a late call of an earlier one. */ protected volatile Object borrowHandle = null; /** - * The parent connection pool. Volatile for the same reason and by the same lock-free readers as the session - * field above: the close and abort branches of {@link #invoke(Object, Method, Object[])} read it into a local - * the whole branch acts on, and {@link #reset(ConnectionPool, PooledConnection)} nulls it on a teardown walk. - * Volatile also makes the ordering that walk relies on hold: the reset writes the session before the pool, so - * those branches - which read the pool first and the session second - never see a nulled pool with a live - * session behind it, the state a late close could otherwise pass its guard on and then dereference. + * The parent connection pool. Volatile because the close and abort branches read it without synchronization; reset + * writes the session before the pool so a reader never sees a null pool with a live session behind it. */ protected volatile ConnectionPool pool = null; @@ -218,39 +166,14 @@ public class ProxyConnection extends JdbcInterceptor { * @return true if this interceptor wraps the given interface */ public boolean isWrapperFor(Class<?> iface) { - // The pooled connection is read once, into a snapshot, the way the - // dispatch in invoke() reads it, so the whole answer is about one - // state of the handle. - // - // No receiver is passed down: this entry point answers about the - // interceptor itself, which is a handle to nobody - a direct call - // carries no borrow identity for the fence's handle leg to consult, - // and the interfaces a dispatched call's handle implements are the - // dispatch's answer to give, not this one's (TestWrapperContract - // pins the two entry points' per-object pair for - // javax.sql.PooledConnection). + // Answer about one snapshot of the handle, with no borrow receiver. return isWrapperFor(iface, connection, null); } /* - * The isWrapperFor answer for one snapshot of the session field. The - * dispatch in invoke() calls this with the snapshot its guard and its - * re-check approved; the public method above takes its own single read - * and answers through here as well, with no receiver. - * - * The fence is the guard's closed test re-run immediately before the - * answer is read off the snapshot, the way every handout branch of - * invoke() re-checks before the handout: a snapshot outlives the check - * made on it, and a discard marking changes the state under one without - * touching the field - the abandon, the forced close and an - * interceptor's setDiscarded(true) all write that flag first and reach - * the seal of the field only later. A handle that reports itself closed - * answers false - a boolean cannot carry the closed failure, and false - * keeps the pair agreed with the failure unwrap() raises from the same - * fence. The receiver joins the fence as the guard's own handle test, - * for the clean recycle that re-armed the tail with the same session - * after the caller's re-check; a call with no receiver has no borrow - * identity to consult and skips that leg. + * The isWrapperFor answer for one snapshot of the session field. The fence re-checks that the snapshot still + * describes this borrow before answering, because a discard marking can move the state without touching the + * field. A closed handle answers false, matching the closed failure unwrap() raises. */ private boolean isWrapperFor(Class<?> iface, PooledConnection pc, Object proxy) { if (pc != null && (connection != pc || pc.isDiscarded() @@ -265,13 +188,6 @@ public class ProxyConnection extends JdbcInterceptor { // same interface hands back from that same state. return true; } - if (iface == PooledConnection.class) { - // The Wrapper answer must agree with what unwrap() hands out: - // for this class unwrap() returns the pooled connection this - // interceptor sits on, so the honest answer is whether one is - // still held. That object is pool-internal - see unwrap(). - return pc != null; - } if (pc == null) { // The borrow has ended and nothing is held, so no interface is // provided - the same false, and matching not-a-wrapper failure @@ -315,90 +231,38 @@ public class ProxyConnection extends JdbcInterceptor { * * @param iface the interface to unwrap to * - * @return the unwrapped connection. For {@link PooledConnection} this is - * the pool's own pooled connection - a pool-internal object meant - * for trusted callers such as interceptors, not for application - * code, as it can close and discard the connection behind the - * pool's own accounting. + * @return the unwrapped connection. The pool-internal {@link PooledConnection} type is not exposed through the + * public Wrapper contract. * * @throws SQLException if the connection does not wrap the given interface */ public Object unwrap(Class<?> iface) throws SQLException { - // The pooled connection is read once, into a snapshot, the way - // isWrapperFor() above and the dispatch in invoke() read it, so the - // whole answer is about one state of the handle. As in - // isWrapperFor(), no receiver is passed down: the direct answer is - // the interceptor's own, and it has no borrow identity for the - // fence's handle leg to run on. + // Answer about one snapshot of the handle, with no borrow receiver. return unwrap(iface, connection, null); } /* - * The unwrap answer for one snapshot of the session field, taken like - * the one isWrapperFor() above answers from. - * - * The fence is the guard's closed test re-run immediately before this - * method reads anything off the snapshot - the way every handout branch - * of invoke() re-checks before the handout. The discard marking is the - * writer that moves that state without touching the field: it is - * written before the seal the field checks can show. The receiver joins - * the fence as the guard's handle test; a call without a receiver skips - * that leg. The refusal is the closed failure every use of such a - * handle gets, not the not-a-wrapper failure: the interface asked is - * not the problem, the borrow is, and a caller told "not a wrapper" - * would be invited to ask again for another interface of the same gone - * session. It is also the answer isWrapperFor() gives false for from - * the same fence, so the pair stays agreed on this state. + * The unwrap answer for one snapshot of the session field. The fence re-checks the snapshot before anything is + * read off it; a sealed handle gets the closed failure rather than not-a-wrapper, since the borrow is the problem. */ private Object unwrap(Class<?> iface, PooledConnection pc, Object proxy) throws SQLException { if (pc != null && (connection != pc || pc.isDiscarded() || (proxy != null && !servesHandle(proxy)))) { throw new SQLException(MessageFactory.get("proxyConnection.connectionClosed")); } - if (iface == PooledConnection.class) { - if (pc == null) { - // The same not-a-wrapper failure the XA case below gives for - // a missing XA connection: with none held there is nothing to - // hand out, and a silent null would contradict what - // isWrapperFor() says and surface as an unrelated - // NullPointerException at the caller's first use. - throw new SQLException(MessageFactory.get("proxyConnection.notAWrapper", iface.getName())); - } - return pc; - } if (pc == null) { - // Nothing is held, so no interface is provided - the XA - // connection and the physical connection among them - and the - // contract's answer is this failure, the refusal isWrapperFor() - // predicts for this state, so the pair stays agreed on a - // released interceptor. throw new SQLException(MessageFactory.get("proxyConnection.notAWrapper", iface.getName())); } if (proxy != null && iface != XAConnection.class && iface.isInstance(proxy) && !iface.isInstance(pc.getConnection())) { - // The Wrapper contract: the handle is a wrapper of every - // interface it implements itself. Interfaces the physical - // connection behind it also provides - java.sql.Connection - // first of all - keep resolving to the physical connection; - // javax.sql.XAConnection is excluded because unwrapping to it - // has always yielded the XA connection object the pooled - // connection holds - the driver's handle, wrapped by the pool's - // branch tracker on an XA session - and the failure below where - // none does. This answer - // runs below the fence like every other handout here: above it, - // a marking or a seal landing with the caller's re-check left - // could hand the handle out on a borrow the pool was already - // retiring, disagreeing with the closed failure the fenced - // answers gave. The proxy handed back is the caller's own - // handle, inert once the borrow is over. + // The handle is a wrapper of every interface it implements itself. Interfaces the physical connection + // also provides keep resolving below; XAConnection is excluded because unwrapping to it yields the + // driver's XA connection. The returned proxy is the caller's own handle, inert once the borrow is over. return proxy; } if (iface == XAConnection.class) { javax.sql.XAConnection xa = pc.getXAConnection(); if (xa == null) { - // The Wrapper contract asks for a failure, not for a silent - // null that turns into an unrelated NullPointerException at - // the caller's first use of the "unwrapped" value. throw new SQLException(MessageFactory.get("proxyConnection.notAWrapper", iface.getName())); } return xa; @@ -408,15 +272,6 @@ public class ProxyConnection extends JdbcInterceptor { return physical; } if (physical != null && isWrapperFor(iface, pc, null)) { - // The driver's connection is a wrapper further down, and - // unwrapping that far is its answer to give, not ours. The - // question is asked of the same physical connection that was - // just tested - not of a fresh read of the field - so the answer - // never describes a state later than this method's fence, and it - // is asked with no receiver: it is about the driver's connection, - // not about this handle's own interfaces. Where the snapshot - // holds no session there is nothing below to ask, and the - // failure below is the honest answer for that state. return physical.unwrap(iface); } throw new SQLException(MessageFactory.get("proxyConnection.notAWrapper", iface.getName())); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/JdbcPoolBaseTest.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/JdbcPoolBaseTest.java index 5da1bc90cf..66f785bfef 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/JdbcPoolBaseTest.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/JdbcPoolBaseTest.java @@ -37,6 +37,7 @@ import org.apache.tomcat.jdbc.pool.ConnectionPool; import org.apache.tomcat.jdbc.pool.DataSource; import org.apache.tomcat.jdbc.pool.JdbcInterceptor; import org.apache.tomcat.jdbc.pool.PooledConnection; +import org.apache.tomcat.jdbc.pool.ProxyConnection; import org.h2.tools.Server; /** @@ -166,6 +167,15 @@ public abstract class JdbcPoolBaseTest { return null; } + /* + * The pool-internal PooledConnection behind a borrowed handle. It is no + * longer reachable through the public Wrapper contract, so tests that need + * it reach it through the ProxyConnection at the bottom of the chain. + */ + protected static PooledConnection internalOf(Connection pooled) { + return interceptorOf(pooled, ProxyConnection.class).getConnection(); + } + /* * An interceptor that always throws from disconnected(), shared by the * tests that pin how the disconnect path isolates a misbehaving link of diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestIdleValidationHeadOfLine.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestIdleValidationHeadOfLine.java index 6c05d5d454..a183222361 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestIdleValidationHeadOfLine.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestIdleValidationHeadOfLine.java @@ -108,8 +108,8 @@ class TestIdleValidationHeadOfLine extends JdbcPoolBaseTest { Connection second = ds.getConnection(); setSessionMarker(head, 11); setSessionMarker(second, 22); - PooledConnection headPc = head.unwrap(PooledConnection.class); - PooledConnection secondPc = second.unwrap(PooledConnection.class); + PooledConnection headPc = internalOf(head); + PooledConnection secondPc = internalOf(second); long now = System.currentTimeMillis(); // Gate both connections' idle validation for the next minute: no // sweep can touch either with its validation while the test sets diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestMBeanDelegationWhenDisconnected.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestMBeanDelegationWhenDisconnected.java index 7c4b9b75b4..a5070470c7 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestMBeanDelegationWhenDisconnected.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestMBeanDelegationWhenDisconnected.java @@ -98,7 +98,7 @@ class TestMBeanDelegationWhenDisconnected extends JdbcPoolBaseTest { DataSource ds = newDataSource(); Connection handle = ds.getConnection(); - PooledConnection pooled = handle.unwrap(PooledConnection.class); + PooledConnection pooled = internalOf(handle); Assertions.assertFalse(pooled.isClosed()); Assertions.assertTrue(pooled.getAutoCommit()); Assertions.assertNotNull(pooled.getCatalog()); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestTestAllIdleClaim.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestTestAllIdleClaim.java index 5af5e3d010..040a8ccc94 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestTestAllIdleClaim.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestTestAllIdleClaim.java @@ -119,7 +119,7 @@ class TestTestAllIdleClaim extends JdbcPoolBaseTest { Connection first = ds.getConnection(); awaitSessions(1); setSessionMarker(first, 7); - PooledConnection pc = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pc = (PooledConnection) internalOf(first); first.close(); forceExpired(pc); @@ -193,7 +193,7 @@ class TestTestAllIdleClaim extends JdbcPoolBaseTest { // the marker set on the old session is gone Connection again = handed.get(); Assertions.assertNotNull(again); - Assertions.assertSame(pc, again.unwrap(PooledConnection.class)); + Assertions.assertSame(pc, internalOf(again)); Assertions.assertNull(sessionMarker(again)); // and nothing was stranded: two physical connects (fill and @@ -218,7 +218,7 @@ class TestTestAllIdleClaim extends JdbcPoolBaseTest { Connection first = ds.getConnection(); awaitSessions(1); - PooledConnection pc = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pc = (PooledConnection) internalOf(first); first.close(); forceExpired(pc); @@ -273,7 +273,7 @@ class TestTestAllIdleClaim extends JdbcPoolBaseTest { Connection first = ds.getConnection(); awaitSessions(1); - PooledConnection pc = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pc = (PooledConnection) internalOf(first); first.close(); forceExpired(pc); @@ -292,7 +292,7 @@ class TestTestAllIdleClaim extends JdbcPoolBaseTest { mode = DIRECT; try (Connection again = ds.getConnection()) { Assertions.assertNotNull(again); - Assertions.assertNotSame(pc, again.unwrap(PooledConnection.class)); + Assertions.assertNotSame(pc, internalOf(again)); } Assertions.assertEquals(1, ds.getPool().getIdle()); awaitSessions(1); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDisconnectedInterceptor.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDisconnectedInterceptor.java index 107f4a9b40..c22afca991 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDisconnectedInterceptor.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDisconnectedInterceptor.java @@ -59,8 +59,8 @@ class TestThrowingDisconnectedInterceptor extends JdbcPoolBaseTest { // test: an abandoned, unclosed physical connection is otherwise // reclaimed by the JDK's socket cleaner and the leaked database // sessions would disappear with GC timing instead of staying open - java.sql.Connection physicalFirst = first.unwrap(PooledConnection.class).getConnection(); - java.sql.Connection physicalSecond = second.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection physicalFirst = internalOf(first).getConnection(); + java.sql.Connection physicalSecond = internalOf(second).getConnection(); awaitSessions(2); first.close(); second.close(); @@ -83,7 +83,7 @@ class TestThrowingDisconnectedInterceptor extends JdbcPoolBaseTest { BoomInterceptor.class.getName() + ";" + RecordingInterceptor.class.getName()); Connection con = ds.getConnection(); // keep the physical connection strongly reachable (see above) - java.sql.Connection physical = con.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection physical = internalOf(con).getConnection(); con.close(); ds.close(); Assertions.assertEquals(1, RecordingInterceptor.DISCONNECTED_CALLS.get(), diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDriverOnClaimedPaths.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDriverOnClaimedPaths.java index 1166d93046..71f9455349 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDriverOnClaimedPaths.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingDriverOnClaimedPaths.java @@ -154,7 +154,7 @@ public class TestThrowingDriverOnClaimedPaths extends JdbcPoolBaseTest { DataSource ds = oneSlotPool(); Connection first = ds.getConnection(); awaitSessions(1); - PooledConnection pc = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pc = (PooledConnection) internalOf(first); first.close(); forceExpired(pc); @@ -180,7 +180,7 @@ public class TestThrowingDriverOnClaimedPaths extends JdbcPoolBaseTest { DataSource ds = oneSlotPool(); Connection first = ds.getConnection(); awaitSessions(1); - PooledConnection pc = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pc = (PooledConnection) internalOf(first); first.close(); forceExpired(pc); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingResetInterceptor.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingResetInterceptor.java index 3b7787fff4..4d59a51d40 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingResetInterceptor.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestThrowingResetInterceptor.java @@ -64,9 +64,9 @@ class TestThrowingResetInterceptor extends JdbcPoolBaseTest { // reclaimed by the JDK's socket cleaner and the leaked database // sessions would disappear with GC timing instead of staying open PooledConnection pooledFirst = - (PooledConnection) first.unwrap(PooledConnection.class); + (PooledConnection) internalOf(first); PooledConnection pooledSecond = - (PooledConnection) second.unwrap(PooledConnection.class); + (PooledConnection) internalOf(second); java.sql.Connection physicalFirst = pooledFirst.getConnection(); java.sql.Connection physicalSecond = pooledSecond.getConnection(); awaitSessions(2); @@ -95,7 +95,7 @@ class TestThrowingResetInterceptor extends JdbcPoolBaseTest { ThrowResetInterceptor.class.getName() + ";" + RecordingInterceptor.class.getName()); Connection con = ds.getConnection(); // keep the physical connection strongly reachable (see above) - java.sql.Connection physical = con.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection physical = internalOf(con).getConnection(); con.close(); ds.close(); // the shutdown reset walks the chain once, through finalize() diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestValidation.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestValidation.java index e53ea9c160..fc251fb27c 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestValidation.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestValidation.java @@ -72,7 +72,7 @@ class TestValidation extends JdbcPoolBaseTest { ds.createPool(); try (Connection pooled = ds.getConnection()) { - PooledConnection pc = pooled.unwrap(PooledConnection.class); + PooledConnection pc = internalOf(pooled); pc.setLastValidated(System.currentTimeMillis()); int before = validator.actions.size(); Assertions.assertTrue(pc.validate(PooledConnection.VALIDATE_BORROW, null), diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperContract.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperContract.java index ad81921da9..de72cbc423 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperContract.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/TestWrapperContract.java @@ -126,20 +126,19 @@ class TestWrapperContract extends JdbcPoolBaseTest { } @Test - void theInternalPooledConnectionUnwrapIdiomIsUnchanged() throws Exception { + void theInternalPooledConnectionIsNotExposedThroughWrapper() throws Exception { DataSource ds = newDataSource(); ds.createPool(); try (Connection pooled = ds.getConnection()) { - // The pool's own introspection idiom (used in tests and by - // interceptors): unwrap to the internal pooled connection. - org.apache.tomcat.jdbc.pool.PooledConnection internal = - pooled.unwrap(org.apache.tomcat.jdbc.pool.PooledConnection.class); - Assertions.assertSame( - interceptorOf(pooled, ProxyConnection.class).getConnection(), - internal, - "unwrapping to the internal pooled connection must keep " - + "returning the pool's own object"); + Assertions.assertFalse( + pooled.isWrapperFor(org.apache.tomcat.jdbc.pool.PooledConnection.class), + "the pool-internal connection must not be advertised as wrapped"); + SQLException thrown = Assertions.assertThrows(SQLException.class, + () -> pooled.unwrap(org.apache.tomcat.jdbc.pool.PooledConnection.class), + "the pool-internal connection must not be reachable through the public Wrapper contract"); + Assertions.assertTrue(thrown.getMessage().contains("Not a wrapper"), + "the failure must say it is a not-a-wrapper case: " + thrown.getMessage()); } } @@ -161,35 +160,28 @@ class TestWrapperContract extends JdbcPoolBaseTest { } @Test - void theInternalPooledConnectionIsAnsweredIdenticallyByBothEntryPoints() + void theInternalPooledConnectionIsRefusedByBothEntryPoints() throws Exception { DataSource ds = newDataSource(); ds.createPool(); try (Connection pooled = ds.getConnection()) { ProxyConnection interceptor = interceptorOf(pooled, ProxyConnection.class); - // What the handle's dispatch answers for the pool's own pooled - // connection, the bottom interceptor's public method answers - // too - one object on both entry points, pinning the - // consistency the two entry points of the chain must keep. - Object viaHandle = pooled.unwrap( - org.apache.tomcat.jdbc.pool.PooledConnection.class); - Object viaInterceptor = interceptor.unwrap( - org.apache.tomcat.jdbc.pool.PooledConnection.class); - Assertions.assertSame(viaInterceptor, viaHandle, - "the handle and the interceptor must answer the same object"); - Assertions.assertSame(interceptor.getConnection(), viaHandle, - "the answer both entry points give is the pool's own pooled connection"); - // The Wrapper contract: isWrapperFor must tell the truth about - // what unwrap hands out. Both objects sit on - are indirectly a - // wrapper of - the pooled connection their unwrap returns, so - // both must answer true, not the false that contradicted unwrap. - Assertions.assertTrue( + // Neither entry point advertises or hands out the pool-internal + // object: the public Wrapper contract exposes only the standard + // interfaces the handle itself implements. + Assertions.assertFalse( pooled.isWrapperFor(org.apache.tomcat.jdbc.pool.PooledConnection.class), - "isWrapperFor must agree with the unwrap the handle answers"); - Assertions.assertTrue( + "the handle must not advertise the pool-internal connection"); + Assertions.assertFalse( interceptor.isWrapperFor(org.apache.tomcat.jdbc.pool.PooledConnection.class), - "isWrapperFor must agree with the unwrap the interceptor answers"); + "the interceptor must not advertise the pool-internal connection"); + Assertions.assertThrows(SQLException.class, + () -> pooled.unwrap(org.apache.tomcat.jdbc.pool.PooledConnection.class), + "the handle must not hand out the pool-internal connection"); + Assertions.assertThrows(SQLException.class, + () -> interceptor.unwrap(org.apache.tomcat.jdbc.pool.PooledConnection.class), + "the interceptor must not hand out the pool-internal connection"); } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestResetAbandonedTimerMBeanIdle.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestResetAbandonedTimerMBeanIdle.java index 6c017d1a21..d5f59a0a36 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestResetAbandonedTimerMBeanIdle.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestResetAbandonedTimerMBeanIdle.java @@ -295,7 +295,7 @@ class TestResetAbandonedTimerMBeanIdle extends JdbcPoolBaseTest { pool = ds; try (Connection connection = ds.getConnection()) { - PooledConnection pc = connection.unwrap(PooledConnection.class); + PooledConnection pc = internalOf(connection); ResetAbandonedTimer anonymous = new ResetAbandonedTimer() { }; anonymous.reset(ds.getPool(), pc); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheCloseAfterDisposal.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheCloseAfterDisposal.java index 5e71a45fdb..629498272a 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheCloseAfterDisposal.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheCloseAfterDisposal.java @@ -141,7 +141,7 @@ class TestStatementCacheCloseAfterDisposal extends JdbcPoolBaseTest { Connection first = ds.getConnection(); StatementCache cache = interceptorOf(first, StatementCache.class); - PooledConnection pooled = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pooled = (PooledConnection) internalOf(first); DisposalAtParkMap parkPoint = new DisposalAtParkMap(); parkPoint.disposal = () -> cache.reset(null, null); pooled.getAttributes().put(StatementCache.STATEMENT_CACHE_ATTR, parkPoint); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheInterceptorOrder.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheInterceptorOrder.java index 62cd4868ea..0ec6c4363c 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheInterceptorOrder.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheInterceptorOrder.java @@ -211,7 +211,7 @@ class TestStatementCacheInterceptorOrder extends JdbcPoolBaseTest { String racingSql = "INSERT INTO " + racingTable + " VALUES (?)"; Connection first = ds.getConnection(); - PooledConnection pcon = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(first); TestStatementCacheRacingClose.RacingCloseStatementCache cache = interceptorOf(first, TestStatementCacheRacingClose.RacingCloseStatementCache.class); setSessionMarker(first, 43); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java index 22c1a12ff2..1dcc18aac6 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCachePurgeCloseFailure.java @@ -95,7 +95,7 @@ class TestStatementCachePurgeCloseFailure extends JdbcPoolBaseTest { try { ErrorCloseStatementCache cache = interceptorOf(shared, ErrorCloseStatementCache.class); - PooledConnection pcon = (PooledConnection) shared.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(shared); FAIL_NEXT_CLOSE.set(true); PreparedStatement first = shared.prepareStatement("SELECT 1"); @@ -135,7 +135,7 @@ class TestStatementCachePurgeCloseFailure extends JdbcPoolBaseTest { try { ErrorCloseStatementCache cache = interceptorOf(shared, ErrorCloseStatementCache.class); - PooledConnection pcon = (PooledConnection) shared.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(shared); FAIL_NEXT_CLOSE.set(true); PreparedStatement statement = shared.prepareStatement( "SELECT 1\nFROM SYSTEM_RANGE(1,1)\r"); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingClose.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingClose.java index 1b781f475f..b086e6bf90 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingClose.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingClose.java @@ -111,7 +111,7 @@ class TestStatementCacheRacingClose extends JdbcPoolBaseTest { String racingSql = "INSERT INTO " + racingTable + " VALUES (?)"; Connection first = ds.getConnection(); - PooledConnection pcon = (PooledConnection) first.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(first); RacingCloseStatementCache cache = interceptorOf(first, RacingCloseStatementCache.class); setSessionMarker(first, 31); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingPark.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingPark.java index 5ce37921b1..8dec624e55 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingPark.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheRacingPark.java @@ -167,7 +167,7 @@ class TestStatementCacheRacingPark extends JdbcPoolBaseTest { Connection shared = ds.getConnection(); try { StatementCache cache = interceptorOf(shared, StatementCache.class); - PooledConnection pcon = (PooledConnection) shared.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(shared); GatedParkMap gated = new GatedParkMap(); pcon.getAttributes().put(StatementCache.STATEMENT_CACHE_ATTR, gated); @@ -230,7 +230,7 @@ class TestStatementCacheRacingPark extends JdbcPoolBaseTest { StatementCache cache = interceptorOf(shared, StatementCache.class); // getCache() re-reads the map from the attributes on every // call, so this swap is what the next closes will meet - PooledConnection pcon = (PooledConnection) shared.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(shared); ParkRendezvousMap parkPoint = new ParkRendezvousMap(); pcon.getAttributes().put(StatementCache.STATEMENT_CACHE_ATTR, parkPoint); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheReconnect.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheReconnect.java index 1975ed19aa..e183857543 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheReconnect.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheReconnect.java @@ -140,7 +140,7 @@ class TestStatementCacheReconnect extends JdbcPoolBaseTest { String insert = "INSERT INTO " + table + " VALUES (?)"; Connection first = ds.getConnection(); - java.sql.Connection oldPhysical = first.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection oldPhysical = internalOf(first).getConnection(); setSessionMarker(first, 11); PreparedStatement prepared = first.prepareStatement(insert); Statement oldStatement = driverStatement(prepared); @@ -181,7 +181,7 @@ class TestStatementCacheReconnect extends JdbcPoolBaseTest { String insert = "INSERT INTO " + table + " VALUES (?)"; Connection first = ds.getConnection(); - java.sql.Connection oldPhysical = first.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection oldPhysical = internalOf(first).getConnection(); setSessionMarker(first, 12); PreparedStatement prepared = first.prepareStatement(insert); Statement oldStatement = driverStatement(prepared); @@ -226,7 +226,7 @@ class TestStatementCacheReconnect extends JdbcPoolBaseTest { String insert = "INSERT INTO " + table + " VALUES (?)"; Connection first = ds.getConnection(); - java.sql.Connection oldPhysical = first.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection oldPhysical = internalOf(first).getConnection(); setSessionMarker(first, 13); PreparedStatement prepared = first.prepareStatement(insert); Statement oldStatement = driverStatement(prepared); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheStaleEntryResurrection.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheStaleEntryResurrection.java index 40f7724014..a5d95820df 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheStaleEntryResurrection.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestStatementCacheStaleEntryResurrection.java @@ -67,7 +67,7 @@ class TestStatementCacheStaleEntryResurrection extends JdbcPoolBaseTest { String insert = "INSERT INTO " + table + " VALUES (?)"; Connection first = ds.getConnection(); - java.sql.Connection oldPhysical = first.unwrap(PooledConnection.class).getConnection(); + java.sql.Connection oldPhysical = internalOf(first).getConnection(); setSessionMarker(first, 21); PreparedStatement prepared = first.prepareStatement(insert); Statement oldDriverStatement = driverStatement(prepared); @@ -84,7 +84,7 @@ class TestStatementCacheStaleEntryResurrection extends JdbcPoolBaseTest { // Replace the physical session underneath the pooled connection, as // any reconnect path does: the purge runs and clears the map. - first.unwrap(PooledConnection.class).reconnect(); + internalOf(first).reconnect(); Assertions.assertTrue(oldPhysical.isClosed(), "the replaced session must be closed by the reconnect"); Assertions.assertEquals(0, cacheMap.size(), 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 66301bc999..85ea6ce576 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 @@ -598,7 +598,7 @@ class TestStatementFinalizer extends JdbcPoolBaseTest { ds.createPool(); try (Connection pooled = ds.getConnection()) { StatementFinalizer finalizer = interceptorOf(pooled, StatementFinalizer.class); - PooledConnection pcon = (PooledConnection) pooled.unwrap(PooledConnection.class); + PooledConnection pcon = (PooledConnection) internalOf(pooled); Method create = Connection.class.getMethod("createStatement"); finalizer.reset(ds.getPool(), pcon); diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestDataSourceRegistrationVsClose.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestDataSourceRegistrationVsClose.java index 0398278011..090a4e710f 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestDataSourceRegistrationVsClose.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestDataSourceRegistrationVsClose.java @@ -304,7 +304,7 @@ class TestDataSourceRegistrationVsClose extends JdbcPoolBaseTest { PooledConnection held; try (java.sql.Connection connection = ds.getConnection()) { - held = connection.unwrap(PooledConnection.class); + held = internalOf(connection); } // The connection is idle now; hold its lock so the registration's // connection sweep parks on it. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
