changeset 2297:524aeb3ae995

Fix resource leak with Byteman plugin Reviewed-by: jerboaa, neugens Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2016-May/018811.html PR2955
author Elliott Baron <ebaron@redhat.com>
date Fri, 13 May 2016 12:28:07 -0400
parents 7596994808a2
children a9decacf1601
files 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.java vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransport.java vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/ThermostatHelperTest.java
diffstat 4 files changed, 40 insertions(+), 21 deletions(-) [+]
line wrap: on
line diff
--- a/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/ThermostatHelper.java	Thu May 12 15:46:30 2016 +0200
+++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/ThermostatHelper.java	Fri May 13 12:28:07 2016 -0400
@@ -50,18 +50,8 @@
  */
 public class ThermostatHelper extends Helper {
     
-    private final Transport transport;
-
-    /**
-     * Constructor
-     */
-    protected ThermostatHelper(Rule rule) {
-        this(rule, new TransportFactory().create());
-    }
-    
-    ThermostatHelper(final Rule rule, final Transport transport) {
-        super(rule);
-        this.transport = transport;
+    private static Transport transport = new TransportFactory().create();
+    static {
         Runtime.getRuntime().addShutdownHook(new Thread(new Runnable() {
             @Override
             public void run() {
@@ -70,6 +60,13 @@
         }));
     }
 
+    /**
+     * Constructor
+     */
+    protected ThermostatHelper(Rule rule) {
+        super(rule);
+    }
+    
     public void send(String marker, String key, String value) {
         send(marker, new Object[]{key, value});
     }
@@ -104,5 +101,10 @@
         BytemanMetric rec = new BytemanMetric(marker, data);
         transport.send(rec);
     }
+    
+    // For testing purposes
+    static void setTransport(Transport transport) {
+        ThermostatHelper.transport = transport;
+    }
 
 }
--- a/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/Transport.java	Thu May 12 15:46:30 2016 +0200
+++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/Transport.java	Fri May 13 12:28:07 2016 -0400
@@ -38,9 +38,10 @@
 
 import java.io.Closeable;
 import java.util.ArrayList;
-import java.util.concurrent.Executor;
+import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 
@@ -63,7 +64,7 @@
     private final Object cacheLock = new Object();
     private long lostCount = 0;
     // executor
-    private final Executor executor = Executors.newSingleThreadExecutor(new ThermostatThreadFactory("thermostat"));
+    private final ExecutorService executor = Executors.newSingleThreadExecutor(new ThermostatThreadFactory("thermostat"));
 
     /**
      * Constructor for inheritors
@@ -114,6 +115,13 @@
                 task.run();
             }
         }
+        // Shut down executor and wait (with a timeout) for it to finish
+        executor.shutdown();
+        try {
+            executor.awaitTermination(5000, TimeUnit.MILLISECONDS);
+        } catch (InterruptedException e) {
+            Thread.currentThread().interrupt();
+        }
     }
 
     /**
--- a/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransport.java	Thu May 12 15:46:30 2016 +0200
+++ b/vm-byteman/byteman-helper/src/main/java/org/jboss/byteman/thermostat/helper/transport/ipc/LocalSocketTransport.java	Fri May 13 12:28:07 2016 -0400
@@ -118,7 +118,9 @@
     public void close() {
         super.close();
         try {
-            channel.close();
+            synchronized (channel) {
+                channel.close();
+            }
         } catch (IOException e) {
             System.err.println("WARNING: Thermostat error closing socket channel: [" + socketName + "]");
             e.printStackTrace();
@@ -153,7 +155,9 @@
         }
         sb.append("]");
         ByteBuffer envelope = ByteBuffer.wrap(sb.toString().getBytes(Charset.forName("UTF-8")));
-        channel.write(envelope);
+        synchronized (channel) {
+            channel.write(envelope);
+        }
     }
 
     // package-private for testing
--- a/vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/ThermostatHelperTest.java	Thu May 12 15:46:30 2016 +0200
+++ b/vm-byteman/byteman-helper/src/test/java/org/jboss/byteman/thermostat/helper/ThermostatHelperTest.java	Fri May 13 12:28:07 2016 -0400
@@ -49,10 +49,8 @@
 import java.util.concurrent.CountDownLatch;
 
 import org.jboss.byteman.rule.Rule;
-import org.jboss.byteman.thermostat.helper.BytemanMetric;
-import org.jboss.byteman.thermostat.helper.ThermostatHelper;
-import org.jboss.byteman.thermostat.helper.Transport;
 import org.jboss.byteman.thermostat.helper.transport.stdout.TestStdOutTransport;
+import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -66,7 +64,13 @@
     @Before
     public void setup() {
         mockTransport = mock(Transport.class);
-        helper = new ThermostatHelper(mock(Rule.class), mockTransport);
+        ThermostatHelper.setTransport(mockTransport);
+        helper = new ThermostatHelper(mock(Rule.class));
+    }
+    
+    @After
+    public void teardown() {
+        ThermostatHelper.setTransport(new TransportFactory().create());
     }
     
     @Test
@@ -158,7 +162,8 @@
         final PrintStream oldOut = System.out;
         final CountDownLatch latch = new CountDownLatch(1);
         Transport stdoutTransport = new TestStdOutTransport(latch);
-        ThermostatHelper helperWithStdout = new ThermostatHelper(mock(Rule.class), stdoutTransport);
+        ThermostatHelper.setTransport(stdoutTransport);
+        ThermostatHelper helperWithStdout = new ThermostatHelper(mock(Rule.class));
         ByteArrayOutputStream bout = new ByteArrayOutputStream();
         PrintStream testSysout = new PrintStream(bout);
         System.setOut(testSysout);