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 4f8bbaebee65156d53275b6efcc1252428738652 Author: opencode <[email protected]> AuthorDate: Mon Oct 26 15:26:02 2026 +0100 Unregister the pool MBean when the DataSource is closed The tomcat.jdbc ConnectionPool MBean was only unregistered from postDeregister(), i.e. only when a container deregistered the DataSource object itself. Closing the pool with close() performed no unregistration, so the MBean of the dead pool stayed registered, served frozen statistics, and kept the closed pool - including its PoolProperties credentials and the JDBC driver class loader - strongly reachable in the platform MBean server. DataSource now unregisters the pool MBean when close() runs and drops the ObjectName registration handle; postDeregister() likewise drops the handle after unregistering, so the two lifecycle steps cannot each try to unregister an already removed name. --- .../org/apache/tomcat/jdbc/pool/DataSource.java | 20 +++++++++++ .../tomcat/jdbc/pool/jmx/TestJmxMonitor.java | 39 +++++++++++++++++++--- 2 files changed, 54 insertions(+), 5 deletions(-) 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 21a63342aa..f83011ed91 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 @@ -51,6 +51,25 @@ public class DataSource extends DataSourceProxy implements javax.sql.DataSource, super(poolProperties); } + /** + * Unregisters the connection pool MBean before closing the pool, so a + * dead pool does not keep reporting stale statistics under a live + * object name (and does not pin the closed pool, its credentials and + * its JDBC driver class loader in the MBean server).<br> + * {@inheritDoc} + */ + @Override + public synchronized void close(boolean all) { + // Same monitor as DataSourceProxy.close(boolean), so the + // check-and-clear of the registration handle is atomic against a + // concurrent close. + if (oname != null) { + unregisterJmx(); + oname = null; + } + super.close(all); + } + @@ -71,6 +90,7 @@ public class DataSource extends DataSourceProxy implements javax.sql.DataSource, public void postDeregister() { if (oname!=null) { unregisterJmx(); + oname = null; } } diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxMonitor.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxMonitor.java index 910e5dbc36..85419e32d0 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxMonitor.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/jmx/TestJmxMonitor.java @@ -93,6 +93,17 @@ class TestJmxMonitor extends JdbcPoolBaseTest { return count; } + /* The pool-MBean registration handle is a private field; tests reach + * it reflectively to pin that a finished lifecycle step drops it. */ + private static void assertNoRegistrationHandle(DataSource ds) + throws Exception { + Field handle = DataSource.class.getDeclaredField("oname"); + handle.setAccessible(true); + Assertions.assertNull(handle.get(ds), + "the DataSource must not keep an ObjectName as a pool-MBean " + + "registration handle at this point"); + } + @Test void poolMBeanTracksLiveTraffic() throws Exception { MBeanServer server = ManagementFactory.getPlatformMBeanServer(); @@ -130,6 +141,7 @@ class TestJmxMonitor extends JdbcPoolBaseTest { ds.postDeregister(); Assertions.assertFalse(server.isRegistered(poolOname), "postDeregister must unregister the pool MBean"); + assertNoRegistrationHandle(ds); } } @@ -228,6 +240,27 @@ class TestJmxMonitor extends JdbcPoolBaseTest { } } + @Test + void closingThePoolUnregistersItsMBean() throws Exception { + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + DataSource ds = newDataSource(); + ds.getPoolProperties().setJmxEnabled(true); + ds.createPool(); + + ObjectName dsName = uniqueDataSourceName("closed"); + ObjectName poolOname = registerAndFindPoolMBean(server, ds, dsName); + Assertions.assertNotNull(poolOname, "an enabled pool must publish its MBean"); + + ds.close(); + Assertions.assertFalse(server.isRegistered(poolOname), + "closing the pool must unregister its MBean instead of " + + "letting it report a dead pool"); + assertNoRegistrationHandle(ds); + // the container still deregisters the DataSource itself afterwards; + // that must stay a clean no-op + Assertions.assertDoesNotThrow(ds::postDeregister); + } + @Test void registrationBeforeThePoolExistsKeepsTheContainerCallback() throws Exception { @@ -250,11 +283,7 @@ class TestJmxMonitor extends JdbcPoolBaseTest { // The registration then completes, so the container will also // deregister later. The pool-side handle postDeregister() // unregisters must not claim a name that was never registered. - Field handle = DataSource.class.getDeclaredField("oname"); - handle.setAccessible(true); - Assertions.assertNull(handle.get(ds), - "a skipped pool MBean registration must not leave its " - + "ObjectName behind as a registration handle"); + assertNoRegistrationHandle(ds); Assertions.assertDoesNotThrow(ds::postDeregister, "deregistering a DataSource whose pool MBean was never " + "published must be a clean no-op"); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
