I looked at the code. It is currently holding a lock while it checks the file timestamp, so no it can’t be reconfigured twice. However, the performance could probably be improved by using double-checked locking instead.
Ralph On May 19, 2014, at 5:22 AM, Ralph Goers <[email protected]> wrote: > I didn't want to manage a pool but to do a fire and forget. > > As for multiple recon figurations, I don't know of the top of my head. I > don't want to introduce locking here but I will see if that is a problem. > > Sent from my iPad > >> On May 19, 2014, at 4:15 AM, Gary Gregory <[email protected]> wrote: >> >> Does this guarantee that only one concurrent reconfiguration happens at one >> any one time? Should it? Use a size 1 thread pool executor with a queue? >> >> Gary >> >> >> -------- Original message -------- >> From: [email protected] >> Date:05/19/2014 00:53 (GMT-05:00) >> To: [email protected] >> Subject: svn commit: r1595741 - in /logging/log4j/log4j2/trunk: >> log4j-core/src/main/java/org/apache/logging/log4j/core/config/ >> log4j-core/src/test/java/org/apache/logging/log4j/core/ >> log4j-core/src/test/java/org/apache/logging/log4j/core/config/ >> log4j-core/src/... >> >> Author: rgoers >> Date: Mon May 19 04:53:11 2014 >> New Revision: 1595741 >> >> URL: http://svn.apache.org/r1595741 >> Log: >> LOG4J2-620 - Perform reconfiguration in a separate thread to prevent >> deadlocks. >> >> Added: >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/ReconfigurationDeadlockTest.java >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/SomethingThatUsesLogging.java >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/UsesLoggingAppender.java >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/resources/reconfiguration-deadlock.xml >> Modified: >> >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/FileConfigurationMonitor.java >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/FileConfigTest.java >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java >> >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/TestConfigurator.java >> logging/log4j/log4j2/trunk/src/changes/changes.xml >> >> Modified: >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/FileConfigurationMonitor.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/FileConfigurationMonitor.java?rev=1595741&r1=1595740&r2=1595741&view=diff >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/FileConfigurationMonitor.java >> (original) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/config/FileConfigurationMonitor.java >> Mon May 19 04:53:11 2014 >> @@ -81,7 +81,9 @@ public class FileConfigurationMonitor im >> if (file.lastModified() > lastModified) { >> lastModified = file.lastModified(); >> for (final ConfigurationListener listener : listeners) { >> - listener.onChange(reconfigurable); >> + Thread thread = new Thread(new >> ReconfigurationWorker(listener, reconfigurable)); >> + thread.setDaemon(true); >> + thread.start(); >> } >> } >> } finally { >> @@ -89,4 +91,20 @@ public class FileConfigurationMonitor im >> } >> } >> } >> + >> + private class ReconfigurationWorker implements Runnable { >> + >> + private final ConfigurationListener listener; >> + private final Reconfigurable reconfigurable; >> + >> + public ReconfigurationWorker(ConfigurationListener listener, >> Reconfigurable reconfigurable) { >> + this.listener = listener; >> + this.reconfigurable = reconfigurable; >> + } >> + >> + @Override >> + public void run() { >> + listener.onChange(reconfigurable); >> + } >> + } >> } >> >> Modified: >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/FileConfigTest.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/FileConfigTest.java?rev=1595741&r1=1595740&r2=1595741&view=diff >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/FileConfigTest.java >> (original) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/FileConfigTest.java >> Mon May 19 04:53:11 2014 >> @@ -81,6 +81,7 @@ public class FileConfigTest { >> for (int i = 0; i < 17; ++i) { >> logger.debug("Reconfigure"); >> } >> + Thread.sleep(100); >> final Configuration cfg = ctx.getConfiguration(); >> assertNotNull("No configuration", cfg); >> assertTrue("Reconfiguration failed", cfg != config); >> >> Modified: >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java?rev=1595741&r1=1595740&r2=1595741&view=diff >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java >> (original) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/LoggerTest.java >> Mon May 19 04:53:11 2014 >> @@ -238,6 +238,7 @@ public class LoggerTest { >> for (int i = 0; i < 17; ++i) { >> logger.debug("Reconfigure"); >> } >> + Thread.sleep(100); >> final Configuration cfg = ctx.getConfiguration(); >> assertNotNull("No configuration", cfg); >> assertTrue("Reconfiguration failed", cfg != config); >> >> Added: >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/ReconfigurationDeadlockTest.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/ReconfigurationDeadlockTest.java?rev=1595741&view=auto >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/ReconfigurationDeadlockTest.java >> (added) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/ReconfigurationDeadlockTest.java >> Mon May 19 04:53:11 2014 >> @@ -0,0 +1,127 @@ >> +/* >> + * Licensed to the Apache Software Foundation (ASF) under one or more >> + * contributor license agreements. See the NOTICE file distributed with >> + * this work for additional information regarding copyright ownership. >> + * The ASF licenses this file to You under the Apache license, Version 2.0 >> + * (the "License"); you may not use this file except in compliance with >> + * the License. You may obtain a copy of the License at >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >> + * See the license for the specific language governing permissions and >> + * limitations under the license. >> + */ >> +package org.apache.logging.log4j.core.config; >> + >> +import org.apache.logging.log4j.LogManager; >> +import org.apache.logging.log4j.Logger; >> +import org.apache.logging.log4j.junit.InitialLoggerContext; >> +import org.apache.logging.log4j.message.ThreadDumpMessage; >> +import org.junit.Rule; >> +import org.junit.Test; >> + >> +import java.io.File; >> + >> +import static org.junit.Assert.assertTrue; >> + >> +/** >> + * >> + */ >> +public class ReconfigurationDeadlockTest { >> + >> + @Rule >> + public InitialLoggerContext init = new >> InitialLoggerContext("reconfiguration-deadlock.xml"); >> + private static final int THREAD_COUNT = 5; >> + private static final boolean[] finished = new boolean[THREAD_COUNT]; >> + private static LoggerThread[] threads = new LoggerThread[THREAD_COUNT]; >> + >> + @Test >> + public void testReconfig() throws InterruptedException { >> + >> + Updater updater = new Updater(); >> + for (int i = 0; i < THREAD_COUNT; ++i) { >> + threads[i] = new LoggerThread(i); >> + threads[i].setDaemon(true); >> + } >> + for (int i = 0; i < THREAD_COUNT; ++i) { >> + >> + threads[i].start(); >> + } >> + updater.setDaemon(true); >> + updater.start(); >> + Thread.sleep(100); >> + boolean stillWaiting = true; >> + for (int i = 0; i < 200; ++i) { >> + int index = 0; >> + for (; index < THREAD_COUNT; ++index) { >> + if (!finished[index]) { >> + break; >> + } >> + } >> + if (index == THREAD_COUNT) { >> + stillWaiting = false; >> + break; >> + } >> + Thread.sleep(100); >> + } >> + updater.shutdown = true; >> + if (stillWaiting) { >> + ThreadDumpMessage message = new ThreadDumpMessage("Waiting"); >> + System.err.print(message.getFormattedMessage()); >> + } >> + for (int i = 0; i < THREAD_COUNT; ++i) { >> + if (threads[i].isAlive()) { >> + threads[i].interrupt(); >> + } >> + } >> + assertTrue("loggerThread didn't finish", stillWaiting == false); >> + >> + } >> + >> + private class LoggerThread extends Thread { >> + >> + private final Logger logger = LogManager.getRootLogger(); >> + private final int index; >> + >> + public LoggerThread(int i) { >> + index = i; >> + } >> + @Override >> + public void run() { >> + int i = 0; >> + try { >> + for (i=0; i < 30; ++i) { >> + logger.error("Thread: " + index + ", Test: " + i++); >> + } >> + } catch (Exception ie) { >> + return; >> + } >> + finished[index] = true; >> + } >> + } >> + >> + private class Updater extends Thread { >> + >> + public volatile boolean shutdown = false; >> + >> + @Override >> + public void run() { >> + while (!shutdown) { >> + try { >> + Thread.sleep(1000); >> + } catch (InterruptedException e) { >> + e.printStackTrace(); >> + } >> + // for running from IDE >> + File file = new >> File("target/test-classes/reconfiguration-deadlock.xml"); >> + if (file.exists()) { >> + file.setLastModified(System.currentTimeMillis()); >> + } >> + } >> + } >> + } >> + >> +} >> >> Modified: >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/TestConfigurator.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/TestConfigurator.java?rev=1595741&r1=1595740&r2=1595741&view=diff >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/TestConfigurator.java >> (original) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/core/config/TestConfigurator.java >> Mon May 19 04:53:11 2014 >> @@ -235,6 +235,7 @@ public class TestConfigurator { >> for (int i = 0; i < 17; ++i) { >> logger.debug("Test message " + i); >> } >> + Thread.sleep(100); >> final Configuration newConfig = ctx.getConfiguration(); >> assertTrue("Configuration not reset", newConfig != config); >> Configurator.shutdown(ctx); >> >> Added: >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/SomethingThatUsesLogging.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/SomethingThatUsesLogging.java?rev=1595741&view=auto >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/SomethingThatUsesLogging.java >> (added) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/SomethingThatUsesLogging.java >> Mon May 19 04:53:11 2014 >> @@ -0,0 +1,36 @@ >> +/* >> + * Licensed to the Apache Software Foundation (ASF) under one or more >> + * contributor license agreements. See the NOTICE file distributed with >> + * this work for additional information regarding copyright ownership. >> + * The ASF licenses this file to You under the Apache license, Version 2.0 >> + * (the "License"); you may not use this file except in compliance with >> + * the License. You may obtain a copy of the License at >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >> + * See the license for the specific language governing permissions and >> + * limitations under the license. >> + */ >> +package org.apache.logging.log4j.test; >> + >> +import org.apache.logging.log4j.Level; >> +import org.apache.logging.log4j.LogManager; >> +import org.apache.logging.log4j.Logger; >> +/** >> + * >> + */ >> +public class SomethingThatUsesLogging { >> + >> + private Logger logger; >> + >> + public SomethingThatUsesLogging() { >> + logger = LogManager.getLogger(); >> + } >> + >> + public void doSomething() { >> + logger.isEnabled(Level.DEBUG); >> + } >> +} >> >> Added: >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/UsesLoggingAppender.java >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/UsesLoggingAppender.java?rev=1595741&view=auto >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/UsesLoggingAppender.java >> (added) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/java/org/apache/logging/log4j/test/appender/UsesLoggingAppender.java >> Mon May 19 04:53:11 2014 >> @@ -0,0 +1,69 @@ >> +/* >> + * Licensed to the Apache Software Foundation (ASF) under one or more >> + * contributor license agreements. See the NOTICE file distributed with >> + * this work for additional information regarding copyright ownership. >> + * The ASF licenses this file to You under the Apache license, Version 2.0 >> + * (the "License"); you may not use this file except in compliance with >> + * the License. You may obtain a copy of the License at >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, software >> + * distributed under the License is distributed on an "AS IS" BASIS, >> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. >> + * See the license for the specific language governing permissions and >> + * limitations under the license. >> + */ >> +package org.apache.logging.log4j.test.appender; >> + >> +import org.apache.logging.log4j.core.Filter; >> +import org.apache.logging.log4j.core.Layout; >> +import org.apache.logging.log4j.core.LogEvent; >> +import org.apache.logging.log4j.core.appender.AbstractAppender; >> +import org.apache.logging.log4j.core.config.plugins.Plugin; >> +import org.apache.logging.log4j.core.config.plugins.PluginAttribute; >> +import org.apache.logging.log4j.core.config.plugins.PluginElement; >> +import org.apache.logging.log4j.core.config.plugins.PluginFactory; >> +import org.apache.logging.log4j.test.SomethingThatUsesLogging; >> + >> +/** >> + * >> + */ >> +@Plugin(name = "UsesLoggingAppender", category = "Core", elementType = >> "appender", printObject = true) >> +public final class UsesLoggingAppender extends AbstractAppender { >> + >> + private SomethingThatUsesLogging thing; >> + >> + private UsesLoggingAppender(String name, Filter filter, Layout<?> >> layout, boolean ignoreExceptions) { >> + super(name, filter, layout, ignoreExceptions); >> + thing = new SomethingThatUsesLogging(); >> + } >> + >> + @PluginFactory >> + public static UsesLoggingAppender >> createAppender(@PluginAttribute("name") String name, >> + >> @PluginAttribute("ignoreExceptions") String ignore, >> + @PluginElement("Layout") >> Layout<?> layout, >> + @PluginElement("Filters") >> Filter filter) { >> + >> + boolean ignoreExceptions = Boolean.parseBoolean(ignore); >> + >> + if (name == null) { >> + LOGGER.error("No name provided for MyAppender"); >> + return null; >> + } >> + >> + return new UsesLoggingAppender(name, filter, layout, >> ignoreExceptions); >> + } >> + >> + public void append(LogEvent event) { >> + try { >> + for (int i = 0; i < 50; i++) { >> + Thread.sleep(10); >> + thing.doSomething(); >> + } >> + } catch (InterruptedException e) { >> + e.printStackTrace(); >> + } >> + // System.out.print("Log: " + getLayout().toSerializable(event)); >> + } >> +} >> >> Added: >> logging/log4j/log4j2/trunk/log4j-core/src/test/resources/reconfiguration-deadlock.xml >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/test/resources/reconfiguration-deadlock.xml?rev=1595741&view=auto >> ============================================================================== >> --- >> logging/log4j/log4j2/trunk/log4j-core/src/test/resources/reconfiguration-deadlock.xml >> (added) >> +++ >> logging/log4j/log4j2/trunk/log4j-core/src/test/resources/reconfiguration-deadlock.xml >> Mon May 19 04:53:11 2014 >> @@ -0,0 +1,19 @@ >> +<?xml version="1.0" encoding="UTF-8"?> >> +<Configuration status="error" monitorInterval="5"> >> + <Appenders> >> + <UsesLoggingAppender name="myAppender"> >> + <PatternLayout pattern="%d{HH:mm:ss.SSS} [%t] %-5level %logger{36} - >> %msg%n" /> >> + </UsesLoggingAppender> >> + <Console name="STDOUT"> >> + <PatternLayout pattern="%m%n"/> >> + </Console> >> + </Appenders> >> + <Loggers> >> + <Logger name="Dump" level="trace" additivity="false"> >> + <AppenderRef ref="STDOUT" /> >> + </Logger> >> + <Root level="ERROR"> >> + <AppenderRef ref="myAppender" /> >> + </Root> >> + </Loggers> >> +</Configuration> >> \ No newline at end of file >> >> Modified: logging/log4j/log4j2/trunk/src/changes/changes.xml >> URL: >> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/src/changes/changes.xml?rev=1595741&r1=1595740&r2=1595741&view=diff >> ============================================================================== >> --- logging/log4j/log4j2/trunk/src/changes/changes.xml (original) >> +++ logging/log4j/log4j2/trunk/src/changes/changes.xml Mon May 19 04:53:11 >> 2014 >> @@ -22,6 +22,9 @@ >> </properties> >> <body> >> <release version="2.0-rc2" date="2014-MM-DD" description="Bug fixes and >> enhancements"> >> + <action issue="LOG4J2-620" dev="rgoers" type="fix"> >> + Perform reconfiguration in a separate thread to prevent deadlocks. >> + </action> >> <action issue="LOG4J2-641" dev="mattsicker" type="update"> >> Override commons-logging dependency version in tests. >> </action> >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
