Mercurial > hg > release > thermostat-1.0

changeset 1351:b7c6db90e034

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix JMXHeapDumper to use agent proxy I have discovered that JMXHeapDumper does not use the MXBeanConnectionPool, and thus does not go through the agent proxy. So heap dumps are still broken in multi-user scenarios. This patch makes JMXHeapDumper use the MXBeanConnectionPool and in the process removes duplicated code. I've also added a test case for this class. Reviewed-by: omajid Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-November/008840.html
author Elliott Baron <ebaron@redhat.com>
date Wed, 20 Nov 2013 12:27:59 -0500
parents ff5a4032f35f
children 9f451c06cb51
files vm-heap-analysis/agent/pom.xml vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/Activator.java vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiver.java vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumper.java vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/ActivatorTest.java vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumperTest.java
diffstat 6 files changed, 109 insertions(+), 41 deletions(-) [+]
line wrap: on
line diff
--- a/vm-heap-analysis/agent/pom.xml	Wed Nov 20 12:26:53 2013 -0500
+++ b/vm-heap-analysis/agent/pom.xml	Wed Nov 20 12:27:59 2013 -0500
@@ -84,6 +84,11 @@
 
     <dependency>
       <groupId>com.redhat.thermostat</groupId>
+      <artifactId>thermostat-agent-core</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>com.redhat.thermostat</groupId>
       <artifactId>thermostat-agent-command</artifactId>
       <version>${project.version}</version>
     </dependency>
--- a/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/Activator.java	Wed Nov 20 12:26:53 2013 -0500
+++ b/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/Activator.java	Wed Nov 20 12:27:59 2013 -0500
@@ -42,6 +42,7 @@
 import org.osgi.framework.BundleContext;
 
 import com.redhat.thermostat.agent.command.ReceiverRegistry;
+import com.redhat.thermostat.utils.management.MXBeanConnectionPool;
 import com.redhat.thermostat.common.MultipleServiceTracker;
 import com.redhat.thermostat.common.MultipleServiceTracker.Action;
 import com.redhat.thermostat.storage.core.WriterID;
@@ -61,6 +62,7 @@
         
         Class<?>[] deps = new Class<?>[] {
                 HeapDAO.class,
+                MXBeanConnectionPool.class,
                 WriterID.class // heap dump receiver uses it.
         };
 
@@ -69,8 +71,9 @@
             @Override
             public void dependenciesAvailable(Map<String, Object> services) {
                 HeapDAO service = (HeapDAO) services.get(HeapDAO.class.getName());
+                MXBeanConnectionPool pool = (MXBeanConnectionPool) services.get(MXBeanConnectionPool.class.getName());
                 WriterID writerId = (WriterID) services.get(WriterID.class.getName());
-                receiver = new HeapDumpReceiver(service, writerId);
+                receiver = new HeapDumpReceiver(service, pool, writerId);
                 receivers.registerReceiver(receiver);
             }
 
--- a/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiver.java	Wed Nov 20 12:26:53 2013 -0500
+++ b/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/HeapDumpReceiver.java	Wed Nov 20 12:27:59 2013 -0500
@@ -48,6 +48,7 @@
 import com.redhat.thermostat.common.command.Response.ResponseType;
 import com.redhat.thermostat.common.utils.LoggingUtils;
 import com.redhat.thermostat.storage.core.WriterID;
+import com.redhat.thermostat.utils.management.MXBeanConnectionPool;
 import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO;
 import com.redhat.thermostat.vm.heap.analysis.common.HistogramLoader;
 import com.redhat.thermostat.vm.heap.analysis.common.ObjectHistogram;
@@ -65,8 +66,8 @@
     private final HistogramLoader histogramLoader;
     private final WriterID writerId;
 
-    public HeapDumpReceiver(HeapDAO heapDao, WriterID writerId) {
-        this(heapDao, new JMXHeapDumper(), new JMapHeapDumper(), new HistogramLoader(), writerId);
+    public HeapDumpReceiver(HeapDAO heapDao, MXBeanConnectionPool pool, WriterID writerId) {
+        this(heapDao, new JMXHeapDumper(pool), new JMapHeapDumper(), new HistogramLoader(), writerId);
     }
 
     HeapDumpReceiver(HeapDAO heapDao, JMXHeapDumper jmxHeapDumper,
--- a/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumper.java	Wed Nov 20 12:26:53 2013 -0500
+++ b/vm-heap-analysis/agent/src/main/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumper.java	Wed Nov 20 12:27:59 2013 -0500
@@ -34,60 +34,42 @@
  * to do so, delete this exception statement from your version.
  */
 
-
 package com.redhat.thermostat.vm.heap.analysis.agent.internal;
 
-import java.io.File;
-import java.util.Properties;
-
 import javax.management.MBeanServerConnection;
 import javax.management.ObjectName;
-import javax.management.remote.JMXConnector;
-import javax.management.remote.JMXConnectorFactory;
-import javax.management.remote.JMXServiceURL;
 
-import com.sun.tools.attach.VirtualMachine;
+import com.redhat.thermostat.utils.management.MXBeanConnection;
+import com.redhat.thermostat.utils.management.MXBeanConnectionPool;
 
 class JMXHeapDumper {
 
-    private static final String CONNECTOR_ADDRESS_PROPERTY = "com.sun.management.jmxremote.localConnectorAddress";
+    private MXBeanConnectionPool pool;
+    
+    JMXHeapDumper(MXBeanConnectionPool pool) {
+        this.pool = pool;
+    }
 
     void dumpHeap(int pid, String filename) throws HeapDumpException {
         try {
-            doHeapDump(pid, filename);
+            MXBeanConnection connection = pool.acquire(pid);
+            try {
+                doHeapDump(connection, filename);
+            } finally {
+                pool.release(pid, connection);
+            }
         } catch (Exception ex) {
             throw new HeapDumpException(ex);
         }
     }
 
-    private void doHeapDump(int pid, String filename) throws Exception {
-
-        VirtualMachine vm = VirtualMachine.attach(String.valueOf(pid));
-        String connectorAddress = getConnectorAddress(vm);
-        JMXServiceURL url = new JMXServiceURL(connectorAddress);
-
-        try (JMXConnector conn = JMXConnectorFactory.connect(url)) {
-            MBeanServerConnection mbsc = conn.getMBeanServerConnection();
-            mbsc.invoke(new ObjectName("com.sun.management:type=HotSpotDiagnostic"),
-                        "dumpHeap",
-                        new Object[] { filename, Boolean.TRUE },
-                        new String[] { String.class.getName(), boolean.class.getName() });
-        }
+    private void doHeapDump(MXBeanConnection connection, String filename) throws Exception {
+        MBeanServerConnection mbsc = connection.get();
+        mbsc.invoke(new ObjectName("com.sun.management:type=HotSpotDiagnostic"),
+                "dumpHeap",
+                new Object[] { filename, Boolean.TRUE },
+                new String[] { String.class.getName(), boolean.class.getName() });
     }
 
-    private String getConnectorAddress(VirtualMachine vm) throws Exception {
-
-        Properties props = vm.getAgentProperties();
-        String connectorAddress = props.getProperty(CONNECTOR_ADDRESS_PROPERTY);
-        if (connectorAddress == null) {
-            props = vm.getSystemProperties();
-            String home = props.getProperty("java.home");
-            String agent = home + File.separator + "lib" + File.separator + "management-agent.jar";
-            vm.loadAgent(agent);
-            props = vm.getAgentProperties();
-            connectorAddress = props.getProperty(CONNECTOR_ADDRESS_PROPERTY);
-        }
-        return connectorAddress;
-    }
 }
 
--- a/vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/ActivatorTest.java	Wed Nov 20 12:26:53 2013 -0500
+++ b/vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/ActivatorTest.java	Wed Nov 20 12:27:59 2013 -0500
@@ -46,6 +46,7 @@
 import com.redhat.thermostat.agent.command.RequestReceiver;
 import com.redhat.thermostat.storage.core.WriterID;
 import com.redhat.thermostat.testutils.StubBundleContext;
+import com.redhat.thermostat.utils.management.MXBeanConnectionPool;
 import com.redhat.thermostat.vm.heap.analysis.common.HeapDAO;
 
 public class ActivatorTest {
@@ -71,8 +72,10 @@
         StubBundleContext ctx = new StubBundleContext();
 
         HeapDAO heapDao = mock(HeapDAO.class);
+        MXBeanConnectionPool pool = mock(MXBeanConnectionPool.class);
         WriterID writerId = mock(WriterID.class);
         ctx.registerService(HeapDAO.class, heapDao, null);
+        ctx.registerService(MXBeanConnectionPool.class, pool, null);
         ctx.registerService(WriterID.class, writerId, null);
 
         Activator activator = new Activator();
@@ -81,7 +84,7 @@
 
         assertTrue(ctx.isServiceRegistered(RequestReceiver.class.getName(), HeapDumpReceiver.class));
 
-        assertEquals(2, ctx.getServiceListeners().size());
+        assertEquals(3, ctx.getServiceListeners().size());
 
         activator.stop(ctx);
 
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/vm-heap-analysis/agent/src/test/java/com/redhat/thermostat/vm/heap/analysis/agent/internal/JMXHeapDumperTest.java	Wed Nov 20 12:27:59 2013 -0500
@@ -0,0 +1,74 @@
+/*
+ * Copyright 2012, 2013 Red Hat, Inc.
+ *
+ * This file is part of Thermostat.
+ *
+ * Thermostat is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published
+ * by the Free Software Foundation; either version 2, or (at your
+ * option) any later version.
+ *
+ * Thermostat is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Thermostat; see the file COPYING.  If not see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Linking this code with other modules is making a combined work
+ * based on this code.  Thus, the terms and conditions of the GNU
+ * General Public License cover the whole combination.
+ *
+ * As a special exception, the copyright holders of this code give
+ * you permission to link this code with independent modules to
+ * produce an executable, regardless of the license terms of these
+ * independent modules, and to copy and distribute the resulting
+ * executable under terms of your choice, provided that you also
+ * meet, for each linked independent module, the terms and conditions
+ * of the license of that module.  An independent module is a module
+ * which is not derived from or based on this code.  If you modify
+ * this code, you may extend this exception to your version of the
+ * library, but you are not obligated to do so.  If you do not wish
+ * to do so, delete this exception statement from your version.
+ */
+
+package com.redhat.thermostat.vm.heap.analysis.agent.internal;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import javax.management.MBeanServerConnection;
+import javax.management.ObjectName;
+
+import org.junit.Test;
+
+import com.redhat.thermostat.utils.management.MXBeanConnection;
+import com.redhat.thermostat.utils.management.MXBeanConnectionPool;
+
+public class JMXHeapDumperTest {
+
+    @Test
+    public void testDumpHeap() throws Exception {
+        // Mock connections
+        MXBeanConnectionPool pool = mock(MXBeanConnectionPool.class);
+        MXBeanConnection mxBeanConn = mock(MXBeanConnection.class);
+        when(pool.acquire(9000)).thenReturn(mxBeanConn);
+        MBeanServerConnection mbsc = mock(MBeanServerConnection.class);
+        when(mxBeanConn.get()).thenReturn(mbsc);
+        
+        JMXHeapDumper dumper = new JMXHeapDumper(pool);
+        dumper.dumpHeap(9000, "/path/to/dump");
+        
+        // Verify pool used correctly, and dump command issued
+        verify(pool).acquire(9000);
+        verify(mbsc).invoke(new ObjectName("com.sun.management:type=HotSpotDiagnostic"),
+                "dumpHeap",
+                new Object[] { "/path/to/dump", Boolean.TRUE },
+                new String[] { String.class.getName(), boolean.class.getName() });
+        verify(pool).release(9000, mxBeanConn);
+    }
+
+}