Skip to content

Commit e87064f

Browse files
vyvy
authored
Reset the fallback listener on StatusLogger#reset() (#2280)
1 parent c6832d6 commit e87064f

File tree

4 files changed

+201
-28
lines changed

4 files changed

+201
-28
lines changed

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

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,18 @@
1616
*/
1717
package org.apache.logging.log4j.status;
1818

19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.spy;
22+
import static org.mockito.Mockito.verify;
23+
1924
import java.io.ByteArrayOutputStream;
2025
import java.io.PrintStream;
2126
import org.apache.logging.log4j.Level;
2227
import org.apache.logging.log4j.message.Message;
2328
import org.apache.logging.log4j.message.MessageFactory;
2429
import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
25-
import org.assertj.core.api.Assertions;
2630
import org.junit.jupiter.api.Test;
27-
import org.mockito.Mockito;
2831

2932
public class StatusConsoleListenerTest {
3033

@@ -34,16 +37,16 @@ public class StatusConsoleListenerTest {
3437
void StatusData_getFormattedStatus_should_be_used() {
3538

3639
// Create the listener.
37-
final PrintStream stream = Mockito.mock(PrintStream.class);
40+
final PrintStream stream = mock(PrintStream.class);
3841
final StatusConsoleListener listener = new StatusConsoleListener(Level.ALL, stream);
3942

4043
// Log a message.
41-
final Message message = Mockito.mock(Message.class);
42-
final StatusData statusData = Mockito.spy(new StatusData(null, Level.TRACE, message, null, null));
44+
final Message message = mock(Message.class);
45+
final StatusData statusData = spy(new StatusData(null, Level.TRACE, message, null, null));
4346
listener.log(statusData);
4447

4548
// Verify the call.
46-
Mockito.verify(statusData).getFormattedStatus();
49+
verify(statusData).getFormattedStatus();
4750
}
4851

4952
@Test
@@ -86,18 +89,50 @@ void level_and_stream_should_be_honored() throws Exception {
8689
final String output = outputStream.toString(encoding);
8790

8891
// Verify the output.
89-
Assertions.assertThat(output)
92+
assertThat(output)
9093
.contains(expectedThrowable.getMessage())
9194
.contains(expectedMessage.getFormattedMessage())
9295
.doesNotContain(discardedThrowable.getMessage())
9396
.doesNotContain(discardedMessage.getFormattedMessage());
9497
}
9598

9699
@Test
97-
void non_system_streams_should_be_closed() throws Exception {
98-
final PrintStream stream = Mockito.mock(PrintStream.class);
100+
void non_system_streams_should_be_closed() {
101+
final PrintStream stream = mock(PrintStream.class);
99102
final StatusConsoleListener listener = new StatusConsoleListener(Level.WARN, stream);
100103
listener.close();
101-
Mockito.verify(stream).close();
104+
verify(stream).close();
105+
}
106+
107+
@Test
108+
void close_should_reset_to_initials() {
109+
110+
// Create the listener
111+
final PrintStream initialStream = mock(PrintStream.class);
112+
final Level initialLevel = Level.TRACE;
113+
final StatusConsoleListener listener = new StatusConsoleListener(initialLevel, initialStream);
114+
115+
// Verify the initial state
116+
assertThat(listener.getStatusLevel()).isEqualTo(initialLevel);
117+
assertThat(listener).hasFieldOrPropertyWithValue("stream", initialStream);
118+
119+
// Update the state
120+
final PrintStream newStream = mock(PrintStream.class);
121+
listener.setStream(newStream);
122+
final Level newLevel = Level.DEBUG;
123+
listener.setLevel(newLevel);
124+
125+
// Verify the update
126+
verify(initialStream).close();
127+
assertThat(listener.getStatusLevel()).isEqualTo(newLevel);
128+
assertThat(listener).hasFieldOrPropertyWithValue("stream", newStream);
129+
130+
// Close the listener
131+
listener.close();
132+
133+
// Verify the reset
134+
verify(newStream).close();
135+
assertThat(listener.getStatusLevel()).isEqualTo(initialLevel);
136+
assertThat(listener).hasFieldOrPropertyWithValue("stream", initialStream);
102137
}
103138
}

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.status;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.mockito.Mockito.mock;
21+
import static org.mockito.Mockito.verify;
22+
import static org.mockito.Mockito.when;
23+
24+
import java.io.IOException;
25+
import java.io.PrintStream;
26+
import org.apache.logging.log4j.Level;
27+
import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
28+
import org.junit.jupiter.api.Test;
29+
30+
class StatusLoggerResetTest {
31+
32+
@Test
33+
void test_reset() throws IOException {
34+
35+
// Create the fallback listener
36+
final PrintStream fallbackListenerInitialStream = mock(PrintStream.class);
37+
final Level fallbackListenerInitialLevel = Level.INFO;
38+
final StatusConsoleListener fallbackListener =
39+
new StatusConsoleListener(fallbackListenerInitialLevel, fallbackListenerInitialStream);
40+
41+
// Create the `StatusLogger`
42+
final StatusLogger.Config loggerConfig = new StatusLogger.Config(false, 0, null);
43+
final StatusLogger logger = new StatusLogger(
44+
StatusLoggerResetTest.class.getSimpleName(),
45+
ParameterizedNoReferenceMessageFactory.INSTANCE,
46+
loggerConfig,
47+
fallbackListener);
48+
49+
// Create and register a listener
50+
final Level listenerLevel = Level.DEBUG;
51+
assertThat(listenerLevel.isLessSpecificThan(fallbackListenerInitialLevel))
52+
.isTrue();
53+
final StatusListener listener = mock(StatusListener.class);
54+
when(listener.getStatusLevel()).thenReturn(listenerLevel);
55+
logger.registerListener(listener);
56+
57+
// Verify the `StatusLogger` state
58+
assertThat(logger.getLevel()).isEqualTo(listenerLevel);
59+
assertThat(logger.getListeners()).containsExactly(listener);
60+
61+
// Update the fallback listener
62+
final PrintStream fallbackListenerNewStream = mock(PrintStream.class);
63+
fallbackListener.setStream(fallbackListenerNewStream);
64+
verify(fallbackListenerInitialStream).close();
65+
final Level fallbackListenerNewLevel = Level.TRACE;
66+
assertThat(fallbackListenerNewLevel.isLessSpecificThan(listenerLevel)).isTrue();
67+
fallbackListener.setLevel(fallbackListenerNewLevel);
68+
69+
// Verify the `StatusLogger` state
70+
assertThat(logger.getLevel()).isEqualTo(fallbackListenerNewLevel);
71+
72+
// Reset the `StatusLogger` and verify the state
73+
logger.reset();
74+
verify(listener).close();
75+
verify(fallbackListenerNewStream).close();
76+
assertThat(logger.getLevel()).isEqualTo(fallbackListenerInitialLevel);
77+
assertThat(logger.getListeners()).isEmpty();
78+
}
79+
}

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

Lines changed: 65 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,12 @@
1818

1919
import static java.util.Objects.requireNonNull;
2020

21+
import edu.umd.cs.findbugs.annotations.Nullable;
2122
import java.io.IOException;
2223
import java.io.OutputStream;
2324
import java.io.PrintStream;
25+
import java.util.concurrent.locks.Lock;
26+
import java.util.concurrent.locks.ReentrantLock;
2427
import org.apache.logging.log4j.Level;
2528

2629
/**
@@ -29,8 +32,16 @@
2932
@SuppressWarnings("UseOfSystemOutOrSystemErr")
3033
public class StatusConsoleListener implements StatusListener {
3134

35+
private final Lock lock = new ReentrantLock();
36+
37+
private final Level initialLevel;
38+
39+
private final PrintStream initialStream;
40+
41+
// `volatile` is necessary to correctly read the `level` without holding the lock
3242
private volatile Level level;
3343

44+
// `volatile` is necessary to correctly read the `stream` without holding the lock
3445
private volatile PrintStream stream;
3546

3647
/**
@@ -54,8 +65,8 @@ public StatusConsoleListener(final Level level) {
5465
* @throws NullPointerException on null {@code level} or {@code stream}
5566
*/
5667
public StatusConsoleListener(final Level level, final PrintStream stream) {
57-
this.level = requireNonNull(level, "level");
58-
this.stream = requireNonNull(stream, "stream");
68+
this.initialLevel = this.level = requireNonNull(level, "level");
69+
this.initialStream = this.stream = requireNonNull(stream, "stream");
5970
}
6071

6172
/**
@@ -65,7 +76,16 @@ public StatusConsoleListener(final Level level, final PrintStream stream) {
6576
* @throws NullPointerException on null {@code level}
6677
*/
6778
public void setLevel(final Level level) {
68-
this.level = requireNonNull(level, "level");
79+
requireNonNull(level, "level");
80+
// Check if a mutation (and locking!) is necessary at all
81+
if (!this.level.equals(level)) {
82+
lock.lock();
83+
try {
84+
this.level = level;
85+
} finally {
86+
lock.unlock();
87+
}
88+
}
6989
}
7090

7191
/**
@@ -76,7 +96,23 @@ public void setLevel(final Level level) {
7696
* @since 2.23.0
7797
*/
7898
public void setStream(final PrintStream stream) {
79-
this.stream = requireNonNull(stream, "stream");
99+
requireNonNull(stream, "stream");
100+
// Check if a mutation (and locking!) is necessary at all
101+
if (this.stream != stream) {
102+
@Nullable OutputStream oldStream = null;
103+
lock.lock();
104+
try {
105+
if (this.stream != stream) {
106+
oldStream = this.stream;
107+
this.stream = stream;
108+
}
109+
} finally {
110+
lock.unlock();
111+
}
112+
if (oldStream != null) {
113+
closeNonSystemStream(oldStream);
114+
}
115+
}
80116
}
81117

82118
/**
@@ -113,13 +149,33 @@ public void log(final StatusData data) {
113149
@Deprecated
114150
public void setFilters(final String... filters) {}
115151

152+
/**
153+
* Resets the level and output stream to its initial values, and closes the output stream, if it is a non-system one.
154+
*/
116155
@Override
117-
public void close() throws IOException {
118-
// Get local copy of the `volatile` member
119-
final OutputStream localStream = stream;
156+
public void close() {
157+
final OutputStream oldStream;
158+
lock.lock();
159+
try {
160+
oldStream = stream;
161+
stream = initialStream;
162+
level = initialLevel;
163+
} finally {
164+
lock.unlock();
165+
}
166+
closeNonSystemStream(oldStream);
167+
}
168+
169+
private static void closeNonSystemStream(final OutputStream stream) {
120170
// Close only non-system streams
121-
if (localStream != System.out && localStream != System.err) {
122-
localStream.close();
171+
if (stream != System.out && stream != System.err) {
172+
try {
173+
stream.close();
174+
} catch (IOException error) {
175+
// We are at the lowest level of the system.
176+
// Hence, there is nothing better we can do but dumping the failure.
177+
error.printStackTrace(System.err);
178+
}
123179
}
124180
}
125181
}

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ public static final class Config {
182182

183183
private final int bufferCapacity;
184184

185+
@Nullable
185186
private final Level fallbackListenerLevel;
186187

187188
@Nullable
@@ -193,22 +194,23 @@ public static final class Config {
193194
*
194195
* @param debugEnabled the value of the {@value DEBUG_PROPERTY_NAME} property
195196
* @param bufferCapacity the value of the {@value MAX_STATUS_ENTRIES} property
196-
* @param fallbackListenerLevel the value of the {@value DEFAULT_STATUS_LISTENER_LEVEL} property
197197
* @param instantFormatter the value of the {@value STATUS_DATE_FORMAT} property
198198
*/
199-
public Config(
200-
boolean debugEnabled,
201-
int bufferCapacity,
202-
Level fallbackListenerLevel,
203-
@Nullable DateTimeFormatter instantFormatter) {
199+
public Config(boolean debugEnabled, int bufferCapacity, @Nullable DateTimeFormatter instantFormatter) {
204200
this.debugEnabled = debugEnabled;
205201
if (bufferCapacity < 0) {
206202
throw new IllegalArgumentException(
207203
"was expecting a positive `bufferCapacity`, found: " + bufferCapacity);
208204
}
209205
this.bufferCapacity = bufferCapacity;
210-
this.fallbackListenerLevel = requireNonNull(fallbackListenerLevel, "fallbackListenerLevel");
211-
this.instantFormatter = requireNonNull(instantFormatter, "instantFormatter");
206+
// Public ctor intentionally doesn't set `fallbackListenerLevel`.
207+
// Because, if public ctor is used, it means user is programmatically creating a `Config` instance.
208+
// Hence, they will use the public `StatusLogger` ctor too.
209+
// There they need to provide the fallback listener explicitly anyway.
210+
// Therefore, there is no need to ask for a `fallbackListenerLevel` here.
211+
// Since this `fallbackListenerLevel` is only used by the private `StatusLogger` ctor.
212+
this.fallbackListenerLevel = null;
213+
this.instantFormatter = instantFormatter;
212214
}
213215

214216
/**
@@ -441,7 +443,7 @@ public Iterable<StatusListener> getListeners() {
441443
}
442444

443445
/**
444-
* Clears the event buffer and removes the <em>registered</em> (not the fallback one!) listeners.
446+
* Clears the event buffer, removes the <em>registered</em> (not the fallback one!) listeners, and resets the fallback listener.
445447
*/
446448
public void reset() {
447449
listenerWriteLock.lock();
@@ -455,6 +457,7 @@ public void reset() {
455457
} finally {
456458
listenerWriteLock.unlock();
457459
}
460+
fallbackListener.close();
458461
buffer.clear();
459462
}
460463

0 commit comments

Comments
 (0)