You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by vy...@apache.org on 2021/11/08 18:40:13 UTC

[logging-log4j2] branch release-2.x updated: LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler. (#597)

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

vy pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git


The following commit(s) were added to refs/heads/release-2.x by this push:
     new b6d2e34  LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler. (#597)
b6d2e34 is described below

commit b6d2e348d8c142754edb0ee13b42521d28fe3db9
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Mon Nov 8 19:40:09 2021 +0100

    LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler. (#597)
---
 .../log4j/core/appender/DefaultErrorHandler.java   | 76 +++++++++++++---------
 src/changes/changes.xml                            |  6 ++
 2 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/DefaultErrorHandler.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/DefaultErrorHandler.java
index fa51b95..6c4c06e 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/DefaultErrorHandler.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/DefaultErrorHandler.java
@@ -24,77 +24,91 @@ import org.apache.logging.log4j.core.ErrorHandler;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.status.StatusLogger;
 
+import static java.util.Objects.requireNonNull;
+
 /**
- *
+ * The default {@link ErrorHandler} implementation falling back to {@link StatusLogger}.
+ * <p>
+ * It avoids flooding the {@link StatusLogger} by allowing either the first 3 errors or errors once every 5 minutes.
+ * </p>
  */
 public class DefaultErrorHandler implements ErrorHandler {
 
     private static final Logger LOGGER = StatusLogger.getLogger();
 
-    private static final int MAX_EXCEPTIONS = 3;
+    private static final int MAX_EXCEPTION_COUNT = 3;
 
-    private static final long EXCEPTION_INTERVAL = TimeUnit.MINUTES.toNanos(5);
+    private static final long EXCEPTION_INTERVAL_NANOS = TimeUnit.MINUTES.toNanos(5);
 
     private int exceptionCount = 0;
 
-    private long lastException = System.nanoTime() - EXCEPTION_INTERVAL - 1;
+    private long lastExceptionInstantNanos = System.nanoTime() - EXCEPTION_INTERVAL_NANOS - 1;
 
     private final Appender appender;
 
     public DefaultErrorHandler(final Appender appender) {
-        this.appender = appender;
+        this.appender = requireNonNull(appender, "appender");
     }
 
-
     /**
      * Handle an error with a message.
-     * @param msg The message.
+     * @param msg a message
      */
     @Override
     public void error(final String msg) {
-        final long current = System.nanoTime();
-        if (current - lastException > EXCEPTION_INTERVAL || exceptionCount++ < MAX_EXCEPTIONS) {
+        final boolean allowed = acquirePermit();
+        if (allowed) {
             LOGGER.error(msg);
         }
-        lastException = current;
     }
 
     /**
      * Handle an error with a message and an exception.
-     * @param msg The message.
-     * @param t The Throwable.
+     *
+     * @param msg a message
+     * @param error a {@link Throwable}
      */
     @Override
-    public void error(final String msg, final Throwable t) {
-        final long current = System.nanoTime();
-        if (current - lastException > EXCEPTION_INTERVAL || exceptionCount++ < MAX_EXCEPTIONS) {
-            LOGGER.error(msg, t);
-        }
-        lastException = current;
-        if (!appender.ignoreExceptions() && t != null && !(t instanceof AppenderLoggingException)) {
-            throw new AppenderLoggingException(msg, t);
+    public void error(final String msg, final Throwable error) {
+        final boolean allowed = acquirePermit();
+        if (allowed && !appender.ignoreExceptions() && error != null && !(error instanceof AppenderLoggingException)) {
+            throw new AppenderLoggingException(msg, error);
         }
     }
 
     /**
-     * Handle an error with a message, and exception and a logging event.
-     * @param msg The message.
-     * @param event The LogEvent.
-     * @param t The Throwable.
+     * Handle an error with a message, an exception, and a logging event.
+     *
+     * @param msg a message
+     * @param event a {@link LogEvent}
+     * @param error a {@link Throwable}
      */
     @Override
-    public void error(final String msg, final LogEvent event, final Throwable t) {
-        final long current = System.nanoTime();
-        if (current - lastException > EXCEPTION_INTERVAL || exceptionCount++ < MAX_EXCEPTIONS) {
-            LOGGER.error(msg, t);
+    public void error(final String msg, final LogEvent event, final Throwable error) {
+        final boolean allowed = acquirePermit();
+        if (allowed && !appender.ignoreExceptions() && error != null && !(error instanceof AppenderLoggingException)) {
+            throw new AppenderLoggingException(msg, error);
         }
-        lastException = current;
-        if (!appender.ignoreExceptions() && t != null && !(t instanceof AppenderLoggingException)) {
-            throw new AppenderLoggingException(msg, t);
+    }
+
+    private boolean acquirePermit() {
+        final long currentInstantNanos = System.nanoTime();
+        synchronized (this) {
+            if (currentInstantNanos - lastExceptionInstantNanos > EXCEPTION_INTERVAL_NANOS) {
+                lastExceptionInstantNanos = currentInstantNanos;
+                return true;
+            } else if (exceptionCount < MAX_EXCEPTION_COUNT) {
+                exceptionCount++;
+                lastExceptionInstantNanos = currentInstantNanos;
+                return true;
+            } else {
+                return false;
+            }
         }
     }
 
     public Appender getAppender() {
         return appender;
     }
+
 }
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index cc65691..470bd06 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -101,6 +101,12 @@
         Improve PatternLayout performance by reducing unnecessary indirection and branching.
       </action>
       <!-- FIXES -->
+      <action issue="LOG4J2-3060" dev="vy" type="fix" due-to=" Nikita Mikhailov">
+        Fix thread-safety issues in DefaultErrorHandler.
+      </action>
+      <action issue="LOG4J2-3185" dev="vy" type="fix" due-to="mzbonnt">
+        Fix thread-safety issues in DefaultErrorHandler.
+      </action>
       <action issue="LOG4J2-3183" dev="vy" type="fix">
         Avoid using MutableInstant of the event as a cache key in JsonTemplateLayout.
       </action>