diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java index 0b93ae1fbcd..5fb84eb3173 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java +++ b/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,7 +89,7 @@ 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()) @@ -94,10 +97,42 @@ void level_and_stream_should_be_honored() throws Exception { } @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); } } diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerResetTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerResetTest.java new file mode 100644 index 00000000000..07fe4ca2955 --- /dev/null +++ b/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(); + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java index 3e30bf19942..99bcd8d1a74 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java +++ b/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); + } } } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java index af6d6687900..d6056932190 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java +++ b/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 getListeners() { } /** - * Clears the event buffer and removes the registered (not the fallback one!) listeners. + * Clears the event buffer, removes the registered (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(); }