#814 always close socket

(cherry picked from commit 2762ce5)
dropwizard · Jan 5, 2017 · 5a0e903 · 5a0e903
1 parent 9ba1053
commit 5a0e903
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 18 deletions.
diff --git a/metrics-graphite/src/main/java/com/codahale/metrics/graphite/Graphite.java b/metrics-graphite/src/main/java/com/codahale/metrics/graphite/Graphite.java
@@ -1,5 +1,8 @@
 package com.codahale.metrics.graphite;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import javax.net.SocketFactory;
 
 import java.io.*;
@@ -27,6 +30,8 @@ public class Graphite implements GraphiteSender {
     private Writer writer;
     private int failures;
 
+    private static final Logger LOGGER = LoggerFactory.getLogger(Graphite.class);
+
     /**
      * Creates a new client which connects to the given address using the default
      * {@link SocketFactory}.
@@ -159,12 +164,19 @@ public void close() throws IOException {
                 writer.close();
             }
         } catch (IOException ex) {
+            LOGGER.debug("Error closing writer", ex);
+        } finally {
+            this.writer = null;
+        }
+
+        try {
             if (socket != null) {
                 socket.close();
             }
+        } catch (IOException ex) {
+            LOGGER.debug("Error closing socket", ex);
         } finally {
             this.socket = null;
-            this.writer = null;
         }
     }
 

diff --git a/metrics-graphite/src/main/java/com/codahale/metrics/graphite/GraphiteReporter.java b/metrics-graphite/src/main/java/com/codahale/metrics/graphite/GraphiteReporter.java
@@ -165,9 +165,7 @@ public void report(SortedMap<String, Gauge> gauges,
 
         // oh it'd be lovely to use Java 7 here
         try {
-            if (!graphite.isConnected()) {
-    	          graphite.connect();
-            }
+            graphite.connect();
 
             for (Map.Entry<String, Gauge> entry : gauges.entrySet()) {
                 reportGauge(entry.getKey(), entry.getValue(), timestamp);
@@ -188,10 +186,10 @@ public void report(SortedMap<String, Gauge> gauges,
             for (Map.Entry<String, Timer> entry : timers.entrySet()) {
                 reportTimer(entry.getKey(), entry.getValue(), timestamp);
             }
-
             graphite.flush();
         } catch (IOException e) {
             LOGGER.warn("Unable to report to Graphite", graphite, e);
+        } finally {
             try {
                 graphite.close();
             } catch (IOException e1) {

diff --git a/metrics-graphite/src/test/java/com/codahale/metrics/graphite/GraphiteReporterTest.java b/metrics-graphite/src/test/java/com/codahale/metrics/graphite/GraphiteReporterTest.java
@@ -39,10 +39,10 @@ public void doesNotReportStringGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite, never()).send("prefix.gauge", "value", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -56,10 +56,10 @@ public void reportsByteGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.gauge", "1", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -73,10 +73,10 @@ public void reportsShortGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.gauge", "1", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -90,10 +90,10 @@ public void reportsIntegerGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.gauge", "1", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -107,10 +107,10 @@ public void reportsLongGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.gauge", "1", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -124,10 +124,10 @@ public void reportsFloatGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.gauge", "1.10", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -141,10 +141,10 @@ public void reportsDoubleGaugeValues() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.gauge", "1.10", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -161,10 +161,10 @@ public void reportsCounters() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.counter.count", "100", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -195,7 +195,6 @@ public void reportsHistograms() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.histogram.count", "1", timestamp);
         inOrder.verify(graphite).send("prefix.histogram.max", "2", timestamp);
@@ -209,6 +208,7 @@ public void reportsHistograms() throws Exception {
         inOrder.verify(graphite).send("prefix.histogram.p99", "10.00", timestamp);
         inOrder.verify(graphite).send("prefix.histogram.p999", "11.00", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -229,14 +229,14 @@ public void reportsMeters() throws Exception {
                         this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.meter.count", "1", timestamp);
         inOrder.verify(graphite).send("prefix.meter.m1_rate", "2.00", timestamp);
         inOrder.verify(graphite).send("prefix.meter.m5_rate", "3.00", timestamp);
         inOrder.verify(graphite).send("prefix.meter.m15_rate", "4.00", timestamp);
         inOrder.verify(graphite).send("prefix.meter.mean_rate", "5.00", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -272,7 +272,6 @@ public void reportsTimers() throws Exception {
                         map("timer", timer));
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).send("prefix.timer.max", "100.00", timestamp);
         inOrder.verify(graphite).send("prefix.timer.mean", "200.00", timestamp);
@@ -290,6 +289,7 @@ public void reportsTimers() throws Exception {
         inOrder.verify(graphite).send("prefix.timer.m15_rate", "5.00", timestamp);
         inOrder.verify(graphite).send("prefix.timer.mean_rate", "2.00", timestamp);
         inOrder.verify(graphite).flush();
+        inOrder.verify(graphite).close();
 
         verifyNoMoreInteractions(graphite);
     }
@@ -304,10 +304,10 @@ public void closesConnectionIfGraphiteIsUnavailable() throws Exception {
             this.<Timer>map());
 
         final InOrder inOrder = inOrder(graphite);
-        inOrder.verify(graphite).isConnected();
         inOrder.verify(graphite).connect();
         inOrder.verify(graphite).close();
 
+
         verifyNoMoreInteractions(graphite);
     }
 

diff --git a/metrics-graphite/src/test/java/com/codahale/metrics/graphite/GraphiteTest.java b/metrics-graphite/src/test/java/com/codahale/metrics/graphite/GraphiteTest.java
@@ -103,7 +103,7 @@ public void disconnectsFromGraphite() throws Exception {
         graphite.connect();
         graphite.close();
 
-        verify(socket).close();
+        verify(socket, times(2)).close();
     }
 
     @Test