This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit de05a3f69bbed824e1e50aaad028943145e9523e Author: opencode <[email protected]> AuthorDate: Fri Oct 30 15:11:23 2026 +0100 Serve the SlowQueriesCD attribute as an empty array when no statistics exist The attribute implementation returned null when the pool name held no statistics map, although the descriptor advertises a CompositeData array and the sibling pool-names operation already degrades to an empty array. The state is reachable between the statistics release and the unregistration at pool close, and permanently when the server refuses the unregistration and JmxUtil swallows the failure, so a client that trusted the advertised type hit a null or an NPE. Default to an empty array and only fill it when statistics exist. --- .../jdbc/pool/interceptor/SlowQueryReportJmx.java | 13 +++- .../interceptor/TestSlowQueryJmxMBeanSurface.java | 72 ++++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java index 5a2af6eb94..33fcf85787 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmx.java @@ -503,11 +503,20 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE /** * JMX operation - returns all the queries we have collected. * - * @return - the slow query report as composite data. + * @return - the slow query report as composite data; an empty array when the pool name holds no statistics map - + * none was ever registered under it, or the last live pool of the name has closed and freed it */ @Override public CompositeData[] getSlowQueriesCD() throws OpenDataException { - CompositeDataSupport[] result = null; + // Default to the empty array, never null: the descriptor advertises + // the attribute as a CompositeData array, getPoolNames() beside it + // already degrades to an empty String array, and the name's statistics + // map can be gone while a client still reaches this registration - + // reading between the release of the statistics and the unregistration + // in poolClosed, or permanently when the unregistration of the closed + // pool's MBean was refused and JmxUtil swallowed the failure. A client + // that trusts the advertised type must not find null in its hands. + CompositeDataSupport[] result = new CompositeDataSupport[0]; ConcurrentHashMap<String, QueryStats> queries = getPoolStats(poolName); if (queries != null) { // Materialise a point-in-time snapshot before sizing the array: diff --git a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxMBeanSurface.java b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxMBeanSurface.java index 11109bc49c..797e1998d5 100644 --- a/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxMBeanSurface.java +++ b/modules/jdbc-pool/src/test/java/org/apache/tomcat/jdbc/pool/interceptor/TestSlowQueryJmxMBeanSurface.java @@ -35,6 +35,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.apache.tomcat.jdbc.JdbcPoolBaseTest; +import org.apache.tomcat.jdbc.pool.ConnectionPool; import org.apache.tomcat.jdbc.pool.DataSource; /** @@ -220,4 +221,75 @@ class TestSlowQueryJmxMBeanSurface extends JdbcPoolBaseTest { "the exposed statistics must be empty for the owning pool as well"); } } + + @Test + void slowQueriesCDIsAnEmptyArrayWhileTheReleasedStatisticsOutliveTheRegistration() + throws Exception { + String poolName = "JmxReleasedStats-" + System.nanoTime(); + DataSource ds = reportingPoolNamed(poolName); + + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + ObjectName oname = objectNameFor(poolName); + ConnectionPool pool = null; + try { + try (Connection con = ds.getConnection(); + Statement st = con.createStatement()) { + st.execute("SELECT 1"); + } + pool = ds.getPool(); + Assertions.assertEquals(1, ((CompositeData[]) server.getAttribute(oname, + "SlowQueriesCD")).length, + "the premise: the query must have been recorded while the pool was live"); + + // The window inside poolClosed: the close decides the release of + // the name's statistics first, and only after that gives up the + // registration - and JmxUtil.unregisterJmx swallows a refused + // unregistration, which leaves that state in place permanently. + // The window of a single close call is a thread race no test can + // force, so the test drives the two decided steps apart, in the + // order the race leaves them, and holds the registration back. + Assertions.assertTrue(SlowQueryReport.releaseStatistics(pool), + "the premise: the pool must have been the last live pool of the name"); + Assertions.assertTrue(server.isRegistered(oname), + "the premise: the MBean of the name must still be registered"); + Assertions.assertNull(SlowQueryReport.getPoolStats(poolName), + "the premise: the statistics map of the name must be gone"); + + Object value = server.getAttribute(oname, "SlowQueriesCD"); + Assertions.assertNotNull(value, + "a client that reads the attribute - advertised as " + + "[Ljavax.management.openmbean.CompositeData; - through the " + + "still-registered MBean must never get null in its hands"); + Assertions.assertEquals(0, ((CompositeData[]) value).length, + "with the statistics gone the attribute must degrade to the empty " + + "array, the way getPoolNames() degrades to an empty String array"); + } finally { + ds.close(); + // complete the release poolClosed decided with the statistics, so + // the window's registration is not left published behind this + // test: the close finds the statistics already decided free and + // skips the release of the name + SlowQueryReportJmx lateRelease = new SlowQueryReportJmx(); + lateRelease.poolName = poolName; + lateRelease.releaseJmxRegistration(pool); + Assertions.assertFalse(server.isRegistered(oname), + "teardown premise: completing the release must leave the name unregistered"); + } + } + + @Test + void slowQueriesCDIsEmptyArrayForANameThatHoldsNoStatistics() throws Exception { + // The same missing-map state pinned on the delegate method itself, + // the way the attribute-race test drives it: an interceptor bound to + // a name that holds no statistics map - the very state the registered + // MBean's delegate is in between the release of the name's statistics + // and its unregistration, and after an unregistration failure. + SlowQueryReportJmx interceptor = new SlowQueryReportJmx(); + interceptor.poolName = "JmxNoStats-" + System.nanoTime(); + Assertions.assertNull(SlowQueryReport.getPoolStats(interceptor.poolName), + "the premise: no statistics map may be registered under the name"); + Assertions.assertEquals(0, interceptor.getSlowQueriesCD().length, + "with no statistics map the read must degrade to the empty array, " + + "not to a null a JMX client has to trip over"); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
