This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 912974f9a1178144298233fa71cf6b94d422fce0 Author: opencode <[email protected]> AuthorDate: Mon Nov 2 20:50:56 2026 +0100 Complete and correct the interceptor base contract's documentation Correct the setUseEquals javadoc, which claimed a default of false while the field, the field javadoc and the pool-level property all default to true (the mirrored wording in ProxyConnection and DisposableConnectionFacade was already corrected earlier). Document the contract the pool actually runs, none of which the base class stated anywhere: - invoke() with no next link fails with a NullPointerException; a pool-built chain always ends in the ProxyConnection terminal, so this is the hand-built-chain case. The pass-through idiom, returning super.invoke(...), is now named. The earlier rewrite of DisposableConnectionFacade removed the facade's dependence on catching this NPE, so the behavior is documented as fail-fast, not as load-bearing control flow. - reset() and disconnected() hooks must not rewire the chain: the borrow-time walk re-reads the next link after the callback while the shutdown walks read it before, so a hook that mutated its own link would be visited differently per walk. - The properties map is the live map of the interceptor definition, shared with every instance built from it and every later chain build, and must be treated as read-only. - Instance cardinality: one set of instances per pooled connection's chain, plus a throwaway instance per class for each poolStarted and poolClosed notification; hooks and invoke can arrive from borrower or pool-management threads. - poolStarted/poolClosed asymmetry: a throwing poolStarted aborts pool initialization (the pool closes and the failure is rethrown); a throwing poolClosed is logged at debug and ignored. The poolStarted @param copy-paste that described the pool being closed is fixed. Align the per-interceptor useEquals property with every other interceptor property: a value that is not exactly "true" or "false" now keeps the current value instead of silently switching the instance to reference comparison, the way Boolean.parseBoolean did. No functional defect was found in the class by the defect report or by an independent clean-room review. --- .../apache/tomcat/jdbc/pool/JdbcInterceptor.java | 68 +++++++++++++++++----- 1 file changed, 52 insertions(+), 16 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/JdbcInterceptor.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/JdbcInterceptor.java index d7ad0d575e..6d2cba00fb 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/JdbcInterceptor.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/JdbcInterceptor.java @@ -23,13 +23,21 @@ import java.util.Map; import org.apache.tomcat.jdbc.pool.PoolProperties.InterceptorProperty; /** - * Abstract class that is to be extended for implementations of interceptors. Everytime an operation is called on the - * {@link java.sql.Connection} object the {@link #invoke(Object, Method, Object[])} method on the interceptor will be - * called. Interceptors are useful to change or improve behavior of the connection pool.<br> + * Abstract class that is to be extended for implementations of interceptors. When an operation is called on the + * {@link java.sql.Connection} handle the pool hands out, the call travels down the interceptor chain; an interceptor + * that does not answer a call itself continues it by returning {@code super.invoke(proxy, method, args)}. A few calls + * are answered before the chain is entered - the {@link DisposableConnectionFacade} on top of every handle answers + * {@link Object#equals(Object)} and {@link Object#hashCode()} itself, and a handle whose borrow has ended is refused + * by that facade - but everything else reaches each link of the chain. Interceptors are useful to change or improve + * behavior of the connection pool.<br> * Interceptors can receive a set of properties. Each sub class is responsible for parsing the properties during runtime * when they are needed or simply override the {@link #setProperties(Map)} method. Properties arrive in a key-value pair - * of Strings as they were received through the configuration. This method is called once per cached connection object - * when the object is first configured. + * of Strings as they were received through the configuration. One set of interceptor instances is built per pooled + * connection (see {@link #setProperties(Map)}), plus a throwaway instance per configured class for each of the + * {@link #poolStarted(ConnectionPool)} and {@link #poolClosed(ConnectionPool)} notifications. Hooks and + * {@link #invoke(Object, Method, Object[])} can arrive from any thread that touches the pool: borrower threads run the + * chain, while pool management threads run {@link #reset(ConnectionPool, PooledConnection)} and + * {@link #disconnected(ConnectionPool, PooledConnection, boolean)} when a connection is shut down. */ public abstract class JdbcInterceptor implements InvocationHandler { /** @@ -100,6 +108,12 @@ public abstract class JdbcInterceptor implements InvocationHandler { /** * Gets invoked each time an operation on {@link java.sql.Connection} is invoked. {@inheritDoc} + * <p> + * The base implementation passes the call to the {@link #getNext() next} interceptor in the chain. When there is no + * next interceptor, the call fails fast with a {@link NullPointerException}: a chain built by the pool always ends + * in the {@link ProxyConnection} terminal, so reaching this case means a chain was hand-built without a terminal + * link. Interceptors that do not answer a call themselves continue it with + * {@code super.invoke(proxy, method, args)}. */ @Override @@ -163,7 +177,12 @@ public abstract class JdbcInterceptor implements InvocationHandler { * reference to the connection the interceptor can be reused for another connection. <br> * This method may be called with null as both arguments when we are closing down the connection. When called for a * shutdown reset, an interceptor that throws is isolated: the exception is logged and ignored, and the remaining - * interceptors in the chain are reset as well. + * interceptors in the chain are reset as well. <br> + * Chain topology is owned by the pool: a hook must not call {@link #setNext(JdbcInterceptor)} on itself or on any + * other link while the chain is wired, and in particular not from this callback. The walks that run this callback do + * not agree on when they read the next link - the walk at borrow time re-reads it after the callback, the shutdown + * walks read it before - so a hook that rewired its own link would be visited differently depending on which walk + * runs, and could be skipped or revisited without limit. * * @param parent - the connection pool owning the connection * @param con - the pooled connection @@ -175,7 +194,8 @@ public abstract class JdbcInterceptor implements InvocationHandler { * interceptors, that the physical connection has been released. Implementation of this method should be thought * through with care, as no actions should trigger an exception. An interceptor that nevertheless throws is * isolated: the exception is logged and ignored, the remaining interceptors in the chain are still notified, and - * the physical connection is closed as it would have been. + * the physical connection is closed as it would have been. As for {@link #reset(ConnectionPool, PooledConnection)}, + * a hook must not rewire the chain from this callback: chain topology is owned by the pool. * * @param parent - the connection pool that this connection belongs to * @param con - the pooled connection that holds this connection @@ -187,17 +207,23 @@ public abstract class JdbcInterceptor implements InvocationHandler { /** - * Returns the properties configured for this interceptor + * Returns the properties configured for this interceptor. The map is shared with every other interceptor instance + * created from the same interceptor definition and with every later chain build, so it must be treated as + * read-only: mutating it changes what all those instances and all later chains see. * - * @return the configured properties for this interceptor + * @return the configured properties for this interceptor, to be treated as read-only */ public Map<String, InterceptorProperty> getProperties() { return properties; } /** - * Called during the creation of an interceptor The properties can be set during the configuration of an interceptor - * Override this method to perform type casts between string values and object properties + * Called during the creation of an interceptor, once per instance: once for each instance created to build a pooled + * connection's interceptor chain, and once on the throwaway instance of each configured class that receives + * {@link #poolStarted(ConnectionPool)} at pool start and {@link #poolClosed(ConnectionPool)} at pool close. The pool + * passes the live map of the interceptor definition, shared across all these instances, so it must be treated as + * read-only (see {@link #getProperties()}). Override this method to perform type casts between string values and + * object properties. * * @param properties The properties */ @@ -206,7 +232,11 @@ public abstract class JdbcInterceptor implements InvocationHandler { final String useEquals = "useEquals"; InterceptorProperty p = properties.get(useEquals); if (p != null) { - setUseEquals(Boolean.parseBoolean(p.getValue())); + // Same parsing as every other interceptor property: a value that + // is not exactly "true" or "false" keeps the current value rather + // than silently switching to reference comparison, the way + // Boolean.parseBoolean would. + setUseEquals(p.getValueAsBoolean(isUseEquals())); } } @@ -222,7 +252,7 @@ public abstract class JdbcInterceptor implements InvocationHandler { /** * Set to true if string comparisons (for the {@link #compare(String, Method)} and {@link #compare(String, String)} - * methods) should use the Object.equals(Object) method The default is false + * methods) should use the Object.equals(Object) method The default is true * * @param useEquals <code>true</code> if equals will be used for comparisons */ @@ -233,7 +263,10 @@ public abstract class JdbcInterceptor implements InvocationHandler { /** * This method is invoked by a connection pool when the pool is closed. Interceptor classes can override this method * if they keep static variables or other tracking means around. <b>This method is only invoked on a single instance - * of the interceptor, and not on every instance created.</b> + * of the interceptor, and not on every instance created.</b> That instance is a throwaway created solely for this + * notification: it shares no state with the instances in any connection's chain, so any state this hook needs must + * be static or keyed on the pool. An exception thrown here is logged at debug level and otherwise ignored; the pool + * close continues. * * @param pool - the pool that is being closed. */ @@ -245,9 +278,12 @@ public abstract class JdbcInterceptor implements InvocationHandler { * This method is invoked by a connection pool when the pool is first started up, usually when the first connection * is requested. Interceptor classes can override this method if they keep static variables or other tracking means * around. <b>This method is only invoked on a single instance of the interceptor, and not on every instance - * created.</b> + * created.</b> That instance is a throwaway created solely for this notification: it shares no state with the + * instances in any connection's chain or with the instance that later receives {@link #poolClosed(ConnectionPool)}, + * so any state this hook needs must be static or keyed on the pool. An exception thrown here aborts pool + * initialization: the pool is closed and the failure is reported to the caller that triggered the start-up. * - * @param pool - the pool that is being closed. + * @param pool - the pool that is being started up. */ public void poolStarted(ConnectionPool pool) { // NOOP --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
