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 b6fe2650290f01066102c84e1aba27dd45b1f15d Author: remm <[email protected]> AuthorDate: Wed Oct 28 19:12:11 2026 +0100 Send the slow query JMX notifications from a serializable source The notifications of the slow query report interceptor carried the interceptor instance itself as their source when the pool was configured to notify through the interceptor's own MBean rather than the pool's. A notification marshals its source, so a listener reached through a JMX connector could only ever receive the notification of the one interceptor instance that happened to own the registration of that MBean, the interceptor instances of every other connection of the pool being silently dropped at the connector, the instance being nothing a remote process can deserialize; a listener attached directly to the emitter received the unserializable instance outright. The pool MBean the other mode notifies through names itself with the object name it is registered under, precisely for this reason. Name the MBean's registration in the source as that MBean does, the same name the registration of the pool name is published under, so every instance of the interceptor sends a source that marshals and identifies the pool rather than one accidental instance of it. An interceptor whose name cannot be formed was never registered, so its source is a string of the default name's shape, which still names the pool for the listeners the broadcaster serves in this JVM. Cover it with tests that the notification of every connection of a pool survives serialization with its source intact, that the source names the registered MBean and honours a configured name, and that an unformable name falls back to the string. --- .../jdbc/pool/interceptor/SlowQueryReportJmx.java | 38 +++- .../interceptor/TestSlowQueryJmxNotifications.java | 192 +++++++++++++++++++++ 2 files changed, 229 insertions(+), 1 deletion(-) 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 a2ba30237a..0c00415a77 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 @@ -329,7 +329,7 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE if (target!=null) { Notification notification = new Notification(type, - this, + getNotificationSource(), sequence, System.currentTimeMillis(), query); @@ -344,6 +344,42 @@ public class SlowQueryReportJmx extends SlowQueryReport implements NotificationE } } + /** + * Returns the source this MBean's notifications are sent with. + * <p> + * {@link Notification} marshals its source, so a listener outside this + * JVM - one reached through a JMX connector, or one that hands the + * notification on to another process - only receives the notification at + * all if the source is serializable. This MBean is not, and neither is + * the pool it reports on, so the source is the {@link ObjectName} the + * MBean is registered under, which names the pool and is serializable. + * Interceptor instances are created per pooled connection and only one + * of them is the registered MBean's resource, so an instance cannot + * name itself: every instance of a pool name sends the very name its + * registration was published under. An instance whose name cannot be + * formed - a malformed {@code objectName} property or a pool name that + * cannot be quoted into an ObjectName - was never registered at all, so + * no listener reached through a connector can exist for it; the source + * is then a string of the default name's shape, which still names the + * pool for the local listeners the broadcaster serves. + * + * @return the ObjectName the registration of this interceptor's pool + * name is published under, or, when that name cannot be + * formed, a string of the default name's shape + */ + protected Object getNotificationSource() { + try { + return getObjectName(getClass(), poolName); + } catch (MalformedObjectNameException x) { + // the name construction getObjectName uses when no override is + // configured - with a malformed name this MBean was never + // registered, so this fallback is only read by local listeners, + // for which it must still name the pool + return ConnectionPool.POOL_JMX_TYPE_PREFIX + getClass().getName() + + ",name=" + poolName; + } + } + /** * Reports a slow query and sends a JMX notification if logging is enabled. * @param query the SQL query 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 e64b0a79b7..2e9b23bf11 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 @@ -16,6 +16,10 @@ */ package org.apache.tomcat.jdbc.pool.interceptor; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.lang.management.ManagementFactory; import java.lang.reflect.Proxy; import java.sql.Connection; @@ -66,9 +70,197 @@ import org.apache.tomcat.jdbc.pool.JdbcInterceptor; * borrow retries, and closing a pool that never got published must * release the name's broadcaster and failure-report bookkeeping along * with the slot.</p> + * <p> + * The notifications themselves must also be deliverable beyond this + * JVM: {@code Notification} marshals its source, so the source has to be + * serializable - the ObjectName the MBean is registered under, or, when + * that name cannot be formed, a string of its shape - or a client behind + * a JMX connector never receives the notification at all.</p> */ class TestSlowQueryJmxNotifications extends JdbcPoolBaseTest { + /* + * The delivery a serialized client sees: javax.management.Notification + * writes its source into the stream, so this fails over a source that + * is not serializable - exactly what marshalling the notification over + * a JMX connector does. + */ + private static Notification roundTrip(Notification notification) throws Exception { + ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + try (ObjectOutputStream out = new ObjectOutputStream(bytes)) { + out.writeObject(notification); + } + try (ObjectInputStream in = + new ObjectInputStream(new ByteArrayInputStream(bytes.toByteArray()))) { + return (Notification) in.readObject(); + } + } + + @Test + void notificationOfEveryConnectionSurvivesSerializationWithItsSource() throws Exception { + String poolName = "JmxNotifySource-" + 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); + + List<Notification> received = new CopyOnWriteArrayList<>(); + NotificationListener listener = (notification, handback) -> received.add(notification); + + // two concurrent borrows with no pre-created connections means two + // physical connections, hence two interceptor instances, and only + // one of them can own the MBean registration. The raw notifications + // are captured from the emitter itself: a listener attached through + // the MBean server could not observe the defect here, because the + // JDK replaces the source with the ObjectName exactly and only when + // it is the registered instance - and a connector client behind + // that path would silently miss every notification an instance + // other than the registration owner sent + try (Connection first = ds.getConnection(); + Connection second = ds.getConnection()) { + Assertions.assertEquals(2, ds.getNumActive()); + interceptorOf(first, SlowQueryReportJmx.class) + .addNotificationListener(listener, null, null); + try (Statement st1 = first.createStatement(); + Statement st2 = second.createStatement()) { + st1.execute("SELECT 1"); + st2.execute("SELECT 2"); + } + } + + List<String> messages = received.stream().map(Notification::getMessage) + .collect(Collectors.toList()); + Assertions.assertEquals(2, messages.size(), + "every slow query of every connection must notify, received: " + messages); + for (Notification notification : received) { + Notification deserialized = roundTrip(notification); + Assertions.assertEquals(notification.getType(), deserialized.getType(), + "the type must survive the delivery a serialized client sees"); + Assertions.assertEquals(notification.getMessage(), deserialized.getMessage(), + "the message must survive that delivery"); + Assertions.assertEquals(notification.getSequenceNumber(), + deserialized.getSequenceNumber(), + "the sequence number must survive that delivery"); + Assertions.assertEquals(notification.getSource(), deserialized.getSource(), + "the source must survive that delivery naming the same pool"); + } + } + + @Test + void notificationSourceNamesTheRegisteredMBean() throws Exception { + String poolName = "JmxSourceOname-" + 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); + List<Notification> received = new CopyOnWriteArrayList<>(); + NotificationListener listener = (notification, handback) -> received.add(notification); + + try (Connection con = ds.getConnection()) { + Assertions.assertTrue(server.isRegistered(oname), + "the interceptor MBean must be registered once the pool is in use"); + interceptorOf(con, SlowQueryReportJmx.class) + .addNotificationListener(listener, null, null); + try (Statement st = con.createStatement()) { + st.execute("SELECT 1"); + } + } + + Assertions.assertEquals(1, received.size(), "the query must have notified"); + Assertions.assertEquals(oname, received.get(0).getSource(), + "the source must be the ObjectName the MBean is registered under: it is what" + + " a client behind a connector reads, and the only serializable name" + + " this MBean can send - it must come from the name the registration" + + " was published under, not from the emitting instance"); + Assertions.assertEquals(oname, roundTrip(received.get(0)).getSource(), + "and the notification must be deliverable to a serialized client"); + } + + @Test + void notificationSourceHonoursTheConfiguredObjectNameOverride() throws Exception { + String poolName = "JmxSourceOverride-" + System.nanoTime(); + // the grammar of jdbcInterceptors keeps everything after the first + // '=' as the property value, so an objectName without a comma is + // configurable; a comma-starved multi-property name cannot be, + // which is why this override carries a single key property + String customName = "tomcat.jdbc:type=SlowQueryOverride_" + System.nanoTime(); + DataSource ds = newDataSource(); + ds.setName(poolName); + ds.setJdbcInterceptors("org.apache.tomcat.jdbc.pool.interceptor.SlowQueryReportJmx" + + "(notifyPool=false,threshold=-1,objectName=" + customName + ")"); + ds.getPoolProperties().setInitialSize(0); + + MBeanServer server = ManagementFactory.getPlatformMBeanServer(); + ObjectName customOname = new ObjectName(customName); + List<Notification> received = new CopyOnWriteArrayList<>(); + NotificationListener listener = (notification, handback) -> received.add(notification); + + try (Connection con = ds.getConnection()) { + Assertions.assertTrue(server.isRegistered(customOname), + "the objectName attribute must decide the name the MBean registers under"); + interceptorOf(con, SlowQueryReportJmx.class) + .addNotificationListener(listener, null, null); + try (Statement st = con.createStatement()) { + st.execute("SELECT 1"); + } + } + + Assertions.assertEquals(1, received.size(), "the query must have notified"); + Assertions.assertEquals(customOname, received.get(0).getSource(), + "the source must name the MBean by the configured override it was" + + " registered under, not by the default shape or the instance"); + } + + @Test + void notificationSourceFallsBackToAStringWhenTheObjectNameCannotBeFormed() throws Exception { + // a pool name containing a colon cannot be quoted into the + // ObjectName the registration would use: registerJmx fails, the + // MBean is never registered - so no listener reached through a + // connector can exist for it - while local listeners must still + // get a source that names the pool and survives serialization + String poolName = "JmxSourceMalformed-" + System.nanoTime() + ":x"; + 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(); + List<Notification> received = new CopyOnWriteArrayList<>(); + NotificationListener listener = (notification, handback) -> received.add(notification); + + try (Connection con = ds.getConnection()) { + Assertions.assertTrue(server.queryNames(new ObjectName("tomcat.jdbc:type=" + + SlowQueryReportJmx.class.getName() + ",*"), null).isEmpty(), + "no interceptor MBean may be registered while the name cannot be formed"); + interceptorOf(con, SlowQueryReportJmx.class) + .addNotificationListener(listener, null, null); + try (Statement st = con.createStatement()) { + st.execute("SELECT 1"); + } + } + + Assertions.assertEquals(1, received.size(), + "an unregistrable pool must still notify its local listeners"); + Object source = received.get(0).getSource(); + Assertions.assertInstanceOf(String.class, source, + "the source must be a string when no ObjectName can be formed, was " + + (source == null ? "null" : source.getClass().getName())); + Assertions.assertEquals("tomcat.jdbc:type=" + SlowQueryReportJmx.class.getName() + + ",name=" + poolName, source, + "the string must read like the shape of the name the MBean would have" + + " been registered under"); + Assertions.assertEquals(source, roundTrip(received.get(0)).getSource(), + "and it must survive the delivery a serialized client sees"); + } + @Test void notifyPoolFalseDeliversNotificationsOfEveryConnection() throws Exception { String poolName = "JmxNotify-" + System.nanoTime(); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
