Handle JMX Collisions

Previously, if two DefaultConnectionContexts were created in a single JVM,
registration of the JMX ByteBuf monitors would cause an ugly message to be
logged.  While this wasn't terminal, it wasn't a great experience.  In
addition, the possibility of multiple DefaultConnectionContexts is a valid one
as you might need to connect to multiple host/port endpoints.  This change
updates the name of the MBeans to include endpoint to properly handle the
valid case of multiple DefaultConnectionContexts for multiple endpoints.  It
also warns about multiple duplicate DefaultConnectionContexts for the same
endpoint and removes the previous MBean, replacing it with the new one.  It
also unregisters the MBean during disposal as part of normal cleanup.

[resolves #836]

Author: nebhale

cloudfoundry-client-reactor/src/main/java/org/cloudfoundry/reactor/_DefaultConnectionContext.java

@@ -38,6 +38,7 @@
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 import javax.management.JMException;
+import javax.management.MalformedObjectNameException;
 import javax.management.ObjectName;
 import java.lang.management.ManagementFactory;
 import java.time.Duration;
@@ -57,6 +58,8 @@
 @Value.Immutable
 abstract class _DefaultConnectionContext implements ConnectionContext {
 
+    private static final int DEFAULT_PORT = 443;
+
     private static final int RECEIVE_BUFFER_SIZE = 10 * 1024 * 1024;
 
     private static final int SEND_BUFFER_SIZE = 10 * 1024 * 1024;
@@ -70,6 +73,16 @@ abstract class _DefaultConnectionContext implements ConnectionContext {
     public final void dispose() {
         getConnectionPool().ifPresent(PoolResources::dispose);
         getThreadPool().dispose();
+
+        try {
+            ObjectName name = getByteBufAllocatorObjectName();
+
+            if (ManagementFactory.getPlatformMBeanServer().isRegistered(name)) {
+                ManagementFactory.getPlatformMBeanServer().unregisterMBean(name);
+            }
+        } catch (JMException e) {
+            this.logger.error("Unable to register ByteBufAllocator MBean", e);
+        }
     }
 
     /**
@@ -146,7 +159,6 @@ public Mono<Void> trust(String host, int port) {
     /**
      * The hostname of the API root.  Typically something like {@code api.run.pivotal.io}.
      */
-    @Nullable
     abstract String getApiHost();
 
     /**
@@ -222,11 +234,21 @@ LoopResources getThreadPool() {
     @PostConstruct
     void monitorByteBufAllocator() {
         try {
-            ManagementFactory.getPlatformMBeanServer()
-                .registerMBean(new ByteBufAllocatorMetricProviderWrapper(PooledByteBufAllocator.DEFAULT), ObjectName.getInstance("org.cloudfoundry.reactor:type=ByteBufAllocator"));
+            ObjectName name = getByteBufAllocatorObjectName();
+
+            if (ManagementFactory.getPlatformMBeanServer().isRegistered(name)) {
+                this.logger.warn("MBean '{}' is already registered and will be removed. You should only have a single DefaultConnectionContext per endpoint.", name);
+                ManagementFactory.getPlatformMBeanServer().unregisterMBean(name);
+            }
+
+            ManagementFactory.getPlatformMBeanServer().registerMBean(new ByteBufAllocatorMetricProviderWrapper(PooledByteBufAllocator.DEFAULT), name);
         } catch (JMException e) {
             this.logger.error("Unable to register ByteBufAllocator MBean", e);
         }
     }
 
+    private ObjectName getByteBufAllocatorObjectName() throws MalformedObjectNameException {
+        return ObjectName.getInstance(String.format("org.cloudfoundry.reactor:type=ByteBufAllocator,endpoint=%s/%d", getApiHost(), getPort().orElse(DEFAULT_PORT)));
+    }
+
 }

cloudfoundry-client-reactor/src/test/java/org/cloudfoundry/reactor/DefaultConnectionContextTest.java

@@ -68,4 +68,21 @@ public void getInfo() throws Exception {
             .verify(Duration.ofSeconds(5));
     }
 
+    @Test
+    public void multipleInstances() {
+        DefaultConnectionContext first = DefaultConnectionContext.builder()
+            .apiHost("test-host")
+            .build();
+
+        DefaultConnectionContext second = DefaultConnectionContext.builder()
+            .apiHost("test-host")
+            .build();
+
+        first.monitorByteBufAllocator();
+        second.monitorByteBufAllocator();
+
+        first.dispose();
+        second.dispose();
+    }
+
 }