Author: markt Date: Thu Jan 30 09:17:13 2014 New Revision: 1562748 URL: http://svn.apache.org/r1562748 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56082 Fix thread safety issue in JULI's LogManager implementation
Modified: tomcat/tc7.0.x/trunk/ (props changed) tomcat/tc7.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java tomcat/tc7.0.x/trunk/test/org/apache/juli/TestClassLoaderLogManager.java tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Propchange: tomcat/tc7.0.x/trunk/ ------------------------------------------------------------------------------ Merged /tomcat/trunk:r1562742,1562746 Modified: tomcat/tc7.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java?rev=1562748&r1=1562747&r2=1562748&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java (original) +++ tomcat/tc7.0.x/trunk/java/org/apache/juli/ClassLoaderLogManager.java Thu Jan 30 09:17:13 2014 @@ -36,6 +36,7 @@ import java.util.Map; import java.util.Properties; import java.util.StringTokenizer; import java.util.WeakHashMap; +import java.util.concurrent.ConcurrentHashMap; import java.util.logging.Handler; import java.util.logging.Level; import java.util.logging.LogManager; @@ -704,7 +705,7 @@ public class ClassLoaderLogManager exten protected static final class ClassLoaderLogInfo { final LogNode rootNode; - final Map<String, Logger> loggers = new HashMap<String, Logger>(); + final Map<String, Logger> loggers = new ConcurrentHashMap<String, Logger>(); final Map<String, Handler> handlers = new HashMap<String, Handler>(); final Properties props = new Properties(); Modified: tomcat/tc7.0.x/trunk/test/org/apache/juli/TestClassLoaderLogManager.java URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/juli/TestClassLoaderLogManager.java?rev=1562748&r1=1562747&r2=1562748&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/test/org/apache/juli/TestClassLoaderLogManager.java (original) +++ tomcat/tc7.0.x/trunk/test/org/apache/juli/TestClassLoaderLogManager.java Thu Jan 30 09:17:13 2014 @@ -14,11 +14,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.apache.juli; -import static org.junit.Assert.assertEquals; +import java.util.Collections; +import java.util.Random; +import java.util.logging.LogManager; +import java.util.logging.Logger; +import org.junit.Assert; import org.junit.Test; /** @@ -29,33 +32,118 @@ public class TestClassLoaderLogManager { @Test public void testReplace() { ClassLoaderLogManager logManager = new ClassLoaderLogManager(); - assertEquals("", logManager.replace("")); - assertEquals("${", logManager.replace("${")); - assertEquals("${undefinedproperty}", + Assert.assertEquals("", logManager.replace("")); + Assert.assertEquals("${", logManager.replace("${")); + Assert.assertEquals("${undefinedproperty}", logManager.replace("${undefinedproperty}")); - assertEquals( + Assert.assertEquals( System.getProperty("line.separator") + System.getProperty("path.separator") + System.getProperty("file.separator"), logManager .replace("${line.separator}${path.separator}${file.separator}")); - assertEquals( + Assert.assertEquals( "foo" + System.getProperty("file.separator") + "bar" + System.getProperty("line.separator") + System.getProperty("path.separator") + "baz", logManager .replace("foo${file.separator}bar${line.separator}${path.separator}baz")); // BZ 51249 - assertEquals( + Assert.assertEquals( "%{file.separator}" + System.getProperty("file.separator"), logManager.replace("%{file.separator}${file.separator}")); - assertEquals( + Assert.assertEquals( System.getProperty("file.separator") + "${undefinedproperty}" + System.getProperty("file.separator"), logManager .replace("${file.separator}${undefinedproperty}${file.separator}")); - assertEquals("${}" + System.getProperty("path.separator"), + Assert.assertEquals("${}" + System.getProperty("path.separator"), logManager.replace("${}${path.separator}")); } + @Test + public void testBug56082() { + ClassLoaderLogManager logManager = new ClassLoaderLogManager(); + + LoggerCreateThread[] createThreads = new LoggerCreateThread[10]; + for (int i = 0; i < createThreads.length; i ++) { + createThreads[i] = new LoggerCreateThread(logManager); + createThreads[i].setName("LoggerCreate-" + i); + createThreads[i].start(); + } + + LoggerListThread listThread = new LoggerListThread(logManager); + listThread.setName("LoggerList"); + listThread.start(); + + int count = 0; + while (count < 4 && listThread.isAlive()) { + try { + Thread.sleep(500); + } catch (InterruptedException e) { + // Ignore + } + count++; + } + + for (int i = 0; i < createThreads.length; i ++) { + createThreads[i].setRunning(false); + } + + Assert.assertTrue(listThread.isRunning()); + listThread.setRunning(false); + } + + private static class LoggerCreateThread extends Thread { + + private final LogManager logManager; + private volatile boolean running = true; + + public LoggerCreateThread(LogManager logManager) { + this.logManager = logManager; + } + + @Override + public void run() { + Random r = new Random(); + while (running) { + Logger logger = Logger.getLogger("Bug56082-" + r.nextInt(100000)); + logManager.addLogger(logger); + } + } + + public void setRunning(boolean running) { + this.running = running; + } + } + + private static class LoggerListThread extends Thread { + + private final LogManager logManager; + private volatile boolean running = true; + + public LoggerListThread(LogManager logManager) { + this.logManager = logManager; + } + + @Override + public void run() { + while (running) { + try { + Collections.list(logManager.getLoggerNames()); + } catch (Exception e) { + e.printStackTrace(); + running = false; + } + } + } + + public boolean isRunning() { + return running; + } + + public void setRunning(boolean running) { + this.running = running; + } + } } Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1562748&r1=1562747&r2=1562748&view=diff ============================================================================== --- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Jan 30 09:17:13 2014 @@ -126,6 +126,10 @@ <code>org.apache.commons.logging.Log</code> but <code>org.apache.juli.logging.Log</code>. (kfujino) </fix> + <fix> + <bug>56082</bug>: Fix a concurrency bug in JULI's LogManager + implementation. (markt) + </fix> </changelog> </subsection> <subsection name="Coyote"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org