Merge pull request #38 from fluent/issue37

Fix FluentLoggerFactory to hold FluentLogger(s) over GC.
fluent · Dec 7, 2014 · 857778c · 857778c
2 parents 1d131e5 + 7cb9077
commit 857778c
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 10 deletions.
diff --git a/src/main/java/org/fluentd/logger/FluentLogger.java b/src/main/java/org/fluentd/logger/FluentLogger.java
@@ -48,7 +48,7 @@ public static synchronized FluentLogger getLogger(String tagPrefix, String host,
     /**
      * the method is for testing
      */
-    static Map<String, FluentLogger> getLoggers() {
+    static Map<FluentLogger, String> getLoggers() {
         return factory.getLoggers();
     }
 

diff --git a/src/main/java/org/fluentd/logger/FluentLoggerFactory.java b/src/main/java/org/fluentd/logger/FluentLoggerFactory.java
@@ -30,10 +30,10 @@
 
 public class FluentLoggerFactory {
 
-    private final Map<String, FluentLogger> loggers;
+    private final Map<FluentLogger, String> loggers;
 
     public FluentLoggerFactory() {
-        loggers = new WeakHashMap<String, FluentLogger>();
+        loggers = new WeakHashMap<FluentLogger, String>();
     }
 
     public FluentLogger getLogger(String tagPrefix) {
@@ -51,8 +51,13 @@ public FluentLogger getLogger(String tagPrefix, String host, int port, int timeo
     public synchronized FluentLogger getLogger(String tagPrefix, String host, int port, int timeout, int bufferCapacity,
             Reconnector reconnector) {
         String key = String.format("%s_%s_%d_%d_%d", new Object[] { tagPrefix, host, port, timeout, bufferCapacity });
-        if (loggers.containsKey(key)) {
-            return loggers.get(key);
+        if (loggers.containsValue(key)) {
+            for (Map.Entry<FluentLogger, String> entry : loggers.entrySet()) {
+                if (entry.getValue().equals(key)) {
+                    return entry.getKey();
+                }
+            }
+            throw new IllegalStateException();
         } else {
             Sender sender = null;
             Properties props = System.getProperties();
@@ -68,7 +73,7 @@ public synchronized FluentLogger getLogger(String tagPrefix, String host, int po
                 }
             }
             FluentLogger logger = new FluentLogger(tagPrefix, sender);
-            loggers.put(key, logger);
+            loggers.put(logger, key);
             return logger;
         }
     }
@@ -86,19 +91,19 @@ private Sender createSenderInstance(final String className, final Object[] param
     /**
      * the method is for testing
      */
-    Map<String, FluentLogger> getLoggers() {
+    Map<FluentLogger, String> getLoggers() {
         return loggers;
     }
 
     public synchronized void closeAll() {
-        for (FluentLogger logger : loggers.values()) {
+        for (FluentLogger logger : loggers.keySet()) {
             logger.close();
         }
         loggers.clear();
     }
 
     public synchronized void flushAll() {
-        for (FluentLogger logger : loggers.values()) {
+        for (FluentLogger logger : loggers.keySet()) {
             logger.flush();
         }
     }

diff --git a/src/test/java/org/fluentd/logger/TestFluentLogFactory.java b/src/test/java/org/fluentd/logger/TestFluentLogFactory.java
@@ -0,0 +1,50 @@
+package org.fluentd.logger;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.concurrent.TimeUnit;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class TestFluentLogFactory {
+    private FluentLoggerFactory loggerFactory;
+
+    @Before
+    public void setup() {
+        loggerFactory = new FluentLoggerFactory();
+    }
+
+    @Test
+    public void testGetLogger() {
+        FluentLogger loggerA0 = loggerFactory.getLogger("tagprefix_a");
+        FluentLogger loggerA1 = loggerFactory.getLogger("tagprefix_a");
+        FluentLogger loggerB0 = loggerFactory.getLogger("tagprefix_b");
+        FluentLogger loggerA_lh0 = loggerFactory.getLogger("tagprefix_a", "localhost", 1234);
+        FluentLogger loggerA_lh1 = loggerFactory.getLogger("tagprefix_a", "127.0.0.1", 1234);
+        assertTrue(loggerA0 == loggerA1);
+        assertTrue(loggerA0 != loggerB0);
+        assertTrue(loggerA0 != loggerA_lh0);
+        assertTrue(loggerA_lh0 != loggerA_lh1);
+    }
+
+    @Test
+    public void testItHoldsLoggersOverGC() throws InterruptedException {
+        ArrayList<FluentLogger> loggers = new ArrayList<FluentLogger>();
+        for(int i=0; i<100; i++) {
+            loggers.add(loggerFactory.getLogger("testtag" + i, "localhost", 999));
+        }
+        System.gc();
+        Thread.sleep(1000);
+        assertEquals(loggers.size(), loggerFactory.getLoggers().size());
+
+        FluentLogger head = loggers.get(0);
+        FluentLogger tail = loggers.get(loggers.size() - 1);
+        loggers.clear();
+        System.gc();
+        Thread.sleep(1000);
+        assertEquals(2, loggerFactory.getLoggers().size());
+    }
+}
diff --git a/src/test/java/org/fluentd/logger/TestFluentLogger.java b/src/test/java/org/fluentd/logger/TestFluentLogger.java
@@ -349,7 +349,7 @@ public void testClose() throws Exception {
         FluentLogger.getLogger("tag2");
         FluentLogger.getLogger("tag3");
 
-        Map<String, FluentLogger> loggers;
+        Map<FluentLogger, String> loggers;
         {
             loggers = FluentLogger.getLoggers();
             assertEquals(3, loggers.size());