Reset the fallback listener on StatusLogger#reset()

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java

@@ -16,15 +16,18 @@
  */
 package org.apache.logging.log4j.status;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.message.MessageFactory;
 import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
-import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
 
 public class StatusConsoleListenerTest {
 
@@ -34,16 +37,16 @@ public class StatusConsoleListenerTest {
     void StatusData_getFormattedStatus_should_be_used() {
 
         // Create the listener.
-        final PrintStream stream = Mockito.mock(PrintStream.class);
+        final PrintStream stream = mock(PrintStream.class);
         final StatusConsoleListener listener = new StatusConsoleListener(Level.ALL, stream);
 
         // Log a message.
-        final Message message = Mockito.mock(Message.class);
-        final StatusData statusData = Mockito.spy(new StatusData(null, Level.TRACE, message, null, null));
+        final Message message = mock(Message.class);
+        final StatusData statusData = spy(new StatusData(null, Level.TRACE, message, null, null));
         listener.log(statusData);
 
         // Verify the call.
-        Mockito.verify(statusData).getFormattedStatus();
+        verify(statusData).getFormattedStatus();
     }
 
     @Test
@@ -86,18 +89,50 @@ void level_and_stream_should_be_honored() throws Exception {
         final String output = outputStream.toString(encoding);
 
         // Verify the output.
-        Assertions.assertThat(output)
+        assertThat(output)
                 .contains(expectedThrowable.getMessage())
                 .contains(expectedMessage.getFormattedMessage())
                 .doesNotContain(discardedThrowable.getMessage())
                 .doesNotContain(discardedMessage.getFormattedMessage());
     }
 
     @Test
-    void non_system_streams_should_be_closed() throws Exception {
-        final PrintStream stream = Mockito.mock(PrintStream.class);
+    void non_system_streams_should_be_closed() {
+        final PrintStream stream = mock(PrintStream.class);
         final StatusConsoleListener listener = new StatusConsoleListener(Level.WARN, stream);
         listener.close();
-        Mockito.verify(stream).close();
+        verify(stream).close();
+    }
+
+    @Test
+    void close_should_reset_to_initials() {
+
+        // Create the listener
+        final PrintStream initialStream = mock(PrintStream.class);
+        final Level initialLevel = Level.TRACE;
+        final StatusConsoleListener listener = new StatusConsoleListener(initialLevel, initialStream);
+
+        // Verify the initial state
+        assertThat(listener.getStatusLevel()).isEqualTo(initialLevel);
+        assertThat(listener).hasFieldOrPropertyWithValue("stream", initialStream);
+
+        // Update the state
+        final PrintStream newStream = mock(PrintStream.class);
+        listener.setStream(newStream);
+        final Level newLevel = Level.DEBUG;
+        listener.setLevel(newLevel);
+
+        // Verify the update
+        verify(initialStream).close();
+        assertThat(listener.getStatusLevel()).isEqualTo(newLevel);
+        assertThat(listener).hasFieldOrPropertyWithValue("stream", newStream);
+
+        // Close the listener
+        listener.close();
+
+        // Verify the reset
+        verify(newStream).close();
+        assertThat(listener.getStatusLevel()).isEqualTo(initialLevel);
+        assertThat(listener).hasFieldOrPropertyWithValue("stream", initialStream);
     }
 }

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerResetTest.java

@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.status;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.io.PrintStream;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
+import org.junit.jupiter.api.Test;
+
+class StatusLoggerResetTest {
+
+    @Test
+    void test_reset() throws IOException {
+
+        // Create the fallback listener
+        final PrintStream fallbackListenerInitialStream = mock(PrintStream.class);
+        final Level fallbackListenerInitialLevel = Level.INFO;
+        final StatusConsoleListener fallbackListener =
+                new StatusConsoleListener(fallbackListenerInitialLevel, fallbackListenerInitialStream);
+
+        // Create the `StatusLogger`
+        final StatusLogger.Config loggerConfig = new StatusLogger.Config(false, 0, null);
+        final StatusLogger logger = new StatusLogger(
+                StatusLoggerResetTest.class.getSimpleName(),
+                ParameterizedNoReferenceMessageFactory.INSTANCE,
+                loggerConfig,
+                fallbackListener);
+
+        // Create and register a listener
+        final Level listenerLevel = Level.DEBUG;
+        assertThat(listenerLevel.isLessSpecificThan(fallbackListenerInitialLevel))
+                .isTrue();
+        final StatusListener listener = mock(StatusListener.class);
+        when(listener.getStatusLevel()).thenReturn(listenerLevel);
+        logger.registerListener(listener);
+
+        // Verify the `StatusLogger` state
+        assertThat(logger.getLevel()).isEqualTo(listenerLevel);
+        assertThat(logger.getListeners()).containsExactly(listener);
+
+        // Update the fallback listener
+        final PrintStream fallbackListenerNewStream = mock(PrintStream.class);
+        fallbackListener.setStream(fallbackListenerNewStream);
+        verify(fallbackListenerInitialStream).close();
+        final Level fallbackListenerNewLevel = Level.TRACE;
+        assertThat(fallbackListenerNewLevel.isLessSpecificThan(listenerLevel)).isTrue();
+        fallbackListener.setLevel(fallbackListenerNewLevel);
+
+        // Verify the `StatusLogger` state
+        assertThat(logger.getLevel()).isEqualTo(fallbackListenerNewLevel);
+
+        // Reset the `StatusLogger` and verify the state
+        logger.reset();
+        verify(listener).close();
+        verify(fallbackListenerNewStream).close();
+        assertThat(logger.getLevel()).isEqualTo(fallbackListenerInitialLevel);
+        assertThat(logger.getListeners()).isEmpty();
+    }
+}

log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java

@@ -18,9 +18,12 @@
 
 import static java.util.Objects.requireNonNull;
 
+import edu.umd.cs.findbugs.annotations.Nullable;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
 import org.apache.logging.log4j.Level;
 
 /**
@@ -29,8 +32,16 @@
 @SuppressWarnings("UseOfSystemOutOrSystemErr")
 public class StatusConsoleListener implements StatusListener {
 
+    private final Lock lock = new ReentrantLock();
+
+    private final Level initialLevel;
+
+    private final PrintStream initialStream;
+
+    // `volatile` is necessary to correctly read the `level` without holding the lock
     private volatile Level level;
 
+    // `volatile` is necessary to correctly read the `stream` without holding the lock
     private volatile PrintStream stream;
 
     /**
@@ -54,8 +65,8 @@ public StatusConsoleListener(final Level level) {
      * @throws NullPointerException on null {@code level} or {@code stream}
      */
     public StatusConsoleListener(final Level level, final PrintStream stream) {
-        this.level = requireNonNull(level, "level");
-        this.stream = requireNonNull(stream, "stream");
+        this.initialLevel = this.level = requireNonNull(level, "level");
+        this.initialStream = this.stream = requireNonNull(stream, "stream");
     }
 
     /**
@@ -65,7 +76,16 @@ public StatusConsoleListener(final Level level, final PrintStream stream) {
      * @throws NullPointerException on null {@code level}
      */
     public void setLevel(final Level level) {
-        this.level = requireNonNull(level, "level");
+        requireNonNull(level, "level");
+        // Check if a mutation (and locking!) is necessary at all
+        if (!this.level.equals(level)) {
+            lock.lock();
+            try {
+                this.level = level;
+            } finally {
+                lock.unlock();
+            }
+        }
     }
 
     /**
@@ -76,7 +96,23 @@ public void setLevel(final Level level) {
      * @since 2.23.0
      */
     public void setStream(final PrintStream stream) {
-        this.stream = requireNonNull(stream, "stream");
+        requireNonNull(stream, "stream");
+        // Check if a mutation (and locking!) is necessary at all
+        if (this.stream != stream) {
+            @Nullable OutputStream oldStream = null;
+            lock.lock();
+            try {
+                if (this.stream != stream) {
+                    oldStream = this.stream;
+                    this.stream = stream;
+                }
+            } finally {
+                lock.unlock();
+            }
+            if (oldStream != null) {
+                closeNonSystemStream(oldStream);
+            }
+        }
     }
 
     /**
@@ -113,13 +149,33 @@ public void log(final StatusData data) {
     @Deprecated
     public void setFilters(final String... filters) {}
 
+    /**
+     * Resets the level and output stream to its initial values, and closes the output stream, if it is a non-system one.
+     */
     @Override
-    public void close() throws IOException {
-        // Get local copy of the `volatile` member
-        final OutputStream localStream = stream;
+    public void close() {
+        final OutputStream oldStream;
+        lock.lock();
+        try {
+            oldStream = stream;
+            stream = initialStream;
+            level = initialLevel;
+        } finally {
+            lock.unlock();
+        }
+        closeNonSystemStream(oldStream);
+    }
+
+    private static void closeNonSystemStream(final OutputStream stream) {
         // Close only non-system streams
-        if (localStream != System.out && localStream != System.err) {
-            localStream.close();
+        if (stream != System.out && stream != System.err) {
+            try {
+                stream.close();
+            } catch (IOException error) {
+                // We are at the lowest level of the system.
+                // Hence, there is nothing better we can do but dumping the failure.
+                error.printStackTrace(System.err);
+            }
         }
     }
 }

log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java

@@ -182,6 +182,7 @@ public static final class Config {
 
         private final int bufferCapacity;
 
+        @Nullable
         private final Level fallbackListenerLevel;
 
         @Nullable
@@ -193,22 +194,23 @@ public static final class Config {
          *
          * @param debugEnabled the value of the {@value DEBUG_PROPERTY_NAME} property
          * @param bufferCapacity the value of the {@value MAX_STATUS_ENTRIES} property
-         * @param fallbackListenerLevel the value of the {@value DEFAULT_STATUS_LISTENER_LEVEL} property
          * @param instantFormatter the value of the {@value STATUS_DATE_FORMAT} property
          */
-        public Config(
-                boolean debugEnabled,
-                int bufferCapacity,
-                Level fallbackListenerLevel,
-                @Nullable DateTimeFormatter instantFormatter) {
+        public Config(boolean debugEnabled, int bufferCapacity, @Nullable DateTimeFormatter instantFormatter) {
             this.debugEnabled = debugEnabled;
             if (bufferCapacity < 0) {
                 throw new IllegalArgumentException(
                         "was expecting a positive `bufferCapacity`, found: " + bufferCapacity);
             }
             this.bufferCapacity = bufferCapacity;
-            this.fallbackListenerLevel = requireNonNull(fallbackListenerLevel, "fallbackListenerLevel");
-            this.instantFormatter = requireNonNull(instantFormatter, "instantFormatter");
+            // Public ctor intentionally doesn't set `fallbackListenerLevel`.
+            // Because, if public ctor is used, it means user is programmatically creating a `Config` instance.
+            // Hence, they will use the public `StatusLogger` ctor too.
+            // There they need to provide the fallback listener explicitly anyway.
+            // Therefore, there is no need to ask for a `fallbackListenerLevel` here.
+            // Since this `fallbackListenerLevel` is only used by the private `StatusLogger` ctor.
+            this.fallbackListenerLevel = null;
+            this.instantFormatter = instantFormatter;
         }
 
         /**
@@ -441,7 +443,7 @@ public Iterable<StatusListener> getListeners() {
     }
 
     /**
-     * Clears the event buffer and removes the <em>registered</em> (not the fallback one!) listeners.
+     * Clears the event buffer, removes the <em>registered</em> (not the fallback one!) listeners, and resets the fallback listener.
      */
     public void reset() {
         listenerWriteLock.lock();
@@ -455,6 +457,7 @@ public void reset() {
         } finally {
             listenerWriteLock.unlock();
         }
+        fallbackListener.close();
         buffer.clear();
     }