Skip to content

Commit

Permalink
Fixes #1718 - QueuedThreadPool not exposed on JMX.
Browse files Browse the repository at this point in the history 
Now MBeanContainer keeps track of the parent/child component relationship.

MBeans are registered for a pair (parent, child) and unregistered only
for the same pair.

For example, Server owns a ThreadPool and a Connector; in turn the
Connector adds the Server ThreadPool as an unmanaged component to itself.
The ThreadPool is registered as MBean with the pair (Server, ThreadPool).
When the Connector is stopped and removed from the component tree,
MBeanContainer sees a removal event for the pair (ServerConnector, ThreadPool).
But since that pair was not the one that triggered the registration of
the MBean, the MBean is not unregistered.
  • Loading branch information
@sbordet
sbordet committed Sep 11, 2017
1 parent c402f8d commit a0cb424
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 23 deletions.
53 changes: 32 additions & 21 deletions jetty-jmx/src/main/java/org/eclipse/jetty/jmx/MBeanContainer.java
View file
Expand Up @@ -21,6 +21,7 @@
import java.io.IOException;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.atomic.AtomicInteger;
Expand All @@ -46,14 +47,11 @@ public class MBeanContainer implements Container.InheritedListener, Dumpable, De
{
private final static Logger LOG = Log.getLogger(MBeanContainer.class.getName());
private final static ConcurrentMap<String, AtomicInteger> __unique = new ConcurrentHashMap<>();

public static void resetUnique()
{
__unique.clear();
}
private static final Container ROOT = new ContainerLifeCycle();

private final MBeanServer _mbeanServer;
private final Map<Object, ObjectName> _beans = new ConcurrentHashMap<>();
private final ConcurrentMap<Object, Container> _beans = new ConcurrentHashMap<>();
private final ConcurrentMap<Object, ObjectName> _mbeans = new ConcurrentHashMap<>();
private String _domain = null;

/**
Expand All @@ -64,7 +62,7 @@ public static void resetUnique()
*/
public ObjectName findMBean(Object object)
{
return _beans.get(object);
return _mbeans.get(object);
}

/**
Expand All @@ -75,7 +73,7 @@ public ObjectName findMBean(Object object)
*/
public Object findBean(ObjectName objectName)
{
for (Map.Entry<Object, ObjectName> entry : _beans.entrySet())
for (Map.Entry<Object, ObjectName> entry : _mbeans.entrySet())
{
if (entry.getValue().equals(objectName))
return entry.getKey();
Expand Down Expand Up @@ -130,9 +128,19 @@ public void beanAdded(Container parent, Object obj)
if (LOG.isDebugEnabled())
LOG.debug("beanAdded {}->{}", parent, obj);

if (obj == null)
return;

if (parent == null)
parent = ROOT;

// Is the bean already tracked ?
if (_beans.putIfAbsent(obj, parent) != null)
return;

// Is there an object name for the parent ?
ObjectName parentObjectName = null;
if (parent != null)
if (parent != ROOT)
{
parentObjectName = findMBean(parent);
if (parentObjectName == null)
Expand All @@ -143,10 +151,6 @@ public void beanAdded(Container parent, Object obj)
}
}

// Does the mbean already exist ?
if (obj == null || _beans.containsKey(obj))
return;

try
{
// Create an MBean for the object.
Expand Down Expand Up @@ -207,7 +211,7 @@ public void beanAdded(Container parent, Object obj)
if (LOG.isDebugEnabled())
LOG.debug("Registered {}", objectName);

_beans.put(obj, objectName);
_mbeans.put(obj, objectName);
}
catch (Throwable x)
{
Expand All @@ -219,12 +223,17 @@ public void beanAdded(Container parent, Object obj)
public void beanRemoved(Container parent, Object obj)
{
if (LOG.isDebugEnabled())
LOG.debug("beanRemoved {}", obj);
LOG.debug("beanRemoved {}->{}", parent, obj);

ObjectName objectName = _beans.remove(obj);
if (parent == null)
parent = ROOT;

if (objectName != null)
unregister(objectName);
if (_beans.remove(obj, parent))
{
ObjectName objectName = _mbeans.remove(obj);
if (objectName != null)
unregister(objectName);
}
}

/**
Expand All @@ -248,7 +257,7 @@ public String makeName(String basis)
public void dump(Appendable out, String indent) throws IOException
{
ContainerLifeCycle.dumpObject(out,this);
ContainerLifeCycle.dump(out, indent, _beans.entrySet());
ContainerLifeCycle.dump(out, indent, _mbeans.entrySet());
}

@Override
Expand All @@ -260,9 +269,11 @@ public String dump()
@Override
public void destroy()
{
_beans.values().stream()
.filter(objectName -> objectName != null)
_mbeans.values().stream()
.filter(Objects::nonNull)
.forEach(this::unregister);
_mbeans.clear();
_beans.clear();
}

private void unregister(ObjectName objectName)
Expand Down
34 changes: 32 additions & 2 deletions jetty-jmx/src/test/java/org/eclipse/jetty/jmx/MBeanContainerTest.java
View file
Expand Up @@ -200,8 +200,8 @@ public void testDestroy() throws Exception
setUpDestroy();

// when
mbeanContainer.destroy();
objectName = mbeanContainer.findMBean(managed);
mbeanContainer.destroy();

// then
Assert.assertFalse("Unregistered bean - managed", mbeanContainer.getMBeanServer().isRegistered(objectName));
Expand All @@ -212,9 +212,9 @@ public void testDestroyInstanceNotFoundException() throws Exception
{
// given
setUpDestroy();
objectName = mbeanContainer.findMBean(managed);

// when
objectName = mbeanContainer.findMBean(managed);
mbeanContainer.getMBeanServer().unregisterMBean(objectName);

// then
Expand All @@ -224,4 +224,34 @@ public void testDestroyInstanceNotFoundException() throws Exception
// an exception of type InstanceNotFoundException occurs.
mbeanContainer.destroy();
}

@Test
public void testNonManagedLifecycleNotUnregistered() throws Exception
{
testNonManagedObjectNotUnregistered(new ContainerLifeCycle());
}

@Test
public void testNonManagedPojoNotUnregistered() throws Exception
{
testNonManagedObjectNotUnregistered(new Object());
}

private void testNonManagedObjectNotUnregistered(Object lifeCycle) throws Exception
{
ContainerLifeCycle parent = new ContainerLifeCycle();
parent.addBean(mbeanContainer);

ContainerLifeCycle child = new ContainerLifeCycle();
parent.addBean(child);

parent.addBean(lifeCycle, true);
child.addBean(lifeCycle, false);

parent.start();

parent.removeBean(child);

Assert.assertNotNull(mbeanContainer.findMBean(lifeCycle));
}
}

0 comments on commit a0cb424

Please sign in to comment.