You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by gg...@apache.org on 2021/02/25 13:15:10 UTC

[logging-log4j2] branch master updated (7847b82 -> 8abfd92)

This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git.


    from 7847b82  [LOG4J2-3026] WatchManager does not stop its ConfigurationScheduler thereby leaking a thread.
     new 545816e  Revert "[LOG4J2-3026] WatchManager does not stop its ConfigurationScheduler thereby leaking a thread."
     new 8abfd92  Validate input in ctor.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../logging/log4j/core/util/WatchManager.java      | 21 ++++--------
 .../logging/log4j/core/util/WatchHttpTest.java     |  5 ++-
 .../logging/log4j/core/util/WatchManagerTest.java  | 40 +++++++++++-----------
 src/changes/changes.xml                            |  3 --
 4 files changed, 29 insertions(+), 40 deletions(-)


[logging-log4j2] 02/02: Validate input in ctor.

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 8abfd929a9c6f5ff66a734d7c37e4fd72a3baaa7
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Feb 25 08:15:05 2021 -0500

    Validate input in ctor.
---
 .../apache/logging/log4j/core/util/WatchManager.java    | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 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 f58f67b..41de183 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
@@ -16,19 +16,13 @@
  */
 package org.apache.logging.log4j.core.util;
 
-import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.core.AbstractLifeCycle;
-import org.apache.logging.log4j.core.config.ConfigurationFileWatcher;
-import org.apache.logging.log4j.core.config.ConfigurationScheduler;
-import org.apache.logging.log4j.status.StatusLogger;
-import org.apache.logging.log4j.util.LoaderUtil;
-
 import java.io.File;
 import java.util.ArrayList;
 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;
@@ -37,6 +31,13 @@ import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.core.AbstractLifeCycle;
+import org.apache.logging.log4j.core.config.ConfigurationFileWatcher;
+import org.apache.logging.log4j.core.config.ConfigurationScheduler;
+import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.LoaderUtil;
+
 /**
  * Manages {@link FileWatcher}s.
  *
@@ -132,7 +133,7 @@ public class WatchManager extends AbstractLifeCycle {
     private final ConcurrentMap<Source, ConfigurationMonitor> watchers = new ConcurrentHashMap<>();
 
     public WatchManager(final ConfigurationScheduler scheduler) {
-        this.scheduler = scheduler;
+        this.scheduler = Objects.requireNonNull(scheduler, "scheduler");
         eventServiceList = getEventServices();
     }
 


[logging-log4j2] 01/02: Revert "[LOG4J2-3026] WatchManager does not stop its ConfigurationScheduler thereby leaking a thread."

Posted by gg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 545816e61982ffe9531940567990545cfa40f7fd
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Thu Feb 25 08:13:57 2021 -0500

    Revert "[LOG4J2-3026] WatchManager does not stop its ConfigurationScheduler thereby leaking a thread."
    
    This reverts commit 7847b82a874f279cf710084b3e32e18679f2e4e3.
---
 .../logging/log4j/core/util/WatchManager.java      | 10 +-----
 .../logging/log4j/core/util/WatchHttpTest.java     |  5 ++-
 .../logging/log4j/core/util/WatchManagerTest.java  | 40 +++++++++++-----------
 src/changes/changes.xml                            |  3 --
 4 files changed, 23 insertions(+), 35 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 89d129b..f58f67b 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
@@ -29,7 +29,6 @@ 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;
@@ -133,7 +132,7 @@ public class WatchManager extends AbstractLifeCycle {
     private final ConcurrentMap<Source, ConfigurationMonitor> watchers = new ConcurrentHashMap<>();
 
     public WatchManager(final ConfigurationScheduler scheduler) {
-        this.scheduler = Objects.requireNonNull(scheduler, "scheduler");
+        this.scheduler = scheduler;
         eventServiceList = getEventServices();
     }
 
@@ -185,10 +184,6 @@ public class WatchManager extends AbstractLifeCycle {
         return this.intervalSeconds;
     }
 
-    ConfigurationScheduler getScheduler() {
-        return scheduler;
-    }
-
     /**
      * Returns a Map of the file watchers.
      *
@@ -311,9 +306,6 @@ 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;
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 42ce7b1..87b1aac 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
@@ -53,7 +53,6 @@ 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.jupiter.api.Assertions.assertTrue;
 
 /**
  * Test the WatchManager
@@ -116,7 +115,7 @@ public class WatchHttpTest {
         } finally {
             removeStub(stubMapping);
             watchManager.stop();
-            assertTrue(watchManager.getScheduler().isStopped());
+            scheduler.stop();
         }
     }
 
@@ -152,7 +151,7 @@ public class WatchHttpTest {
         } finally {
             removeStub(stubMapping);
             watchManager.stop();
-            assertTrue(watchManager.getScheduler().isStopped());
+            scheduler.stop();
         }
     }
 
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 c1bd674..fecb2e7 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
@@ -42,24 +42,9 @@ import static org.junit.jupiter.api.Assertions.*;
 @EnabledIfSystemProperty(named = "WatchManagerTest.forceRun", matches = "true")
 public class WatchManagerTest {
 
-    private static class TestWatcher implements FileWatcher {
-
-        private final Queue<File> queue;
-
-        public TestWatcher(final Queue<File> queue) {
-            this.queue = queue;
-        }
-
-        @Override
-        public void fileModified(final File file) {
-            System.out.println(file.toString() + " was modified");
-            queue.add(file);
-        }
-    }
-
-    private final String newFile = "target/test-classes/log4j-test1.yaml";
-    private final String originalFile = "target/test-classes/log4j-test1.xml";
     private final String testFile = "target/testWatchFile";
+    private final String originalFile = "target/test-classes/log4j-test1.xml";
+    private final String newFile = "target/test-classes/log4j-test1.yaml";
 
     @Test
     public void testWatchManager() throws Exception {
@@ -87,7 +72,7 @@ public class WatchManagerTest {
             assertNotNull(f, "File change not detected");
         } finally {
             watchManager.stop();
-            assertTrue(watchManager.getScheduler().isStopped());
+            scheduler.stop();
         }
     }
 
@@ -120,7 +105,7 @@ public class WatchManagerTest {
             assertNull(f, "File change detected");
         } finally {
             watchManager.stop();
-            assertTrue(watchManager.getScheduler().isStopped());
+            scheduler.stop();
         }
     }
 
@@ -153,7 +138,22 @@ public class WatchManagerTest {
             assertNull(f, "File change detected");
         } finally {
             watchManager.stop();
-            assertTrue(watchManager.getScheduler().isStopped());
+            scheduler.stop();
+        }
+    }
+
+    private static class TestWatcher implements FileWatcher {
+
+        private final Queue<File> queue;
+
+        public TestWatcher(final Queue<File> queue) {
+            this.queue = queue;
+        }
+
+        @Override
+        public void fileModified(final File file) {
+            System.out.println(file.toString() + " was modified");
+            queue.add(file);
         }
     }
 }
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index a6284d1..9d2f6f0 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -245,9 +245,6 @@
       <action issue="LOG4J2-2916" dev="vy" type="fix" due-to="wuqian0808">
         Avoid redundant Kafka producer instantiation causing thread leaks.
       </action>
-      <action issue="LOG4J2-3026" dev="ggregory" type="fix" due-to="Gary Gregory">
-        WatchManager does not stop its ConfigurationScheduler thereby leaking a thread.
-      </action>
     </release>
     <release version="2.14.0" date="2020-MM-DD" description="GA Release 2.14.0">
       <action issue="LOG4J2-2911" dev="rgoers" type="fix">