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 43e43f5275de402518e39f855b58d54e68ee6933 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 22:38:13 2026 +0100 Release child MBeans when a pool MBean is deregistered Releasing the pool MBean left the per-connection and interceptor MBeans registered. They sit below the pool MBean, hold the pooled connection and therefore the pool, and keep their names occupied, so a DataSource that is deregistered and then dropped without closing the pool pins the pool, its sessions and its driver class loader for the life of the JVM. Add ConnectionPool.unregisterConnectionMBeans() and PooledConnection.unregisterMBean(), which release the connection MBean and walk the interceptor chain through a new JdbcInterceptor.unregisterMBean() hook. StatementCache and ResetAbandonedTimer override it to release their own MBean without disturbing their connection state. DataSource.releasePoolMBean() invokes the release before unregistering the pool MBean, and the cleared handles let a later registration republish the children. [...] --- .../apache/tomcat/jdbc/pool/ConnectionPool.java | 25 +++++++++++ .../org/apache/tomcat/jdbc/pool/DataSource.java | 21 +++++++++ .../apache/tomcat/jdbc/pool/JdbcInterceptor.java | 10 +++++ .../apache/tomcat/jdbc/pool/PooledConnection.java | 26 +++++++++++ .../jdbc/pool/interceptor/ResetAbandonedTimer.java | 10 +++++ .../jdbc/pool/interceptor/StatementCache.java | 26 ++++++++--- .../tomcat/jdbc/pool/util/LocalStrings.properties | 1 + .../tomcat/jdbc/pool/jmx/TestJmxRegistration.java | 52 ++++++++++++++++++++++ 8 files changed, 165 insertions(+), 6 deletions(-) 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 87048e21b0..915a47f023 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 @@ -2665,6 +2665,31 @@ public class ConnectionPool { registerConnectionMBeans(busy); } + /** + * Releases the MBeans published for the connections this pool currently holds and for the interceptors of their + * chains, when the pool's own MBean is released while the pool stays live. The connections and their sessions are + * untouched; a later borrow or reconnect can publish them again. + */ + public void unregisterConnectionMBeans() { + unregisterConnectionMBeans(idle); + unregisterConnectionMBeans(busy); + } + + private void unregisterConnectionMBeans(BlockingQueue<PooledConnection> pool) { + for (PooledConnection con : pool) { + try { + con.unregisterMBean(); + } catch (Throwable x) { + // JMX is best effort: one connection that fails to release + // its MBeans must not keep the others registered + ExceptionUtils.handleThrowable(x); + if (log.isDebugEnabled()) { + log.debug("Unable to unregister connection MBean", x); + } + } + } + } + private void registerConnectionMBeans(BlockingQueue<PooledConnection> pool) { // The fair queue iterates a snapshot taken under its lock, the default // queue a view that tolerates concurrent modification: either way, a diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSource.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSource.java index 27dd5c4079..d69a1d03b1 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSource.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/DataSource.java @@ -26,6 +26,7 @@ import javax.management.ObjectName; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.jdbc.pool.jmx.JmxUtil; +import org.apache.tomcat.jdbc.pool.util.ExceptionUtils; import org.apache.tomcat.jdbc.pool.util.MessageFactory; @@ -338,6 +339,25 @@ public class DataSource extends DataSourceProxy implements javax.sql.DataSource, JmxUtil.unregisterJmx(name); } + /** + * Releases the MBeans published below the pool MBean - the connections the pool holds and the interceptors of their + * chains - so that deregistering this data source does not leave them pinning a pool that stays live. The pool and + * its sessions are untouched, and a later registration publishes the children again. The release is best effort: + * JMX is a management channel, so a failure here is logged and does not stop the pool MBean from being released. + */ + private void releaseConnectionMBeans() { + ConnectionPool p = pool; + if (p == null) { + return; + } + try { + p.unregisterConnectionMBeans(); + } catch (Throwable t) { + ExceptionUtils.handleThrowable(t); + log.error(MessageFactory.get("dataSource.unableToUnregisterConnectionMBeans"), t); + } + } + /** * Releases the registration handle and unregisters the pool MBean * published under the name it points to, as one step under this data @@ -358,6 +378,7 @@ public class DataSource extends DataSourceProxy implements javax.sql.DataSource, oname = null; publishedByRegistration = false; if (name != null) { + releaseConnectionMBeans(); try { unregisterJmx(name); } catch (Throwable t) { 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 2494baaa26..d20af0d483 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 @@ -143,6 +143,16 @@ public abstract class JdbcInterceptor implements InvocationHandler { this.next = next; } + /** + * Releases any MBean this interceptor published for the connection it serves, without tearing down the connection + * state. The pool calls this when it releases its own MBean while the pool stays live; the connection and its + * chain remain usable, and a later borrow can publish the interceptor again. The default does nothing; an + * interceptor that publishes an MBean overrides it. + */ + public void unregisterMBean() { + // No MBean to release + } + /** * Performs a string comparison, using references unless the useEquals property is set to true. * diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java index 6690c94f82..66b827a123 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/PooledConnection.java @@ -1684,6 +1684,32 @@ public class PooledConnection implements PooledConnectionMBean { oname = JmxUtil.registerJmx(jmx.getObjectName(), keyprop, this); } + /** + * Releases the MBeans published for this connection and for the interceptors of its chain, without releasing the + * physical session. The pool uses this when its own MBean is released while the pool stays live: the connection + * and its chain remain usable, and a later borrow or reconnect can publish them again. + */ + public void unregisterMBean() { + synchronized (this) { + if (oname != null) { + JmxUtil.unregisterJmx(oname); + oname = null; + } + } + for (JdbcInterceptor interceptor = getHandler(); interceptor != null; interceptor = interceptor.getNext()) { + try { + interceptor.unregisterMBean(); + } catch (Throwable x) { + // JMX is best effort: one interceptor that fails to release + // its MBean must not keep the others registered + ExceptionUtils.handleThrowable(x); + if (log.isDebugEnabled()) { + log.debug("Unable to unregister interceptor MBean", x); + } + } + } + } + /** * Returns the JMX ObjectName for this pooled connection. * diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ResetAbandonedTimer.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ResetAbandonedTimer.java index ee5da06fb1..138d61c610 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ResetAbandonedTimer.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/ResetAbandonedTimer.java @@ -175,6 +175,16 @@ public class ResetAbandonedTimer extends AbstractQueryReport implements ResetAba } } + @Override + public void unregisterMBean() { + synchronized (this) { + if (oname != null) { + JmxUtil.unregisterJmx(oname); + oname = null; + } + } + } + /** * Resets the abandoned timer of the pooled connection this interceptor is bound to, and only while that connection * is checked out. This is the implementation of the MBean operation; the activity hooks below stamp through diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java index 1a7ba2a949..6620e7297d 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/StatementCache.java @@ -262,9 +262,11 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta if (parent == null) { cacheSize = null; this.pcon = null; - if (oname != null) { - JmxUtil.unregisterJmx(oname); - oname = null; + synchronized (this) { + if (oname != null) { + JmxUtil.unregisterJmx(oname); + oname = null; + } } } else { draining = false; @@ -283,9 +285,21 @@ public class StatementCache extends StatementDecoratorInterceptor implements Sta ConcurrentHashMap<CacheKey, CachedStatement> cache = new ConcurrentHashMap<>(); pcon.getAttributes().put(STATEMENT_CACHE_ATTR, cache); } - if (oname == null) { - String keyprop = ",JdbcInterceptor=" + getClass().getSimpleName(); - oname = JmxUtil.registerJmx(pcon.getObjectName(), keyprop, this); + synchronized (this) { + if (oname == null) { + String keyprop = ",JdbcInterceptor=" + getClass().getSimpleName(); + oname = JmxUtil.registerJmx(pcon.getObjectName(), keyprop, this); + } + } + } + } + + @Override + public void unregisterMBean() { + synchronized (this) { + if (oname != null) { + JmxUtil.unregisterJmx(oname); + oname = null; } } } diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties index 54d36c999d..1b5e4b79dd 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/util/LocalStrings.properties @@ -100,6 +100,7 @@ dataSource.mbeanNotRegistered=JDBC pool MBean [{0}] not registered: the pool has dataSource.unableToCreateObjectName=Unable to create object name for JDBC pool. dataSource.unableToRegisterMBean=Unable to register JDBC pool MBean. dataSource.unableToUnregisterMBean=Unable to unregister JDBC pool MBean [{0}]. The registration handle was already given up, so the close or callback that asked for this release continues. +dataSource.unableToUnregisterConnectionMBeans=Unable to unregister the JDBC pool's connection MBeans. JMX is best effort, so the pool MBean release continues; some connection MBeans may stay registered. dataSourceFactory.cannotSetDataSourceLayered=Can''t set dataSource property to another connection pool ({0}): a pool cannot wrap another pool. Carry the JDBC driver''s data source object under the dataSource attribute, or use the dataSourceJNDI attribute to name one bound in JNDI. dataSourceFactory.cannotSetDataSourceString=Can''t set dataSource property as a string "{0}". Either carry the data source object itself under the dataSource attribute of the Reference - a javax.sql.DataSource, javax.sql.XADataSource or javax.sql.ConnectionPoolDataSource - or use the dataSourceJNDI attribute to name a data source bound in JNDI. diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java index 1f948fdd5a..90d58a8c35 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxRegistration.java @@ -317,6 +317,58 @@ class TestJmxRegistration extends JdbcPoolBaseTest { return new ReusableCache(ds, cache, cacheName); } + @Test + void releasingThePoolMBeanReleasesTheConnectionAndInterceptorMBeans() throws Exception { + MBeanServer server = server(); + DataSource ds = newDataSource(); + ds.getPoolProperties().setJmxEnabled(true); + ds.setJdbcInterceptors(StatementCache.class.getName() + "(prepared=true,max=5)"); + ds.getPoolProperties().setInitialSize(0); + ds.createPool(); + ds.preRegister(server, uniqueName("children-released")); + ObjectName poolOname = ds.getPool().getJmxPool().getObjectName(); + Assertions.assertNotNull(poolOname, "precondition: the pool MBean is published"); + + ObjectName connectionName; + ObjectName cacheName; + try (Connection borrowed = ds.getConnection()) { + connectionName = connectionMBeanUnder(server, poolOname); + cacheName = new ObjectName(connectionName.toString() + ",JdbcInterceptor=StatementCache"); + Assertions.assertTrue(server.isRegistered(cacheName), + "precondition: the cache of the borrowed connection publishes its MBean"); + } + + try { + ds.postDeregister(); + + Assertions.assertFalse(server.isRegistered(poolOname), + "postDeregister must unregister the pool MBean"); + Assertions.assertFalse(server.isRegistered(connectionName), + "postDeregister must unregister the connection MBean below it"); + Assertions.assertFalse(server.isRegistered(cacheName), + "postDeregister must unregister the interceptor MBeans below the connection"); + + // The pool stays live; registering the DataSource again must + // republish the children whose handles the release cleared. + ds.preRegister(server, uniqueName("children-republished")); + ObjectName republishedPool = ds.getPool().getJmxPool().getObjectName(); + Assertions.assertNotNull(republishedPool, + "precondition: the DataSource can be registered again"); + ObjectName republishedConnection = connectionMBeanUnder(server, republishedPool); + Assertions.assertNotNull(republishedConnection, + "a connection released by the deregistration must republish itself"); + try (Connection borrowed = ds.getConnection()) { + ObjectName republishedCache = new ObjectName( + republishedConnection.toString() + ",JdbcInterceptor=StatementCache"); + Assertions.assertTrue(server.isRegistered(republishedCache), + "an interceptor released by the deregistration must republish on the next borrow"); + } + } finally { + ds.close(); + ds.postDeregister(); + } + } + @Test void aPublishedMBeanComesBackAsItsOwnNameAndStaysReleasable() throws Exception { MBeanServer server = server(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
