use a ConcurrentHashMap in various loger factories - fix bug #298
diff --git a/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLogFactory.java b/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLogFactory.java
index 14927b5..c46b457 100644
--- a/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLogFactory.java
+++ b/jcl-over-slf4j/src/main/java/org/apache/commons/logging/impl/SLF4JLogFactory.java
@@ -16,13 +16,6 @@
package org.apache.commons.logging.impl;
-import java.util.ArrayList;
-import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.Hashtable;
-import java.util.List;
-import java.util.Map;
-
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogConfigurationException;
import org.apache.commons.logging.LogFactory;
@@ -30,6 +23,13 @@
import org.slf4j.LoggerFactory;
import org.slf4j.spi.LocationAwareLogger;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.Hashtable;
+import java.util.List;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
/**
* <p>
* Concrete subclass of {@link LogFactory} which always delegates to the
@@ -58,13 +58,13 @@
* The {@link org.apache.commons.logging.Log}instances that have already been
* created, keyed by logger name.
*/
- Map loggerMap;
+ ConcurrentMap<String, Log> loggerMap;
/**
* Public no-arguments constructor required by the lookup mechanism.
*/
public SLF4JLogFactory() {
- loggerMap = new HashMap();
+ loggerMap = new ConcurrentHashMap<String, Log>();
}
// ----------------------------------------------------- Manifest Constants
@@ -148,22 +148,20 @@
* if a suitable <code>Log</code> instance cannot be returned
*/
public Log getInstance(String name) throws LogConfigurationException {
- Log instance = null;
- // protect against concurrent access of loggerMap
- synchronized (loggerMap) {
- instance = (Log) loggerMap.get(name);
- if (instance == null) {
- Logger logger = LoggerFactory.getLogger(name);
- if(logger instanceof LocationAwareLogger) {
- instance = new SLF4JLocationAwareLog((LocationAwareLogger) logger);
- } else {
- instance = new SLF4JLog(logger);
- }
- loggerMap.put(name, instance);
+ Log instance = loggerMap.get(name);
+ if (instance != null) {
+ return instance;
+ } else {
+ Log newInstance;
+ Logger slf4jLogger = LoggerFactory.getLogger(name);
+ if (slf4jLogger instanceof LocationAwareLogger) {
+ newInstance = new SLF4JLocationAwareLog((LocationAwareLogger) slf4jLogger);
+ } else {
+ newInstance = new SLF4JLog(slf4jLogger);
}
+ Log oldInstance = loggerMap.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
}
- return (instance);
-
}
/**
diff --git a/log4j-over-slf4j/src/main/java/org/apache/log4j/Log4jLoggerFactory.java b/log4j-over-slf4j/src/main/java/org/apache/log4j/Log4jLoggerFactory.java
index a0e38c5..8f591a5 100644
--- a/log4j-over-slf4j/src/main/java/org/apache/log4j/Log4jLoggerFactory.java
+++ b/log4j-over-slf4j/src/main/java/org/apache/log4j/Log4jLoggerFactory.java
@@ -16,63 +16,63 @@
package org.apache.log4j;
-import java.util.Hashtable;
-
import org.apache.log4j.spi.LoggerFactory;
import org.slf4j.helpers.Util;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+
/**
* This class is a factory that creates and maintains org.apache.log4j.Loggers
* wrapping org.slf4j.Loggers.
- *
+ *
* It keeps a hashtable of all created org.apache.log4j.Logger instances so that
- * all newly created instances are not dulpicates of existing loggers.
- *
+ * all newly created instances are not duplicates of existing loggers.
+ *
* @author Sébastien Pennec
*/
class Log4jLoggerFactory {
// String, Logger
- private static Hashtable log4jLoggers = new Hashtable();
+ private static ConcurrentMap<String, Logger> log4jLoggers = new ConcurrentHashMap<String, Logger>();
private static final String LOG4J_DELEGATION_LOOP_URL = "http://www.slf4j.org/codes.html#log4jDelegationLoop";
-
+
// check for delegation loops
static {
try {
Class.forName("org.slf4j.impl.Log4jLoggerFactory");
String part1 = "Detected both log4j-over-slf4j.jar AND slf4j-log4j12.jar on the class path, preempting StackOverflowError. ";
String part2 = "See also " + LOG4J_DELEGATION_LOOP_URL
- + " for more details.";
+ + " for more details.";
Util.report(part1);
Util.report(part2);
- throw new IllegalStateException(part1 + part2);
+ throw new IllegalStateException(part1 + part2);
} catch (ClassNotFoundException e) {
// this is the good case
}
}
- public static synchronized Logger getLogger(String name) {
- if (log4jLoggers.containsKey(name)) {
- return (org.apache.log4j.Logger) log4jLoggers.get(name);
+ public static Logger getLogger(String name) {
+ org.apache.log4j.Logger instance = log4jLoggers.get(name);
+ if (instance != null) {
+ return instance;
} else {
- Logger log4jLogger = new Logger(name);
-
- log4jLoggers.put(name, log4jLogger);
- return log4jLogger;
+ Logger newInstance = new Logger(name);
+ Logger oldInstance = log4jLoggers.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
}
}
-
- public static synchronized Logger getLogger(String name, LoggerFactory loggerFactory) {
- if (log4jLoggers.containsKey(name)) {
- return (org.apache.log4j.Logger) log4jLoggers.get(name);
- } else {
- Logger log4jLogger = loggerFactory.makeNewLoggerInstance(name);
- log4jLoggers.put(name, log4jLogger);
- return log4jLogger;
- }
+ public static Logger getLogger(String name, LoggerFactory loggerFactory) {
+ org.apache.log4j.Logger instance = log4jLoggers.get(name);
+ if (instance != null) {
+ return instance;
+ } else {
+ Logger newInstance = loggerFactory.makeNewLoggerInstance(name);
+ Logger oldInstance = log4jLoggers.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
+ }
}
-
}
diff --git a/slf4j-jcl/src/main/java/org/slf4j/impl/JCLLoggerFactory.java b/slf4j-jcl/src/main/java/org/slf4j/impl/JCLLoggerFactory.java
index 263e694..9bc092a 100644
--- a/slf4j-jcl/src/main/java/org/slf4j/impl/JCLLoggerFactory.java
+++ b/slf4j-jcl/src/main/java/org/slf4j/impl/JCLLoggerFactory.java
@@ -26,6 +26,8 @@
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import org.apache.commons.logging.LogFactory;
import org.slf4j.ILoggerFactory;
@@ -59,10 +61,10 @@
}
// key: name (String), value: a JCLLoggerAdapter;
- Map loggerMap;
+ ConcurrentMap<String, Logger> loggerMap;
public JCLLoggerFactory() {
- loggerMap = new HashMap();
+ loggerMap = new ConcurrentHashMap<String, Logger>();
}
/*
@@ -71,16 +73,14 @@
* @see org.slf4j.ILoggerFactory#getLogger(java.lang.String)
*/
public Logger getLogger(String name) {
- Logger logger = null;
- // protect against concurrent access of loggerMap
- synchronized (this) {
- logger = (Logger) loggerMap.get(name);
- if (logger == null) {
- org.apache.commons.logging.Log jclLogger = LogFactory.getLog(name);
- logger = new JCLLoggerAdapter(jclLogger, name);
- loggerMap.put(name, logger);
- }
+ Logger slf4jLogger = loggerMap.get(name);
+ if (slf4jLogger != null) {
+ return slf4jLogger;
+ } else {
+ org.apache.commons.logging.Log jclLogger = LogFactory.getLog(name);
+ Logger newInstance = new JCLLoggerAdapter(jclLogger, name);
+ Logger oldInstance = loggerMap.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
}
- return logger;
}
}
diff --git a/slf4j-jdk14/src/main/java/org/slf4j/impl/JDK14LoggerFactory.java b/slf4j-jdk14/src/main/java/org/slf4j/impl/JDK14LoggerFactory.java
index 335364d..b7aaec9 100644
--- a/slf4j-jdk14/src/main/java/org/slf4j/impl/JDK14LoggerFactory.java
+++ b/slf4j-jdk14/src/main/java/org/slf4j/impl/JDK14LoggerFactory.java
@@ -27,8 +27,8 @@
import org.slf4j.Logger;
import org.slf4j.ILoggerFactory;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
/**
* JDK14LoggerFactory is an implementation of {@link ILoggerFactory} returning
@@ -39,10 +39,10 @@
public class JDK14LoggerFactory implements ILoggerFactory {
// key: name (String), value: a JDK14LoggerAdapter;
- Map loggerMap;
+ ConcurrentMap<String, Logger> loggerMap;
public JDK14LoggerFactory() {
- loggerMap = new HashMap();
+ loggerMap =new ConcurrentHashMap<String, Logger>();
}
/*
@@ -51,21 +51,20 @@
* @see org.slf4j.ILoggerFactory#getLogger(java.lang.String)
*/
public synchronized Logger getLogger(String name) {
- Logger ulogger = null;
- // protect against concurrent access of loggerMap
- synchronized (this) {
- // the root logger is called "" in JUL
- if(name.equalsIgnoreCase(Logger.ROOT_LOGGER_NAME)) {
- name = "";
- }
- ulogger = (Logger) loggerMap.get(name);
- if (ulogger == null) {
- java.util.logging.Logger logger = java.util.logging.Logger
- .getLogger(name);
- ulogger = new JDK14LoggerAdapter(logger);
- loggerMap.put(name, ulogger);
- }
+ // the root logger is called "" in JUL
+ if(name.equalsIgnoreCase(Logger.ROOT_LOGGER_NAME)) {
+ name = "";
}
- return ulogger;
+
+ Logger slf4jLogger = loggerMap.get(name);
+ if (slf4jLogger != null)
+ return slf4jLogger;
+ else {
+ java.util.logging.Logger julLogger = java.util.logging.Logger
+ .getLogger(name);
+ Logger newInstance = new JDK14LoggerAdapter(julLogger);
+ Logger oldInstance = loggerMap.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
+ }
}
}
diff --git a/slf4j-log4j12/src/main/java/org/slf4j/impl/Log4jLoggerFactory.java b/slf4j-log4j12/src/main/java/org/slf4j/impl/Log4jLoggerFactory.java
index d5a7be7..d560ce3 100644
--- a/slf4j-log4j12/src/main/java/org/slf4j/impl/Log4jLoggerFactory.java
+++ b/slf4j-log4j12/src/main/java/org/slf4j/impl/Log4jLoggerFactory.java
@@ -26,6 +26,8 @@
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import org.apache.log4j.LogManager;
import org.slf4j.ILoggerFactory;
@@ -40,10 +42,11 @@
public class Log4jLoggerFactory implements ILoggerFactory {
// key: name (String), value: a Log4jLoggerAdapter;
- Map loggerMap;
+ ConcurrentMap<String, Logger> loggerMap;
+
public Log4jLoggerFactory() {
- loggerMap = new HashMap();
+ loggerMap = new ConcurrentHashMap<String, Logger>();
}
/*
@@ -52,21 +55,19 @@
* @see org.slf4j.ILoggerFactory#getLogger(java.lang.String)
*/
public Logger getLogger(String name) {
- Logger slf4jLogger = null;
- // protect against concurrent access of loggerMap
- synchronized (this) {
- slf4jLogger = (Logger) loggerMap.get(name);
- if (slf4jLogger == null) {
- org.apache.log4j.Logger log4jLogger;
- if(name.equalsIgnoreCase(Logger.ROOT_LOGGER_NAME)) {
- log4jLogger = LogManager.getRootLogger();
- } else {
- log4jLogger = LogManager.getLogger(name);
- }
- slf4jLogger = new Log4jLoggerAdapter(log4jLogger);
- loggerMap.put(name, slf4jLogger);
- }
+ Logger slf4jLogger = loggerMap.get(name);
+ if (slf4jLogger != null) {
+ return slf4jLogger;
+ } else {
+ org.apache.log4j.Logger log4jLogger;
+ if(name.equalsIgnoreCase(Logger.ROOT_LOGGER_NAME))
+ log4jLogger = LogManager.getRootLogger();
+ else
+ log4jLogger = LogManager.getLogger(name);
+
+ Logger newInstance = new Log4jLoggerAdapter(log4jLogger);
+ Logger oldInstance = loggerMap.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
}
- return slf4jLogger;
}
}
diff --git a/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLoggerFactory.java b/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLoggerFactory.java
index 94a027c..764bd78 100644
--- a/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLoggerFactory.java
+++ b/slf4j-simple/src/main/java/org/slf4j/impl/SimpleLoggerFactory.java
@@ -26,6 +26,8 @@
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
import org.slf4j.Logger;
import org.slf4j.ILoggerFactory;
@@ -38,26 +40,24 @@
*/
public class SimpleLoggerFactory implements ILoggerFactory {
- Map loggerMap;
+ ConcurrentMap<String, Logger> loggerMap;
public SimpleLoggerFactory() {
- loggerMap = new HashMap();
+ loggerMap = new ConcurrentHashMap<String, Logger>();
}
/**
* Return an appropriate {@link SimpleLogger} instance by name.
*/
public Logger getLogger(String name) {
- Logger slogger = null;
- // protect against concurrent access of the loggerMap
- synchronized (this) {
- slogger = (Logger) loggerMap.get(name);
- if (slogger == null) {
- slogger = new SimpleLogger(name);
- loggerMap.put(name, slogger);
- }
+ Logger simpleLogger = loggerMap.get(name);
+ if (simpleLogger != null) {
+ return simpleLogger;
+ } else {
+ Logger newInstance = new SimpleLogger(name);
+ Logger oldInstance = loggerMap.putIfAbsent(name, newInstance);
+ return oldInstance == null ? newInstance : oldInstance;
}
- return slogger;
}
/**