You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/15 18:56:01 UTC
(tomcat) 04/24: Fix name-scoped removal of attribute change notification listeners
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 e00a3d64af895fd6b66471631edafe19d4f2f74d
Author: opencode <re...@apache.org>
AuthorDate: Thu Oct 15 10:12:47 2026 +0200
Fix name-scoped removal of attribute change notification listeners
BaseModelMBean.removeAttributeChangeNotificationListener() previously
removed every attribute change registration for the given listener
instead of only the registration for the requested attribute name
(the long-standing FIXME in the method).
Add a removal method to BaseNotificationBroadcaster that is scoped to
an attribute name. Since addNotificationListener() coalesces
registrations with the same listener and handback into a single entry
with a multi-name BaseAttributeFilter, removal drops the name from the
filter and only removes the entry when no explicit name remains, so a
single-name filter is never silently turned into an accept-all filter.
Registrations that accept all attribute names (created with a null
attribute name) can only be removed by passing a null name, mirroring
the registration semantics. Entries registered with other filters are
left untouched.
Also add a unit test covering the coalesced, per-handback, accept-all
and non-attribute-filter cases.
---
.../apache/tomcat/util/modeler/BaseModelMBean.java | 7 +-
.../util/modeler/BaseNotificationBroadcaster.java | 38 ++++++
.../modeler/TestBaseModelMBeanNotifications.java | 141 +++++++++++++++++++++
3 files changed, 184 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/tomcat/util/modeler/BaseModelMBean.java b/java/org/apache/tomcat/util/modeler/BaseModelMBean.java
index ad5566cb6e..d1f09b57ae 100644
--- a/java/org/apache/tomcat/util/modeler/BaseModelMBean.java
+++ b/java/org/apache/tomcat/util/modeler/BaseModelMBean.java
@@ -517,9 +517,12 @@ public class BaseModelMBean implements DynamicMBean, MBeanRegistration, ModelMBe
throw new IllegalArgumentException(sm.getString("baseModelMBean.nullListener"));
}
- // FIXME - currently this removes *all* notifications for this listener
+ if (log.isTraceEnabled()) {
+ log.trace("removeAttributeNotificationListener " + listener);
+ }
+
if (attributeBroadcaster != null) {
- attributeBroadcaster.removeNotificationListener(listener);
+ attributeBroadcaster.removeAttributeNotificationListener(listener, name);
}
}
diff --git a/java/org/apache/tomcat/util/modeler/BaseNotificationBroadcaster.java b/java/org/apache/tomcat/util/modeler/BaseNotificationBroadcaster.java
index ac287c85dd..8e58b976d8 100644
--- a/java/org/apache/tomcat/util/modeler/BaseNotificationBroadcaster.java
+++ b/java/org/apache/tomcat/util/modeler/BaseNotificationBroadcaster.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.modeler;
import java.util.ArrayList;
+import java.util.Iterator;
import javax.management.ListenerNotFoundException;
import javax.management.MBeanNotificationInfo;
@@ -124,6 +125,43 @@ public class BaseNotificationBroadcaster implements NotificationBroadcaster {
}
+ /**
+ * Remove an attribute change notification event listener for a specific attribute name from this MBean. Other
+ * registrations of the same listener are left untouched. If a coalesced filter ends up accepting no explicit
+ * names, the corresponding entry is removed entirely rather than being turned into an accept-all filter. A
+ * registration that accepts all attribute names (a filter with no explicit names, created when the listener was
+ * added with a <code>null</code> attribute name) can only be removed by passing a <code>null</code> name here.
+ *
+ * @param listener The listener to be removed
+ * @param name The attribute name for which the registration should be removed, or <code>null</code> to remove
+ * a registration that accepts all attribute names
+ */
+ public void removeAttributeNotificationListener(NotificationListener listener, String name) {
+
+ synchronized (entries) {
+ Iterator<BaseNotificationBroadcasterEntry> iterator = entries.iterator();
+ while (iterator.hasNext()) {
+ BaseNotificationBroadcasterEntry item = iterator.next();
+ if (item.listener != listener || !(item.filter instanceof BaseAttributeFilter filter)) {
+ continue;
+ }
+ String[] names = filter.getNames();
+ if (name == null) {
+ if (names.length == 0) {
+ // The accept-all registration
+ iterator.remove();
+ }
+ } else if (names.length == 1 && names[0].equals(name)) {
+ iterator.remove();
+ } else {
+ filter.removeAttribute(name);
+ }
+ }
+ }
+
+ }
+
+
/**
* Send the specified notification to all interested listeners.
*
diff --git a/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java b/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java
new file mode 100644
index 0000000000..4d92b2c934
--- /dev/null
+++ b/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.modeler;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.management.Attribute;
+import javax.management.AttributeChangeNotification;
+import javax.management.Notification;
+import javax.management.NotificationFilter;
+import javax.management.NotificationListener;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestBaseModelMBeanNotifications {
+
+ private static final class TesterNotificationListener implements NotificationListener {
+
+ private final List<String> received = new ArrayList<>();
+
+ @Override
+ public void handleNotification(Notification notification, Object handback) {
+ String name = notification instanceof AttributeChangeNotification ?
+ ((AttributeChangeNotification) notification).getAttributeName() : notification.getType();
+ received.add(name + ":" + handback);
+ }
+ }
+
+ private static void send(BaseModelMBean mbean, String attributeName) throws Exception {
+ mbean.sendAttributeChangeNotification(new Attribute(attributeName, "old"),
+ new Attribute(attributeName, "new"));
+ }
+
+ @Test
+ public void testCoalescedRemovalKeepsOtherNames() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener = new TesterNotificationListener();
+ // Same listener and handback: these registrations are coalesced into a
+ // single entry with a filter accepting both names
+ mbean.addAttributeChangeNotificationListener(listener, "A", "hb");
+ mbean.addAttributeChangeNotificationListener(listener, "B", "hb");
+ mbean.removeAttributeChangeNotificationListener(listener, "A");
+ send(mbean, "A");
+ send(mbean, "B");
+ Assert.assertEquals(List.of("B:hb"), listener.received);
+ }
+
+ @Test
+ public void testSingleNameRemovalDoesNotAcceptAll() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener = new TesterNotificationListener();
+ mbean.addAttributeChangeNotificationListener(listener, "A", "hb");
+ mbean.removeAttributeChangeNotificationListener(listener, "A");
+ // Removing the only accepted name must remove the entry entirely rather
+ // than turning the (now empty) filter into an accept-all filter
+ send(mbean, "A");
+ send(mbean, "B");
+ Assert.assertTrue(listener.received.isEmpty());
+ }
+
+ @Test
+ public void testSeparateHandbacksRemovedIndependently() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener = new TesterNotificationListener();
+ mbean.addAttributeChangeNotificationListener(listener, "A", "hb1");
+ mbean.addAttributeChangeNotificationListener(listener, "B", "hb2");
+ mbean.removeAttributeChangeNotificationListener(listener, "A");
+ send(mbean, "A");
+ send(mbean, "B");
+ Assert.assertEquals(List.of("B:hb2"), listener.received);
+ }
+
+ @Test
+ public void testAcceptAllRemoval() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener = new TesterNotificationListener();
+ // A null attribute name registers an accept-all listener
+ mbean.addAttributeChangeNotificationListener(listener, null, "hb");
+ // A specific name can not be removed from an accept-all registration
+ mbean.removeAttributeChangeNotificationListener(listener, "A");
+ send(mbean, "A");
+ Assert.assertEquals(List.of("A:hb"), listener.received);
+ // The accept-all registration is removed by a null name
+ mbean.removeAttributeChangeNotificationListener(listener, null);
+ listener.received.clear();
+ send(mbean, "A");
+ Assert.assertTrue(listener.received.isEmpty());
+ }
+
+ @Test
+ public void testUnknownNameRemovalIsNoOp() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener = new TesterNotificationListener();
+ mbean.addAttributeChangeNotificationListener(listener, "A", "hb");
+ mbean.removeAttributeChangeNotificationListener(listener, "Z");
+ send(mbean, "A");
+ Assert.assertEquals(List.of("A:hb"), listener.received);
+ }
+
+ @Test
+ public void testOtherListenerRemovalDoesNotAffectThisListener() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener1 = new TesterNotificationListener();
+ TesterNotificationListener listener2 = new TesterNotificationListener();
+ mbean.addAttributeChangeNotificationListener(listener1, "A", "hb");
+ mbean.addAttributeChangeNotificationListener(listener2, "A", "hb");
+ mbean.removeAttributeChangeNotificationListener(listener1, "A");
+ send(mbean, "A");
+ Assert.assertTrue(listener1.received.isEmpty());
+ Assert.assertEquals(List.of("A:hb"), listener2.received);
+ }
+
+ @Test
+ public void testNonAttributeFilterIsNotRemovedByName() throws Exception {
+ BaseModelMBean mbean = new BaseModelMBean();
+ TesterNotificationListener listener = new TesterNotificationListener();
+ NotificationFilter acceptAll = notification -> true;
+ // Registered with a general (non attribute) filter via the
+ // NotificationBroadcaster method
+ mbean.addNotificationListener(listener, acceptAll, "hb");
+ mbean.removeAttributeChangeNotificationListener(listener, "A");
+ send(mbean, "A");
+ Assert.assertEquals(List.of("A:hb"), listener.received);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org