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 d632ce1141120e8be8d0f3e69596dfe1a0b1eb53 Author: opencode <[email protected]> AuthorDate: Tue Oct 27 18:53:57 2026 +0100 Pin SlowQueryReportJmx MBean survival across connection eviction Report item 2 claimed that an eviction-triggered destroy() deregisters the SlowQueryReportJmx MBean while other connections stay live, and that per-instance registration flags then prevent any re-registration, permanently disconnecting JMX clients. Verification against current code shows the defect is stale: no per-connection destroy path deregisters JMX at all (eviction only calls reset(null, null), which is JMX-inert, and JdbcInterceptor has no destroy()), registration ownership already follows the static per-pool-name mbeans registry exactly as the report proposed, and deregisterJmx() runs only from poolClosed() at full pool shutdown. The one half of the scenario that was ever live - notifications severing when the MBean-owning connection was evicted, because the registered instance's broadcaster was instance-private - was resolved by cef17d34be, which made the broadcaster shared per pool name. The same-named-pool orphaning case belongs to a separate report item and is untouched here. Rather than change production code, this converts the verification into regression pins so a future re-introduction of an instance-level registration latch or an eviction-time deregistration fails loudly. evictionOfMBeanOwningConnectionKeepsMBeanAndListenersLive deterministically evicts the MBean-owning connection through the maxIdle/return path (sweeper disabled, physical eviction confirmed against the database session count) and asserts the MBean stays registered and a listener attached before the eviction still receives notifications raised afterward. Against an emulated revert of cef17d34be it fails exactly on the notification assertion, while the MBean-presence assertion passes even there. releasedRegistrationSlotIsTakenOverByTheNextBorrow releases the registry slot the way pool shutdown does and asserts the next borrow re-registers the MBean and takes over the slot. Test-only change; full jdbc-pool suite 268 tests, 0 failures. --- .../interceptor/TestSlowQueryJmxNotifications.java | 121 +++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java index 14ccbdef23..9e61ef6635 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxNotifications.java @@ -23,6 +23,7 @@ import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.stream.Collectors; +import javax.management.ListenerNotFoundException; import javax.management.MBeanServer; import javax.management.Notification; import javax.management.NotificationListener; @@ -42,6 +43,12 @@ import org.apache.tomcat.jdbc.pool.DataSource; * occurring on any connection must reach those listeners, not only the * ones occurring on the connection whose interceptor instance happened * to win the MBean registration. + * <p> + * The MBean registration is owned by the per-pool-name registry, not by + * the interceptor instance that won it: evicting the connection whose + * instance owns the MBean must leave the MBean and its listeners live, + * and a released registration slot must be taken over by the next + * borrow.</p> */ class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { @@ -89,4 +96,118 @@ class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { "the query run on the second physical connection must also reach the listener, received: " + messages); } + + @Test + void evictionOfMBeanOwningConnectionKeepsMBeanAndListenersLive() throws Exception { + String poolName = "JmxEvict-" + System.nanoTime(); + DataSource ds = newDataSource(); + ds.setName(poolName); + ds.setJdbcInterceptors("org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx" + + "(notifyPool=false,threshold=-1)"); + ds.getPoolProperties().setMaxActive(2); + ds.getPoolProperties().setInitialSize(0); + ds.getPoolProperties().setMaxIdle(1); + // sweeper disabled: the maxIdle cap must be enforced by the + // return path itself, so the eviction of the owning connection is + // synchronous with its close and needs no timing + ds.getPoolProperties().setTimeBetweenEvictionRunsMillis(0); + + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + ObjectName oname = new ObjectName("tomcat.jdbc:type=" + + SlowQueryReportJmx.class.getName() + ",name=" + poolName); + List<Notification> received = new CopyOnWriteArrayList<>(); + NotificationListener listener = (notification, handback) -> received.add(notification); + + // the first borrow's interceptor instance wins the MBean registry + // slot; the second concurrent borrow (hence second physical + // connection and second interceptor instance) shares the notifier + Connection ownerConnection = ds.getConnection(); + Connection otherConnection = ds.getConnection(); + try { + Assertions.assertEquals(2, ds.getNumActive()); + Assertions.assertSame(SlowQueryReportJmx.mbeans.get(poolName), + interceptorOf(ownerConnection, SlowQueryReportJmx.class), + "the first connection's interceptor instance must own the MBean registration"); + + server.addNotificationListener(oname, listener, null, null); + try (Statement st1 = ownerConnection.createStatement(); + Statement st2 = otherConnection.createStatement()) { + st1.execute("SELECT 1"); + st2.execute("SELECT 2"); + } + + // return the non-owning connection first so it stays within + // maxIdle; the owner then comes back over the cap and the + // return path closes it - the reported defect's trigger: an + // eviction of the very connection owning the MBean + otherConnection.close(); + ownerConnection.close(); + ownerConnection = null; + awaitSessions(1); // the owner's physical session is really gone + + Assertions.assertTrue(server.isRegistered(oname), + "evicting the MBean-owning connection must not unregister the pool's MBean"); + + // the surviving connection is reused; a query on its (not + // registering) interceptor instance must still notify the + // listener that was attached before the eviction + try (Connection survivor = ds.getConnection(); + Statement st = survivor.createStatement()) { + st.execute("SELECT 3"); + } + + List<String> messages = received.stream().map(Notification::getMessage) + .collect(Collectors.toList()); + Assertions.assertTrue(messages.stream().anyMatch(m -> m.contains("SELECT 3")), + "notifications raised after the owning connection was evicted must still " + + "reach the listener registered with the pool's MBean, received: " + + messages); + } finally { + if (ownerConnection != null) { + ownerConnection.close(); + } + try { + server.removeNotificationListener(oname, listener); + } catch (ListenerNotFoundException ignore) { + // the listener was never added - the failure that matters + // already surfaced in the try block + } + } + } + + @Test + void releasedRegistrationSlotIsTakenOverByTheNextBorrow() throws Exception { + String poolName = "JmxTakeover-" + System.nanoTime(); + DataSource ds = newDataSource(); + ds.setName(poolName); + ds.setJdbcInterceptors("org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx" + + "(notifyPool=false,threshold=-1)"); + ds.getPoolProperties().setInitialSize(0); + + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + ObjectName oname = new ObjectName("tomcat.jdbc:type=" + + SlowQueryReportJmx.class.getName() + ",name=" + poolName); + + try (Connection first = ds.getConnection()) { + Assertions.assertTrue(server.isRegistered(oname), + "the interceptor MBean must be registered once the pool is in use"); + // release the registry slot the way the pool's own closing does + // it, while another connection of the same pool stays live + SlowQueryReportJmx owner = interceptorOf(first, SlowQueryReportJmx.class); + owner.deregisterJmx(); + Assertions.assertFalse(server.isRegistered(oname), + "releasing the registry slot must unregister the MBean"); + + // a borrow afterwards must republish the MBean: registration + // ownership lives in the per-name registry, and must not latch + // onto an instance flag that would make register() a no-op + try (Connection next = ds.getConnection()) { + Assertions.assertTrue(server.isRegistered(oname), + "the next borrow after the slot was released must re-register the MBean"); + Assertions.assertSame(SlowQueryReportJmx.mbeans.get(poolName), + interceptorOf(next, SlowQueryReportJmx.class), + "the borrowing connection's interceptor instance must take over the slot"); + } + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
