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 2024/02/10 20:52:06 UTC
(logging-log4j2) branch 2.x-StatusLogger-reset-fix created (now a226860f2e)
This is an automated email from the ASF dual-hosted git repository.
vy pushed a change to branch 2.x-StatusLogger-reset-fix
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
at a226860f2e Reset the fallback listener on `StatusLogger#reset()`
This branch includes the following new commits:
new a226860f2e Reset the fallback listener on `StatusLogger#reset()`
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: Reset the fallback listener on `StatusLogger#reset()`
Posted by vy...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch 2.x-StatusLogger-reset-fix
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit a226860f2e5315f635bb9fb1910d5f1c6acbafdd
Author: Volkan Yazıcı <vo...@yazi.ci>
AuthorDate: Sat Feb 10 21:51:55 2024 +0100
Reset the fallback listener on `StatusLogger#reset()`
---
.../log4j/status/StatusConsoleListenerTest.java | 55 ++++++++++++---
.../log4j/status/StatusLoggerResetTest.java | 79 ++++++++++++++++++++++
.../log4j/status/StatusConsoleListener.java | 74 +++++++++++++++++---
.../apache/logging/log4j/status/StatusLogger.java | 16 +++--
4 files changed, 200 insertions(+), 24 deletions(-)
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 0b93ae1fbc..5fb84eb317 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 @@ public class StatusConsoleListenerTest {
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 @@ public class StatusConsoleListenerTest {
}
@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 0000000000..47c7f139d0
--- /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 org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.io.PrintStream;
+
+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;
+
+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 3e30bf1994..99bcd8d1a7 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 @@ package org.apache.logging.log4j.status;
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 @@ import org.apache.logging.log4j.Level;
@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 class StatusConsoleListener implements StatusListener {
* @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 class StatusConsoleListener implements StatusListener {
* @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 class StatusConsoleListener implements StatusListener {
* @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 class StatusConsoleListener implements StatusListener {
@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 af6d668790..09a0ef9f2a 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 class StatusLogger extends AbstractLogger {
private final int bufferCapacity;
+ @Nullable
private final Level fallbackListenerLevel;
@Nullable
@@ -193,13 +194,11 @@ public class StatusLogger extends AbstractLogger {
*
* @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) {
this.debugEnabled = debugEnabled;
if (bufferCapacity < 0) {
@@ -207,8 +206,14 @@ public class StatusLogger extends AbstractLogger {
"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 +446,7 @@ public class StatusLogger extends AbstractLogger {
}
/**
- * 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 +460,7 @@ public class StatusLogger extends AbstractLogger {
} finally {
listenerWriteLock.unlock();
}
+ fallbackListener.close();
buffer.clear();
}