Mercurial > hg > release > thermostat-1.0
changeset 1351:b7c6db90e034
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); + } + +}