Skip to content

Commit 3e334dd

Browse files
ppkarwaszppkarwasz
committed
Allow multiple Log4jServletContextListener registrations
This closes #1782.
1 parent 0a5a4cd commit 3e334dd

File tree

5 files changed

+246
-100
lines changed

5 files changed

+246
-100
lines changed

‎log4j-jakarta-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
*/
3737
public class Log4jServletContextListener implements ServletContextListener {
3838

39+
static final String START_COUNT_ATTR = Log4jServletContextListener.class.getName() + ".START_COUNT";
40+
3941
private static final int DEFAULT_STOP_TIMEOUT = 30;
4042
private static final TimeUnit DEFAULT_STOP_TIMEOUT_TIMEUNIT = TimeUnit.SECONDS;
4143

@@ -47,11 +49,30 @@ public class Log4jServletContextListener implements ServletContextListener {
4749
private ServletContext servletContext;
4850
private Log4jWebLifeCycle initializer;
4951

52+
private int getAndIncrementCount() {
53+
Integer count = (Integer) servletContext.getAttribute(START_COUNT_ATTR);
54+
if (count == null) {
55+
count = 0;
56+
}
57+
servletContext.setAttribute(START_COUNT_ATTR, count + 1);
58+
return count;
59+
}
60+
61+
private int decrementAndGetCount() {
62+
Integer count = (Integer) servletContext.getAttribute(START_COUNT_ATTR);
63+
if (count == null) {
64+
LOGGER.warn(
65+
"{} received a 'contextDestroyed' message without a corresponding 'contextInitialized' message.",
66+
getClass().getName());
67+
count = 1;
68+
}
69+
servletContext.setAttribute(START_COUNT_ATTR, --count);
70+
return count;
71+
}
72+
5073
@Override
5174
public void contextInitialized(final ServletContextEvent event) {
5275
this.servletContext = event.getServletContext();
53-
LOGGER.debug("Log4jServletContextListener ensuring that Log4j starts up properly.");
54-
5576
if ("true".equalsIgnoreCase(servletContext.getInitParameter(
5677
Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED))) {
5778
throw new IllegalStateException("Do not use " + getClass().getSimpleName() + " when "
@@ -61,6 +82,12 @@ public void contextInitialized(final ServletContextEvent event) {
6182
}
6283

6384
this.initializer = WebLoggerContextUtils.getWebLifeCycle(this.servletContext);
85+
if (getAndIncrementCount() != 0) {
86+
LOGGER.debug("Skipping Log4j context initialization, since {} is registered multiple times.",
87+
getClass().getSimpleName());
88+
return;
89+
}
90+
LOGGER.info("{} triggered a Log4j context initialization.", getClass().getSimpleName());
6491
try {
6592
this.initializer.start();
6693
this.initializer.setLoggerContext(); // the application is just now starting to start up
@@ -72,23 +99,32 @@ public void contextInitialized(final ServletContextEvent event) {
7299
@Override
73100
public void contextDestroyed(final ServletContextEvent event) {
74101
if (this.servletContext == null || this.initializer == null) {
75-
LOGGER.warn("Context destroyed before it was initialized.");
102+
LOGGER.warn("Servlet context destroyed before it was initialized.");
76103
return;
77104
}
78-
LOGGER.debug("Log4jServletContextListener ensuring that Log4j shuts down properly.");
79-
80-
this.initializer.clearLoggerContext(); // the application is finished
81-
// shutting down now
82-
if (initializer instanceof LifeCycle2) {
83-
final String stopTimeoutStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT);
84-
final long stopTimeout = Strings.isEmpty(stopTimeoutStr) ? DEFAULT_STOP_TIMEOUT
85-
: Long.parseLong(stopTimeoutStr);
86-
final String timeoutTimeUnitStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT_TIMEUNIT);
87-
final TimeUnit timeoutTimeUnit = Strings.isEmpty(timeoutTimeUnitStr) ? DEFAULT_STOP_TIMEOUT_TIMEUNIT
88-
: TimeUnit.valueOf(toRootUpperCase(timeoutTimeUnitStr));
89-
((LifeCycle2) this.initializer).stop(stopTimeout, timeoutTimeUnit);
90-
} else {
91-
this.initializer.stop();
105+
106+
if (decrementAndGetCount() != 0) {
107+
LOGGER.debug("Skipping Log4j context shutdown, since {} is registered multiple times.",
108+
getClass().getSimpleName());
109+
return;
110+
}
111+
LOGGER.info("{} triggered a Log4j context shutdown.", getClass().getSimpleName());
112+
try {
113+
this.initializer.clearLoggerContext(); // the application is finished
114+
// shutting down now
115+
if (initializer instanceof LifeCycle2) {
116+
final String stopTimeoutStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT);
117+
final long stopTimeout = Strings.isEmpty(stopTimeoutStr) ? DEFAULT_STOP_TIMEOUT
118+
: Long.parseLong(stopTimeoutStr);
119+
final String timeoutTimeUnitStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT_TIMEUNIT);
120+
final TimeUnit timeoutTimeUnit = Strings.isEmpty(timeoutTimeUnitStr) ? DEFAULT_STOP_TIMEOUT_TIMEUNIT
121+
: TimeUnit.valueOf(toRootUpperCase(timeoutTimeUnitStr));
122+
((LifeCycle2) this.initializer).stop(stopTimeout, timeoutTimeUnit);
123+
} else {
124+
this.initializer.stop();
125+
}
126+
} catch (final IllegalStateException e) {
127+
throw new IllegalStateException("Failed to shutdown Log4j properly.", e);
92128
}
93129
}
94130
}

‎log4j-jakarta-web/src/test/java/org/apache/logging/log4j/web/Log4jServletContextListenerTest.java

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,66 +16,94 @@
1616
*/
1717
package org.apache.logging.log4j.web;
1818

19+
import java.util.concurrent.atomic.AtomicReference;
20+
1921
import jakarta.servlet.ServletContext;
2022
import jakarta.servlet.ServletContextEvent;
2123

2224
import org.apache.logging.log4j.util.Strings;
2325
import org.junit.jupiter.api.BeforeEach;
2426
import org.junit.jupiter.api.Test;
2527
import org.junit.jupiter.api.extension.ExtendWith;
28+
import org.junit.jupiter.params.ParameterizedTest;
29+
import org.junit.jupiter.params.provider.ValueSource;
2630
import org.mockito.Mock;
31+
import org.mockito.Mock.Strictness;
2732
import org.mockito.junit.jupiter.MockitoExtension;
2833

29-
import static org.junit.jupiter.api.Assertions.*;
30-
import static org.mockito.BDDMockito.eq;
34+
import static org.junit.jupiter.api.Assertions.assertThrows;
35+
import static org.mockito.AdditionalAnswers.answerVoid;
36+
import static org.mockito.ArgumentMatchers.any;
37+
import static org.mockito.ArgumentMatchers.eq;
3138
import static org.mockito.BDDMockito.given;
3239
import static org.mockito.BDDMockito.then;
3340
import static org.mockito.BDDMockito.willThrow;
41+
import static org.mockito.Mockito.doAnswer;
3442

3543
@ExtendWith(MockitoExtension.class)
3644
public class Log4jServletContextListenerTest {
3745
/* event and servletContext are marked lenient because they aren't used in the
3846
* testDestroyWithNoInit but are only accessed during initialization
3947
*/
40-
@Mock(lenient = true)
48+
@Mock(strictness = Strictness.LENIENT)
4149
private ServletContextEvent event;
42-
@Mock(lenient = true)
50+
@Mock(strictness = Strictness.LENIENT)
4351
private ServletContext servletContext;
4452
@Mock
4553
private Log4jWebLifeCycle initializer;
4654

47-
private Log4jServletContextListener listener;
55+
private final AtomicReference<Object> count = new AtomicReference<>();
4856

4957
@BeforeEach
5058
public void setUp() {
51-
this.listener = new Log4jServletContextListener();
5259
given(event.getServletContext()).willReturn(servletContext);
5360
given(servletContext.getAttribute(Log4jWebSupport.SUPPORT_ATTRIBUTE)).willReturn(initializer);
54-
}
5561

56-
@Test
57-
public void testInitAndDestroy() throws Exception {
58-
this.listener.contextInitialized(this.event);
62+
doAnswer(answerVoid((k, v) -> count.set(v)))
63+
.when(servletContext)
64+
.setAttribute(eq(Log4jServletContextListener.START_COUNT_ATTR), any());
65+
doAnswer(__ -> count.get())
66+
.when(servletContext)
67+
.getAttribute(Log4jServletContextListener.START_COUNT_ATTR);
68+
}
5969

60-
then(initializer).should().start();
61-
then(initializer).should().setLoggerContext();
70+
@ParameterizedTest
71+
@ValueSource(ints = { 1, 2, 3 })
72+
public void testInitAndDestroy(final int listenerCount) throws Exception {
73+
final Log4jServletContextListener[] listeners = new Log4jServletContextListener[listenerCount];
74+
for (int idx = 0; idx < listenerCount; idx++) {
75+
final Log4jServletContextListener listener = new Log4jServletContextListener();
76+
listeners[idx] = listener;
77+
78+
listener.contextInitialized(event);
79+
if (idx == 0) {
80+
then(initializer).should().start();
81+
then(initializer).should().setLoggerContext();
82+
} else {
83+
then(initializer).shouldHaveNoMoreInteractions();
84+
}
85+
}
6286

63-
this.listener.contextDestroyed(this.event);
87+
for (int idx = listenerCount - 1; idx >= 0; idx--) {
88+
final Log4jServletContextListener listener = listeners[idx];
6489

65-
then(initializer).should().clearLoggerContext();
66-
then(initializer).should().stop();
90+
listener.contextDestroyed(event);
91+
if (idx == 0) {
92+
then(initializer).should().clearLoggerContext();
93+
then(initializer).should().stop();
94+
} else {
95+
then(initializer).shouldHaveNoMoreInteractions();
96+
}
97+
}
6798
}
6899

69100
@Test
70101
public void testInitFailure() throws Exception {
71102
willThrow(new IllegalStateException(Strings.EMPTY)).given(initializer).start();
103+
final Log4jServletContextListener listener = new Log4jServletContextListener();
72104

73-
try {
74-
this.listener.contextInitialized(this.event);
75-
fail("Expected a RuntimeException.");
76-
} catch (final RuntimeException e) {
77-
assertEquals("Failed to initialize Log4j properly.", e.getMessage(), "The message is not correct.");
78-
}
105+
assertThrows(RuntimeException.class, () -> listener.contextInitialized(this.event),
106+
"Failed to initialize Log4j properly.");
79107
}
80108

81109
@Test
@@ -93,17 +121,12 @@ public void initializingLog4jServletContextListenerShouldFaileWhenAutoShutdownIs
93121
}
94122

95123
private void ensureInitializingFailsWhenAuthShutdownIsEnabled() {
96-
try {
97-
this.listener.contextInitialized(this.event);
98-
fail("Expected a RuntimeException.");
99-
} catch (final RuntimeException e) {
100-
final String expectedMessage =
101-
"Do not use " + Log4jServletContextListener.class.getSimpleName() + " when "
102-
+ Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED + " is true. Please use "
103-
+ Log4jShutdownOnContextDestroyedListener.class.getSimpleName() + " instead of "
104-
+ Log4jServletContextListener.class.getSimpleName() + ".";
105-
106-
assertEquals(expectedMessage, e.getMessage(), "The message is not correct");
107-
}
124+
final Log4jServletContextListener listener = new Log4jServletContextListener();
125+
final String message = "Do not use " + Log4jServletContextListener.class.getSimpleName() + " when "
126+
+ Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED + " is true. Please use "
127+
+ Log4jShutdownOnContextDestroyedListener.class.getSimpleName() + " instead of "
128+
+ Log4jServletContextListener.class.getSimpleName() + ".";
129+
130+
assertThrows(RuntimeException.class, () -> listener.contextInitialized(event), message);
108131
}
109132
}

‎log4j-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
*/
3737
public class Log4jServletContextListener implements ServletContextListener {
3838

39+
static final String START_COUNT_ATTR = Log4jServletContextListener.class.getName() + ".START_COUNT";
40+
3941
private static final int DEFAULT_STOP_TIMEOUT = 30;
4042
private static final TimeUnit DEFAULT_STOP_TIMEOUT_TIMEUNIT = TimeUnit.SECONDS;
4143

@@ -47,11 +49,30 @@ public class Log4jServletContextListener implements ServletContextListener {
4749
private ServletContext servletContext;
4850
private Log4jWebLifeCycle initializer;
4951

52+
private int getAndIncrementCount() {
53+
Integer count = (Integer) servletContext.getAttribute(START_COUNT_ATTR);
54+
if (count == null) {
55+
count = 0;
56+
}
57+
servletContext.setAttribute(START_COUNT_ATTR, count + 1);
58+
return count;
59+
}
60+
61+
private int decrementAndGetCount() {
62+
Integer count = (Integer) servletContext.getAttribute(START_COUNT_ATTR);
63+
if (count == null) {
64+
LOGGER.warn(
65+
"{} received a 'contextDestroyed' message without a corresponding 'contextInitialized' message.",
66+
getClass().getName());
67+
count = 1;
68+
}
69+
servletContext.setAttribute(START_COUNT_ATTR, --count);
70+
return count;
71+
}
72+
5073
@Override
5174
public void contextInitialized(final ServletContextEvent event) {
5275
this.servletContext = event.getServletContext();
53-
LOGGER.debug("Log4jServletContextListener ensuring that Log4j starts up properly.");
54-
5576
if ("true".equalsIgnoreCase(servletContext.getInitParameter(
5677
Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED))) {
5778
throw new IllegalStateException("Do not use " + getClass().getSimpleName() + " when "
@@ -61,6 +82,12 @@ public void contextInitialized(final ServletContextEvent event) {
6182
}
6283

6384
this.initializer = WebLoggerContextUtils.getWebLifeCycle(this.servletContext);
85+
if (getAndIncrementCount() != 0) {
86+
LOGGER.debug("Skipping Log4j context initialization, since {} is registered multiple times.",
87+
getClass().getSimpleName());
88+
return;
89+
}
90+
LOGGER.info("{} triggered a Log4j context initialization.", getClass().getSimpleName());
6491
try {
6592
this.initializer.start();
6693
this.initializer.setLoggerContext(); // the application is just now starting to start up
@@ -72,23 +99,32 @@ public void contextInitialized(final ServletContextEvent event) {
7299
@Override
73100
public void contextDestroyed(final ServletContextEvent event) {
74101
if (this.servletContext == null || this.initializer == null) {
75-
LOGGER.warn("Context destroyed before it was initialized.");
102+
LOGGER.warn("Servlet context destroyed before it was initialized.");
76103
return;
77104
}
78-
LOGGER.debug("Log4jServletContextListener ensuring that Log4j shuts down properly.");
79-
80-
this.initializer.clearLoggerContext(); // the application is finished
81-
// shutting down now
82-
if (initializer instanceof LifeCycle2) {
83-
final String stopTimeoutStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT);
84-
final long stopTimeout = Strings.isEmpty(stopTimeoutStr) ? DEFAULT_STOP_TIMEOUT
85-
: Long.parseLong(stopTimeoutStr);
86-
final String timeoutTimeUnitStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT_TIMEUNIT);
87-
final TimeUnit timeoutTimeUnit = Strings.isEmpty(timeoutTimeUnitStr) ? DEFAULT_STOP_TIMEOUT_TIMEUNIT
88-
: TimeUnit.valueOf(toRootUpperCase(timeoutTimeUnitStr));
89-
((LifeCycle2) this.initializer).stop(stopTimeout, timeoutTimeUnit);
90-
} else {
91-
this.initializer.stop();
105+
106+
if (decrementAndGetCount() != 0) {
107+
LOGGER.debug("Skipping Log4j context shutdown, since {} is registered multiple times.",
108+
getClass().getSimpleName());
109+
return;
110+
}
111+
LOGGER.info("{} triggered a Log4j context shutdown.", getClass().getSimpleName());
112+
try {
113+
this.initializer.clearLoggerContext(); // the application is finished
114+
// shutting down now
115+
if (initializer instanceof LifeCycle2) {
116+
final String stopTimeoutStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT);
117+
final long stopTimeout = Strings.isEmpty(stopTimeoutStr) ? DEFAULT_STOP_TIMEOUT
118+
: Long.parseLong(stopTimeoutStr);
119+
final String timeoutTimeUnitStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT_TIMEUNIT);
120+
final TimeUnit timeoutTimeUnit = Strings.isEmpty(timeoutTimeUnitStr) ? DEFAULT_STOP_TIMEOUT_TIMEUNIT
121+
: TimeUnit.valueOf(toRootUpperCase(timeoutTimeUnitStr));
122+
((LifeCycle2) this.initializer).stop(stopTimeout, timeoutTimeUnit);
123+
} else {
124+
this.initializer.stop();
125+
}
126+
} catch (final IllegalStateException e) {
127+
throw new IllegalStateException("Failed to shutdown Log4j properly.", e);
92128
}
93129
}
94130
}

0 commit comments

Comments
 (0)