This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch release-2.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 86ccf41d6dc999c39ddeb5af1dc0968c167c4643 Author: Gary Gregory <[email protected]> AuthorDate: Wed Feb 24 16:11:28 2021 -0500 [LOG4J2-3026] WatchManager does not stop its ConfigurationScheduler thereby leaking a thread. --- .../java/org/apache/logging/log4j/core/util/WatchManager.java | 10 +++++++++- .../org/apache/logging/log4j/core/util/WatchHttpTest.java | 5 +++-- .../org/apache/logging/log4j/core/util/WatchManagerTest.java | 7 ++++--- src/changes/changes.xml | 11 +++++++---- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java index e760809..48de57d 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/util/WatchManager.java @@ -22,6 +22,7 @@ import java.util.Date; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.ServiceLoader; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; @@ -132,7 +133,7 @@ public class WatchManager extends AbstractLifeCycle { private final UUID id = LocalUUID.get(); public WatchManager(final ConfigurationScheduler scheduler) { - this.scheduler = scheduler; + this.scheduler = Objects.requireNonNull(scheduler, "scheduler"); eventServiceList = getEventServices(); } @@ -306,6 +307,9 @@ public class WatchManager extends AbstractLifeCycle { for (WatchEventService service : eventServiceList) { service.unsubscribe(this); } + if (scheduler.isStarted()) { + scheduler.stop(timeout, timeUnit); + } final boolean stopped = stop(future); setStopped(); return stopped; @@ -372,4 +376,8 @@ public class WatchManager extends AbstractLifeCycle { Source source = new Source(file); watch(source, watcher); } + + ConfigurationScheduler getScheduler() { + return scheduler; + } } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java index 01b837c..c16d525 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchHttpTest.java @@ -51,6 +51,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; /** * Test the WatchManager @@ -113,7 +114,7 @@ public class WatchHttpTest { } finally { removeStub(stubMapping); watchManager.stop(); - scheduler.stop(); + assertTrue(watchManager.getScheduler().isStopped()); } } @@ -149,7 +150,7 @@ public class WatchHttpTest { } finally { removeStub(stubMapping); watchManager.stop(); - scheduler.stop(); + assertTrue(watchManager.getScheduler().isStopped()); } } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java index fecb2e7..5902414 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/util/WatchManagerTest.java @@ -33,6 +33,7 @@ import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.condition.EnabledIfSystemProperty; import org.junit.jupiter.api.condition.OS; +import static org.junit.Assert.assertTrue; import static org.junit.jupiter.api.Assertions.*; /** @@ -72,7 +73,7 @@ public class WatchManagerTest { assertNotNull(f, "File change not detected"); } finally { watchManager.stop(); - scheduler.stop(); + assertTrue(watchManager.getScheduler().isStopped()); } } @@ -105,7 +106,7 @@ public class WatchManagerTest { assertNull(f, "File change detected"); } finally { watchManager.stop(); - scheduler.stop(); + assertTrue(watchManager.getScheduler().isStopped()); } } @@ -138,7 +139,7 @@ public class WatchManagerTest { assertNull(f, "File change detected"); } finally { watchManager.stop(); - scheduler.stop(); + assertTrue(watchManager.getScheduler().isStopped()); } } diff --git a/src/changes/changes.xml b/src/changes/changes.xml index dc548d2..8c6f21f 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -38,13 +38,13 @@ Directly create a thread instead of using the common ForkJoin pool when initializing ThreadContextDataInjector" </action> <action issue="LOG4J2-2624" dev="mattsicker" type="fix" due-to="Tim Perry"> - Allow auto-shutdown of log4j in log4j-web to be turned off and provide a + Allow auto-shutdown of log4j in log4j-web to be turned off and provide a ServletContextListener "Log4jShutdownOnContextDestroyedListener" to stop log4j. Register the listener at the top of web.xml to ensure the shutdown happens last. </action> <action issue="LOG4J2-1606" dev="mattsicker" type="fix" due-to="Tim Perry"> - Allow auto-shutdown of log4j in log4j-web to be turned off and provide a - ServletContextListener "Log4jShutdownOnContextDestroyedListener" to stop log4j. + Allow auto-shutdown of log4j in log4j-web to be turned off and provide a + ServletContextListener "Log4jShutdownOnContextDestroyedListener" to stop log4j. Register the listener at the top of web.xml to ensure the shutdown happens last. </action> <action issue="LOG4J2-2998" dev="vy" type="fix"> @@ -83,6 +83,9 @@ <action issue="LOG4J2-3014" dev="ggregory" type="fix" due-to="Lee Breisacher, Gary Gregory"> Log4j1ConfigurationConverter on Windows produces "
" at end of every line. </action> + <action issue="LOG4J2-3026" dev="ggregory" type="fix" due-to="Gary Gregory"> + WatchManager does not stop its ConfigurationScheduler thereby leaking a thread. + </action> <!-- ADDS --> <action issue="LOG4J2-2962" dev="vy" type="add"> Enrich "map" resolver by unifying its backend with "mdc" resolver. @@ -189,7 +192,7 @@ </action> <action dev="ggregory" type="update"> Update Woodstox 5.0.3 -> 6.2.3 to match Jackson 2.12.1. - </action> + </action> <action dev="ggregory" type="update"> Update org.apache.activemq:* 5.16.0 -> 5.16.1. </action>
