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 09:12:04 UTC

[logging-log4j2] branch LOG4J2-3185 created (now 0ef9637)

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

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


      at 0ef9637  LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler.

This branch includes the following new commits:

     new 0ef9637  LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler.

The 1 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.


[logging-log4j2] 01/01: LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler.

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

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

commit 0ef963714a85359c774038e3648913282f4d2d6d
Author: Volkan Yazici <vo...@yazi.ci>
AuthorDate: Mon Nov 8 10:10:06 2021 +0100

    LOG4J2-3060 LOG4J2-3185 Fix thread-safety issues in DefaultErrorHandler.
---
 .../log4j/core/appender/DefaultErrorHandler.java   | 78 +++++++++++++---------
 src/changes/changes.xml                            |  6 ++
 2 files changed, 52 insertions(+), 32 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..c4aa865 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 volatile int exceptionCount = 0;
 
-    private long lastException = System.nanoTime() - EXCEPTION_INTERVAL - 1;
+    private volatile 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>