[
https://issues.apache.org/jira/browse/LOG4J2-3026?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17290269#comment-17290269
]
Gary D. Gregory commented on LOG4J2-3026:
-----------------------------------------
Nope, that's not enough, it's the {{WatchManager}} that is deficient, I am
testing locally:
{noformat}
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.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 @@
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 @@
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 @@
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.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 @@
} finally {
removeStub(stubMapping);
watchManager.stop();
- scheduler.stop();
+ assertTrue(watchManager.getScheduler().isStopped());
}
}
@@ -149,7 +150,7 @@
} 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.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 @@
assertNotNull(f, "File change not detected");
} finally {
watchManager.stop();
- scheduler.stop();
+ assertTrue(watchManager.getScheduler().isStopped());
}
}
@@ -105,7 +106,7 @@
assertNull(f, "File change detected");
} finally {
watchManager.stop();
- scheduler.stop();
+ assertTrue(watchManager.getScheduler().isStopped());
}
}
@@ -138,7 +139,7 @@
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>
{noformat}
> WatchManager does not stop its ConfigurationScheduler thereby leaking a thread
> ------------------------------------------------------------------------------
>
> Key: LOG4J2-3026
> URL: https://issues.apache.org/jira/browse/LOG4J2-3026
> Project: Log4j 2
> Issue Type: Bug
> Components: Core
> Affects Versions: 2.14.0
> Reporter: Gary D. Gregory
> Assignee: Gary D. Gregory
> Priority: Major
> Fix For: 2.14.1
>
>
> The {{WatchManager}} does not stop its {{ConfigurationScheduler}} thereby
> leaking a thread as reported by Tomcat 9:
> {noformat}
> 24-Feb-2021 14:13:49.747 WARNING [http-nio-8080-exec-9]
> org.apache.catalina.loader.WebappClassLoaderBase.clearReferencesThreads The
> web application [discoveryrecorder] appears to have started a thread named
> [Log4j2-TF-8-Scheduled-2] but has failed to stop it. This is very likely to
> create a memory leak. Stack trace of thread:
> sun.misc.Unsafe.park(Native Method)
> java.util.concurrent.locks.LockSupport.parkNanos(LockSupport.java:215)
>
> java.util.concurrent.locks.AbstractQueuedSynchronizer$ConditionObject.awaitNanos(AbstractQueuedSynchronizer.java:2078)
>
> java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:1093)
>
> java.util.concurrent.ScheduledThreadPoolExecutor$DelayedWorkQueue.take(ScheduledThreadPoolExecutor.java:809)
> java.util.concurrent.ThreadPoolExecutor.getTask(ThreadPoolExecutor.java:1074)
>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1134)
>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
> java.lang.Thread.run(Thread.java:748)
> {noformat}
> I am experiencing this in 2.14.0 but it looks to have been a bug since day 1.
> Windows 10, Java 8.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)