Mercurial > hg > thermostat-ng > agent
changeset 2426:ec9be9806a79
Improve Byteman Thermostat helper.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-August/020636.html
author | Severin Gehwolf <sgehwolf@redhat.com> |
---|---|
date | Wed, 24 Aug 2016 12:06:35 +0200 |
parents | 47c53031d2bc |
children | bc83ef2d7047 |
files | vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/Properties.java vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/ThermostatHelper.java vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransport.java vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/NoopLocalSocketChannel.java vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/ThermostatHelperTest.java vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransportTest.java |
diffstat | 6 files changed, 152 insertions(+), 17 deletions(-) [+] |
line wrap: on
line diff
--- a/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/Properties.java Tue Aug 23 19:16:09 2016 +0200 +++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/Properties.java Wed Aug 24 12:06:35 2016 +0200 @@ -39,5 +39,6 @@ public interface Properties { String PREFIX = "org.jboss.byteman.thermostat."; + String BYTEMAN_VERBOSE_PROP = "org.jboss.byteman.verbose"; }
--- a/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/ThermostatHelper.java Tue Aug 23 19:16:09 2016 +0200 +++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/ThermostatHelper.java Wed Aug 24 12:06:35 2016 +0200 @@ -53,26 +53,28 @@ // Lock to synchronize initialization of transport between instances private static final Object transportLock = new Object(); private static Transport transport = null; - - static void initTransport() { - transport = new TransportFactory().create(); - Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() { - @Override - public void run() { - transport.close(); - } - })); - } /** * Constructor */ protected ThermostatHelper(Rule rule) { super(rule); - // Initialize transport if not yet done + Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() { + @Override + public void run() { + synchronized (transportLock) { + if (transport != null) { + transport.close(); + } + } + } + })); + } + + private static void initTransport() { synchronized (transportLock) { if (transport == null) { - initTransport(); + transport = new TransportFactory().create(); } } } @@ -109,14 +111,33 @@ public void send(String marker, Object... dataArray) { LinkedHashMap<String, Object> data = toMap(dataArray); BytemanMetric rec = new BytemanMetric(marker, data); - transport.send(rec); + synchronized (transportLock) { + transport.send(rec); + } } - // For testing purposes static void setTransport(Transport transport) { synchronized (transportLock) { ThermostatHelper.transport = transport; } } + + // package-private for testing + static Transport getTransport() { + return transport; + } + + // Called by Byteman on helper de-activation + public static void deactivated() { + synchronized (transportLock) { + transport.close(); + } + setTransport(null); + } + + // Called by Byteman on helper activation + public static void activated() { + initTransport(); + } }
--- a/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransport.java Tue Aug 23 19:16:09 2016 +0200 +++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransport.java Wed Aug 24 12:06:35 2016 +0200 @@ -45,7 +45,6 @@ import org.jboss.byteman.thermostat.helper.BytemanMetric; import org.jboss.byteman.thermostat.helper.Transport; -import org.jboss.byteman.thermostat.helper.TransportException; import org.jboss.byteman.thermostat.helper.Utils; /** @@ -79,11 +78,17 @@ this.attempts = attempts; this.breakIntervalMillis = breakIntervalMillis; this.socketName = socketName; + LocalSocketChannel ch; try { - this.channel = channelFactory.open(ipcConfig, socketName); + ch = channelFactory.open(ipcConfig, socketName); } catch (Exception e) { - throw new TransportException("Error opening Thermostat socket: [" + socketName + "]", e); + // Cannot write to thermostat endpoint for some reason. + // Use a no-op channel so that we keep things reasonable in the + // target JVM + String reason = "Error opening Thermostat socket: [" + socketName + "]"; + ch = new NoopLocalSocketChannel(reason); } + this.channel = ch; } /**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/NoopLocalSocketChannel.java Wed Aug 24 12:06:35 2016 +0200 @@ -0,0 +1,65 @@ +/* + * Copyright 2012-2016 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 org.jboss.byteman.thermostat.helper.transport.ipc; + +import java.io.IOException; +import java.nio.ByteBuffer; + +import org.jboss.byteman.thermostat.Properties; + +public class NoopLocalSocketChannel implements LocalSocketChannel { + + private static final boolean IS_VERBOSE = Boolean.getBoolean(Properties.BYTEMAN_VERBOSE_PROP); + private final String reason; + + public NoopLocalSocketChannel(String reason) { + this.reason = reason; + } + + @Override + public void close() throws IOException { + // nothing + } + + @Override + public void write(ByteBuffer buffer) throws IOException { + if (IS_VERBOSE) { + System.out.println("[" + NoopLocalSocketChannel.class.getSimpleName() + "] Discarding message received via send(). Reason: " + reason); + } + } + +}
--- a/vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/ThermostatHelperTest.java Tue Aug 23 19:16:09 2016 +0200 +++ b/vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/ThermostatHelperTest.java Wed Aug 24 12:06:35 2016 +0200 @@ -38,10 +38,13 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import java.io.ByteArrayOutputStream; import java.io.PrintStream; @@ -178,6 +181,33 @@ System.setOut(oldOut); } } + + @Test + public void testActivatedNonNull() { + Transport t = mock(Transport.class); + ThermostatHelper.setTransport(t); + ThermostatHelper.activated(); + assertSame(t, ThermostatHelper.getTransport()); + } + + @Test + public void testConstructorDoesNotSetTransport() { + ThermostatHelper.setTransport(null); + new ThermostatHelper(mock(Rule.class)); + assertNull(ThermostatHelper.getTransport()); + } + + @Test + public void testDeactivated() { + Transport t = mock(Transport.class); + ThermostatHelper.setTransport(t); + assertSame(t, ThermostatHelper.getTransport()); + + ThermostatHelper.deactivated(); + verify(t).close(); + verifyNoMoreInteractions(t); + assertNull(ThermostatHelper.getTransport()); + } private Object getValue(boolean stringValue, int i) { if (stringValue) {
--- a/vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransportTest.java Tue Aug 23 19:16:09 2016 +0200 +++ b/vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransportTest.java Wed Aug 24 12:06:35 2016 +0200 @@ -37,6 +37,7 @@ package org.jboss.byteman.thermostat.helper.transport.ipc; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.mock; @@ -79,6 +80,18 @@ t.close(); verify(factory).open(any(File.class), eq("foo-name")); } + + @SuppressWarnings("unchecked") + @Test + public void instantiationFailureOfChannelOpeningUsesNoopChannelInstead() { + LocalSocketTransport t = null; + LocalSocketChannelFactory failFactory = mock(LocalSocketChannelFactory.class); + when(failFactory.open(any(File.class), any(String.class))).thenThrow(RuntimeException.class); + t = new LocalSocketTransport(0, 0, mock(File.class), "foo-name", 10, 1, 100, failFactory); + LocalSocketChannel channel = t.getChannel(); + assertTrue(channel instanceof NoopLocalSocketChannel); + t.close(); + } @Test public void sendWritesToChannelInBatches() throws InterruptedException, IOException {