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 88b7030ae19ba60dd261b893aeb663483d909a54 Author: opencode <[email protected]> AuthorDate: Thu Nov 5 15:44:37 2026 +0100 Answer no JMX state once a statistics store is released PoolStatistics.release() set released but kept jmxState, and getJmxState() returned it unconditionally. A caller holding the released store could therefore see the state the close had already given up, contrary to the contract documented on stateFor(). Return null from getJmxState() once the store is released. release() still reads the field directly under the store monitor before marking it. Add a test that retains a store reference across close and asserts its JMX state is null. --- .../jdbc/pool/interceptor/SlowQueryReport.java | 5 ++-- .../jdbc/pool/interceptor/SlowQueryReportJmx.java | 8 +++++-- .../pool/interceptor/SlowQueryReportJmxMBean.java | 27 +++++++++++++--------- .../tomcat/jdbc/pool/util/LocalStrings.properties | 1 - .../interceptor/TestSlowQueryJmxNotifications.java | 22 ++++++++++++++++++ 5 files changed, 47 insertions(+), 16 deletions(-) diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java index 50114a5573..dfee7eb379 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReport.java @@ -827,10 +827,11 @@ public class SlowQueryReport extends AbstractQueryReport { /** * Returns what the reporting subclass has published for this pool with JMX. * - * @return the pool's JMX state, or {@code null} while no borrow of the pool has needed one + * @return the pool's JMX state, or {@code null} while no borrow of the pool has needed one or after the store + * has been released with its pool's close */ protected SlowQueryReportJmx.PoolJmx getJmxState() { - return jmxState; + return released ? null : jmxState; } /** 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 8bfb374fb5..eed1bfd505 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 @@ -62,6 +62,8 @@ import org.apache.tomcat.jdbc.pool.util.MessageFactory; * The MBean and the notification broadcaster belong to a <em>pool</em>, exactly like the statistics they publish: the * first interceptor instance of a pool to borrow a connection publishes them, and every other instance of that pool - * its other connections, the second definition of the recommended two-thresholds configuration - reports through them. + * The pool start callback only records the pool and its registration name; an idle pool is not published, so + * monitoring begins with the first borrow. * They are published under the name the pool started with - see {@link ConnectionPool#getRegistrationName()} - which * identifies the pool, because a container names every pool it manages uniquely and the default name of a * programmatically built pool is unique as well. Renaming a running pool, which the pool's own JMX allows, moves @@ -735,9 +737,11 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE } /** - * JMX operation - return the name of the pool + * JMX operation - return the name of the pool. * - * @return the name of the pool, unique within the JVM, or {@code null} before this instance is bound to a pool + * @return the registration name recorded when this instance was bound to its pool, or {@code null} before binding. + * The value is retained when the chain is released with {@code reset(null, null)}, so it can remain + * non-null after that release */ @Override public String getPoolName() { diff --git a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmxMBean.java b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmxMBean.java index 07af71253f..649026f4a9 100644 --- a/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmxMBean.java +++ b/modules/jdbc-pool/src/main/java/org/apache/tomcat/jdbc/pool/interceptor/SlowQueryReportJmxMBean.java @@ -28,11 +28,12 @@ import javax.management.openmbean.OpenDataException; * interceptor's {@code mbeans-descriptors.xml} advertises, therefore has to be declared here. * </p> * <p> - * The MBean is published once per connection pool name - under + * Each pool publishes its own MBean, under * {@code tomcat.jdbc:type=org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx,name=<pool>} unless the - * {@code objectName} interceptor property overrides it - and only when the interceptor is configured with + * {@code objectName} interceptor property overrides it, and only when the interceptor is configured with * {@code notifyPool=false}. With the default {@code notifyPool=true} no MBean is registered and the statistics are only - * observable through the connection pool's own MBean. + * observable through the connection pool's own MBean. A second live pool trying to publish under a name that is already + * occupied is refused by the MBean server; sharing one name between live pools is not supported. * </p> * <p> * {@code notifyPool} itself is deliberately not part of this interface. It is a per-interceptor-instance configuration @@ -46,12 +47,15 @@ import javax.management.openmbean.OpenDataException; */ public interface SlowQueryReportJmxMBean { /** - * Returns the name of the connection pool this MBean reports for, as the {@code PoolName} attribute. The - * registration, the notifications and the statistics are all keyed by pool name, so every live pool configured with - * the same name shares the one MBean reporting that name. + * Returns the registration name of the connection pool this MBean reports for, as the {@code PoolName} attribute. + * The registration, the notifications and the statistics belong to that one pool; a second live pool configured + * with the same name does not share this MBean, and its registration is refused while this one holds the name. + * <p> + * The value is recorded when the pool starts. It is not cleared when the pool's chain is released, so it can remain + * non-null after the MBean has been given up; only an instance that was never bound to a pool answers + * {@code null}. * - * @return the name of the pool, unique within the JVM; {@code null} when this interceptor instance is not bound to - * a pool yet - an instance registered with the MBeanServer always is + * @return the pool's recorded registration name, or {@code null} before this instance is bound to a pool */ String getPoolName(); @@ -68,9 +72,10 @@ public interface SlowQueryReportJmxMBean { /** * Removes all query statistics recorded for the pool this MBean reports for, so an operator can start from a clean - * slate instead of restarting the pool. Statistics are keyed by pool name, so this clears the map shared by every - * live pool configured with that name. Only the recorded content is dropped: the pool stays monitored and keeps - * accumulating statistics afterwards, and a query recorded again starts from empty counters. + * slate instead of restarting the pool. Only that pool's store is cleared; another live pool's statistics are + * untouched, even if it shares the name (a configuration the pool does not support for JMX publication). Only the + * recorded content is dropped: the pool stays monitored and keeps accumulating statistics afterwards, and a query + * recorded again starts from empty counters. */ void resetStats(); 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 01d861e64b..117cbb1130 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 @@ -215,7 +215,6 @@ slowQueryReport.slowQueryReport=Slow Query Report SQL={0}; time={1} ms; slowQueryReport.unknownProperty=The interceptor property [{0}] is not recognized and is not applied: property names are matched exactly, and a name this interceptor does not know leaves the setting it was meant to configure at the value it carries. Check the jdbcInterceptors configuration for the name that was meant to be configured. slowQueryReportJmx.invalidNotifyPool=The configured notifyPool [{0}] is neither true nor false and is not applied: query notifications keep going to the channel the current setting [{1}] selects. Correct the value in the jdbcInterceptors configuration to choose the notification channel that was meant to be configured. -slowQueryReportJmx.jmxDeregistrationFailed=Jmx deregistration failed. slowQueryReportJmx.jmxRegistrationFailed=Jmx registration of [{0}] failed, no JMX data will be exposed for the query stats until a registration succeeds. The registration is retried by later borrows; repeated failures are logged at debug level. slowQueryReportJmx.jmxRegistrationFailedNoData=Jmx registration failed, no JMX data will be exposed for the query stats. slowQueryReportJmx.jmxRegistrationInUse=Jmx registration of [{0}] failed: the name is already in use by another MBean. The registration is retried by later borrows; repeated failures are logged at debug level. 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 546d499a6f..5698f700dd 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 @@ -1200,6 +1200,28 @@ class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { + @Test + void aReleasedStoreAnswersNoJmxState() throws Exception { + String poolName = "JmxReleasedState-" + System.nanoTime(); + DataSource ds = reportingPoolNamed(poolName, + "org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx" + + "(notifyPool=false,threshold=-1)"); + ds.createPool(); + try (Connection ignored = ds.getConnection()) { + // the borrow publishes the state + Assertions.assertNotNull(SlowQueryReportJmx.jmxStateFor(ds.getPool()), + "precondition: the borrow must have published the pool's state"); + } + SlowQueryReport.PoolStatistics statistics = SlowQueryReport.statisticsFor(ds.getPool()); + Assertions.assertNotNull(statistics, "precondition: the pool has a statistics store"); + + ds.close(); + + Assertions.assertTrue(statistics.isReleased(), "the close must release the store"); + Assertions.assertNull(statistics.getJmxState(), + "a released store must not answer the state it gave up"); + } + @Test void aMalformedObjectNameOverrideFailsOnlyTheRegistration() throws Exception { // An override value that is not an ObjectName at all (no domain --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
