Merge pull request #69 from pxcurtis/bugfix/issue64

Bugfix for issue #64; purge closed loggers from factory cache

Author: komamitsu

src/main/java/org/fluentd/logger/FluentLogger.java

@@ -112,6 +112,7 @@ public void close() {
             sender.close();
             sender = null;
         }
+        factory.purgeLogger(this);
     }
 
     public String getName() {

src/main/java/org/fluentd/logger/FluentLoggerFactory.java

@@ -19,7 +19,10 @@
 
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
+import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.WeakHashMap;
 
@@ -80,6 +83,18 @@ public synchronized FluentLogger getLogger(String tagPrefix, String host, int po
         return logger;
     }
 
+    /** Purges an invalid logger from the cache.
+     */
+	protected synchronized void purgeLogger(FluentLogger logger) {
+		Iterator<Entry<FluentLogger, String>> it = loggers.entrySet().iterator();
+		while (it.hasNext()) {
+			if (it.next().getKey() == logger) {
+				it.remove();
+				return;
+			}
+		}
+	}
+
     @SuppressWarnings("unchecked")
     private Sender createSenderInstance(final String className, final Object[] params) throws ClassNotFoundException,
             SecurityException, NoSuchMethodException, IllegalArgumentException, InstantiationException,
@@ -98,7 +113,7 @@ Map<FluentLogger, String> getLoggers() {
     }
 
     public synchronized void closeAll() {
-        for (FluentLogger logger : loggers.keySet()) {
+        for (FluentLogger logger : new ArrayList<FluentLogger>(loggers.keySet())) {
             logger.close();
         }
         loggers.clear();

src/test/java/org/fluentd/logger/TestBugfixes.java

@@ -0,0 +1,31 @@
+package org.fluentd.logger;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.Collections;
+
+import org.junit.Test;
+
+public class TestBugfixes {
+	/** Prior to the issue fix, this test fails with an NPE on the last line.
+	 */
+	@Test
+	public void validLoggerReturned_whenOpenThenCloseThenOpenWithSameParameters() {
+		// use test sender so we don't need to have an actual fluentd running...
+		System.setProperty(Config.FLUENT_SENDER_CLASS, "org.fluentd.logger.sender.NullSender");		
+		FluentLogger logger = FluentLogger.getLogger("test");
+
+		// this works
+		logger.log("tag", Collections.<String, Object>emptyMap());
+		
+		// now close it; sender is closed and set to null	
+		logger.close();
+		assertEquals(null, logger.sender);
+		
+		// get another logger with the exact same parameters; we'd expect this to work, yes?
+		FluentLogger logger2 = FluentLogger.getLogger("test");
+
+		// let's see if it does
+		logger2.log("tag", Collections.<String, Object>emptyMap());
+	}
+}