Fix problems from apache#2112 reviews

Author: ppkarwasz

log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerConfigDisruptor.java

@@ -93,6 +93,9 @@ public String toString() {
 
     /**
      * EventHandler performs the work in a separate thread.
+     * <p>
+     *     <strong>Warning:</strong> this implementation only works with Disruptor 4.x.
+     * </p>
      */
     private static class Log4jEventWrapperHandler implements EventHandler<Log4jEventWrapper> {
         private static final int NOTIFY_PROGRESS_THRESHOLD = 50;
@@ -129,7 +132,10 @@ private void notifyIntermediateProgress(final long sequence) {
     }
 
     /**
-     * A version of Log4jEventWrapperHandler for LMAX Disruptor 3.x.
+     * EventHandler performs the work in a separate thread.
+     * <p>
+     *     <strong>Warning:</strong> this implementation only works with Disruptor 3.x.
+     * </p>
      */
     private static final class Log4jEventWrapperHandler3 extends Log4jEventWrapperHandler
             implements SequenceReportingEventHandler<Log4jEventWrapper> {}
@@ -166,11 +172,13 @@ private static final class Log4jEventWrapperHandler3 extends Log4jEventWrapperHa
             };
 
     private Log4jEventWrapperHandler createEventHandler() {
-        try {
-            return LoaderUtil.newInstanceOf(
-                    "org.apache.logging.log4j.core.async.AsyncLoggerConfigDisruptor$Log4jEventWrapperHandler3");
-        } catch (final ReflectiveOperationException | LinkageError e) {
-            LOGGER.debug("LMAX Disruptor 3.x is missing, trying version 4.x.", e);
+        if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 3) {
+            try {
+                return LoaderUtil.newInstanceOf(
+                        "org.apache.logging.log4j.core.async.AsyncLoggerConfigDisruptor$Log4jEventWrapperHandler3");
+            } catch (final ReflectiveOperationException | LinkageError e) {
+                LOGGER.warn("Failed to create event handler for LMAX Disruptor 3.x, trying version 4.x.", e);
+            }
         }
         return new Log4jEventWrapperHandler();
     }

log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLoggerDisruptor.java

@@ -16,6 +16,7 @@
  */
 package org.apache.logging.log4j.core.async;
 
+import com.lmax.disruptor.EventHandler;
 import com.lmax.disruptor.EventTranslatorVararg;
 import com.lmax.disruptor.ExceptionHandler;
 import com.lmax.disruptor.RingBuffer;
@@ -35,6 +36,7 @@
 import org.apache.logging.log4j.core.util.Log4jThreadFactory;
 import org.apache.logging.log4j.core.util.Throwables;
 import org.apache.logging.log4j.message.Message;
+import org.apache.logging.log4j.util.LoaderUtil;
 
 /**
  * Helper class for async loggers: AsyncLoggerDisruptor handles the mechanics of working with the LMAX Disruptor, and
@@ -46,6 +48,20 @@ class AsyncLoggerDisruptor extends AbstractLifeCycle {
     private static final int SLEEP_MILLIS_BETWEEN_DRAIN_ATTEMPTS = 50;
     private static final int MAX_DRAIN_ATTEMPTS_BEFORE_SHUTDOWN = 200;
 
+    /**
+     * Creates an appropriate event handler for the Disruptor library used.
+     */
+    private static EventHandler<RingBufferLogEvent> createEventHandler() {
+        if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 3) {
+            try {
+                return LoaderUtil.newInstanceOf("org.apache.logging.log4j.core.async.RingBufferLogEventHandler");
+            } catch (final ReflectiveOperationException | LinkageError e) {
+                LOGGER.warn("Failed to create event handler for LMAX Disruptor 3.x, trying version 4.x.", e);
+            }
+        }
+        return new RingBufferLogEventHandler4();
+    }
+
     private final Object queueFullEnqueueLock = new Object();
 
     private volatile Disruptor<RingBufferLogEvent> disruptor;
@@ -122,8 +138,8 @@ public Thread newThread(final Runnable r) {
         final ExceptionHandler<RingBufferLogEvent> errorHandler = DisruptorUtil.getAsyncLoggerExceptionHandler();
         disruptor.setDefaultExceptionHandler(errorHandler);
 
-        final RingBufferLogEventHandler4[] handlers = {RingBufferLogEventHandler4.create()};
-        disruptor.handleEventsWith(handlers);
+        final EventHandler<RingBufferLogEvent> handler = createEventHandler();
+        disruptor.handleEventsWith(handler);
 
         LOGGER.debug(
                 "[{}] Starting AsyncLogger disruptor for this context with ringbufferSize={}, waitStrategy={}, "

log4j-core/src/main/java/org/apache/logging/log4j/core/async/DefaultAsyncWaitStrategyFactory.java

@@ -74,17 +74,17 @@ static WaitStrategy createDefaultWaitStrategy(final String propertyName) {
         LOGGER.trace(
                 "DefaultAsyncWaitStrategyFactory creating TimeoutBlockingWaitStrategy(timeout={}, unit=MILLIS)",
                 timeoutMillis);
-        try {
-            // Check for the v 4.x version of the strategy, the version in 3.x is not garbage-free.
-            if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 4) {
+        // Check for the v 4.x version of the strategy, the version in 3.x is not garbage-free.
+        if (DisruptorUtil.DISRUPTOR_MAJOR_VERSION == 4) {
+            try {
                 return (WaitStrategy) Class.forName("com.lmax.disruptor.TimeoutBlockingWaitStrategy")
                         .getConstructor(long.class, TimeUnit.class)
                         .newInstance(timeoutMillis, TimeUnit.MILLISECONDS);
+            } catch (final ReflectiveOperationException | LinkageError e) {
+                LOGGER.debug(
+                        "DefaultAsyncWaitStrategyFactory failed to load 'com.lmax.disruptor.TimeoutBlockingWaitStrategy', using '{}' instead.",
+                        TimeoutBlockingWaitStrategy.class.getName());
             }
-        } catch (final ReflectiveOperationException | LinkageError e) {
-            LOGGER.debug(
-                    "DefaultAsyncWaitStrategyFactory failed to load 'com.lmax.disruptor.TimeoutBlockingWaitStrategy', using '{}' instead.",
-                    TimeoutBlockingWaitStrategy.class.getName());
         }
         // Use our version
         return new TimeoutBlockingWaitStrategy(timeoutMillis, TimeUnit.MILLISECONDS);

log4j-core/src/main/java/org/apache/logging/log4j/core/async/RingBufferLogEventHandler.java

@@ -24,13 +24,11 @@
  * available. Processing of these messages is done in a separate thread,
  * controlled by the {@code Executor} passed to the {@code Disruptor}
  * constructor.
+ * <p>
+ *     <strong>Warning:</strong> this class only works with Disruptor 3.x.
+ * </p>
+ * @deprecated Only used internally, will be removed in the next major version.
  */
+@Deprecated
 public class RingBufferLogEventHandler extends RingBufferLogEventHandler4
-        implements SequenceReportingEventHandler<RingBufferLogEvent>, LifecycleAware {
-
-    /**
-     * @deprecated Use the {@link RingBufferLogEventHandler4#create()} factory method instead.
-     */
-    @Deprecated
-    public RingBufferLogEventHandler() {}
-}
+        implements SequenceReportingEventHandler<RingBufferLogEvent>, LifecycleAware {}

log4j-core/src/main/java/org/apache/logging/log4j/core/async/RingBufferLogEventHandler4.java

@@ -18,14 +18,15 @@
 
 import com.lmax.disruptor.EventHandler;
 import com.lmax.disruptor.Sequence;
-import org.apache.logging.log4j.status.StatusLogger;
-import org.apache.logging.log4j.util.LoaderUtil;
 
 /**
  * This event handler gets passed messages from the RingBuffer as they become
  * available. Processing of these messages is done in a separate thread,
  * controlled by the {@code Executor} passed to the {@code Disruptor}
  * constructor.
+ *  * <p>
+ *  *     <strong>Warning:</strong> this class only works with Disruptor 4.x.
+ *  * </p>
  */
 class RingBufferLogEventHandler4 implements EventHandler<RingBufferLogEvent> {
 
@@ -34,18 +35,6 @@ class RingBufferLogEventHandler4 implements EventHandler<RingBufferLogEvent> {
     private int counter;
     private long threadId = -1;
 
-    /**
-     * Returns the appropriate {@link EventHandler} for the version of LMAX Disruptor used.
-     */
-    public static RingBufferLogEventHandler4 create() {
-        try {
-            return LoaderUtil.newInstanceOf("org.apache.logging.log4j.core.async.RingBufferLogEventHandler");
-        } catch (final ReflectiveOperationException | LinkageError e) {
-            StatusLogger.getLogger().debug("LMAX Disruptor 3.x is missing, trying version 4.x.", e);
-        }
-        return new RingBufferLogEventHandler4();
-    }
-
     /*
      * Overrides a method from Disruptor 4.x. Do not remove.
      */