This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 05cd238b63ba4167760b5cb0293fd2fd145e4856 Author: Piotr P. Karwasz <[email protected]> AuthorDate: Mon Sep 11 13:11:37 2023 +0200 Allow multiple Log4jServletContextListener registrations This closes #1782. --- .../log4j/web/Log4jServletContextListener.java | 64 ++++++++++++---- .../log4j/web/Log4jServletContextListenerTest.java | 88 ++++++++++++++-------- ...w_multiple_servletcontextlistener_instances.xml | 28 +++++++ 3 files changed, 133 insertions(+), 47 deletions(-) diff --git a/log4j-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java b/log4j-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java index 56020841c2..903deb9bc4 100644 --- a/log4j-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java +++ b/log4j-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java @@ -35,6 +35,8 @@ import static org.apache.logging.log4j.util.Strings.toRootUpperCase; */ public class Log4jServletContextListener implements ServletContextListener { + static final String START_COUNT_ATTR = Log4jServletContextListener.class.getName() + ".START_COUNT"; + static final int DEFAULT_STOP_TIMEOUT = 30; static final TimeUnit DEFAULT_STOP_TIMEOUT_TIMEUNIT = TimeUnit.SECONDS; @@ -46,11 +48,30 @@ public class Log4jServletContextListener implements ServletContextListener { private ServletContext servletContext; private Log4jWebLifeCycle initializer; + private int getAndIncrementCount() { + Integer count = (Integer) servletContext.getAttribute(START_COUNT_ATTR); + if (count == null) { + count = 0; + } + servletContext.setAttribute(START_COUNT_ATTR, count + 1); + return count; + } + + private int decrementAndGetCount() { + Integer count = (Integer) servletContext.getAttribute(START_COUNT_ATTR); + if (count == null) { + LOGGER.warn( + "{} received a 'contextDestroyed' message without a corresponding 'contextInitialized' message.", + getClass().getName()); + count = 1; + } + servletContext.setAttribute(START_COUNT_ATTR, --count); + return count; + } + @Override public void contextInitialized(final ServletContextEvent event) { this.servletContext = event.getServletContext(); - LOGGER.debug("Log4jServletContextListener ensuring that Log4j starts up properly."); - if ("true".equalsIgnoreCase(servletContext.getInitParameter( Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED))) { throw new IllegalStateException("Do not use " + getClass().getSimpleName() + " when " @@ -60,6 +81,12 @@ public class Log4jServletContextListener implements ServletContextListener { } this.initializer = WebLoggerContextUtils.getWebLifeCycle(this.servletContext); + if (getAndIncrementCount() != 0) { + LOGGER.debug("Skipping Log4j context initialization, since {} is registered multiple times.", + getClass().getSimpleName()); + return; + } + LOGGER.info("{} triggered a Log4j context initialization.", getClass().getSimpleName()); try { this.initializer.start(); this.initializer.setLoggerContext(); // the application is just now starting to start up @@ -71,19 +98,28 @@ public class Log4jServletContextListener implements ServletContextListener { @Override public void contextDestroyed(final ServletContextEvent event) { if (this.servletContext == null || this.initializer == null) { - LOGGER.warn("Context destroyed before it was initialized."); + LOGGER.warn("Servlet context destroyed before it was initialized."); + return; + } + + if (decrementAndGetCount() != 0) { + LOGGER.debug("Skipping Log4j context shutdown, since {} is registered multiple times.", + getClass().getSimpleName()); return; } - LOGGER.debug("Log4jServletContextListener ensuring that Log4j shuts down properly."); - - this.initializer.clearLoggerContext(); // the application is finished - // shutting down now - final String stopTimeoutStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT); - final long stopTimeout = Strings.isEmpty(stopTimeoutStr) ? DEFAULT_STOP_TIMEOUT - : Long.parseLong(stopTimeoutStr); - final String timeoutTimeUnitStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT_TIMEUNIT); - final TimeUnit timeoutTimeUnit = Strings.isEmpty(timeoutTimeUnitStr) ? DEFAULT_STOP_TIMEOUT_TIMEUNIT - : TimeUnit.valueOf(toRootUpperCase(timeoutTimeUnitStr)); - this.initializer.stop(stopTimeout, timeoutTimeUnit); + LOGGER.info("{} triggered a Log4j context shutdown.", getClass().getSimpleName()); + try { + this.initializer.clearLoggerContext(); // the application is finished + // shutting down now + final String stopTimeoutStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT); + final long stopTimeout = Strings.isEmpty(stopTimeoutStr) ? DEFAULT_STOP_TIMEOUT + : Long.parseLong(stopTimeoutStr); + final String timeoutTimeUnitStr = servletContext.getInitParameter(KEY_STOP_TIMEOUT_TIMEUNIT); + final TimeUnit timeoutTimeUnit = Strings.isEmpty(timeoutTimeUnitStr) ? DEFAULT_STOP_TIMEOUT_TIMEUNIT + : TimeUnit.valueOf(toRootUpperCase(timeoutTimeUnitStr)); + this.initializer.stop(stopTimeout, timeoutTimeUnit); + } catch (final IllegalStateException e) { + throw new IllegalStateException("Failed to shutdown Log4j properly.", e); + } } } diff --git a/log4j-web/src/test/java/org/apache/logging/log4j/web/Log4jServletContextListenerTest.java b/log4j-web/src/test/java/org/apache/logging/log4j/web/Log4jServletContextListenerTest.java index 05544851a0..158e738bf7 100644 --- a/log4j-web/src/test/java/org/apache/logging/log4j/web/Log4jServletContextListenerTest.java +++ b/log4j-web/src/test/java/org/apache/logging/log4j/web/Log4jServletContextListenerTest.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.web; +import java.util.concurrent.atomic.AtomicReference; + import javax.servlet.ServletContext; import javax.servlet.ServletContextEvent; @@ -23,62 +25,87 @@ import org.apache.logging.log4j.util.Strings; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mock; +import org.mockito.Mock.Strictness; import org.mockito.junit.jupiter.MockitoExtension; import static org.apache.logging.log4j.web.Log4jServletContextListener.DEFAULT_STOP_TIMEOUT; import static org.apache.logging.log4j.web.Log4jServletContextListener.DEFAULT_STOP_TIMEOUT_TIMEUNIT; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.AdditionalAnswers.answerVoid; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.then; import static org.mockito.BDDMockito.willThrow; +import static org.mockito.Mockito.doAnswer; @ExtendWith(MockitoExtension.class) public class Log4jServletContextListenerTest { /* event and servletContext are marked lenient because they aren't used in the * testDestroyWithNoInit but are only accessed during initialization */ - @Mock(lenient = true) + @Mock(strictness = Strictness.LENIENT) private ServletContextEvent event; - @Mock(lenient = true) + @Mock(strictness = Strictness.LENIENT) private ServletContext servletContext; @Mock private Log4jWebLifeCycle initializer; - private Log4jServletContextListener listener; + private final AtomicReference<Object> count = new AtomicReference<>(); @BeforeEach public void setUp() { - this.listener = new Log4jServletContextListener(); given(event.getServletContext()).willReturn(servletContext); given(servletContext.getAttribute(Log4jWebSupport.SUPPORT_ATTRIBUTE)).willReturn(initializer); - } - @Test - public void testInitAndDestroy() throws Exception { - this.listener.contextInitialized(this.event); + doAnswer(answerVoid((k, v) -> count.set(v))) + .when(servletContext) + .setAttribute(eq(Log4jServletContextListener.START_COUNT_ATTR), any()); + doAnswer(__ -> count.get()) + .when(servletContext) + .getAttribute(Log4jServletContextListener.START_COUNT_ATTR); + } - then(initializer).should().start(); - then(initializer).should().setLoggerContext(); + @ParameterizedTest + @ValueSource(ints = { 1, 2, 3 }) + public void testInitAndDestroy(final int listenerCount) throws Exception { + final Log4jServletContextListener[] listeners = new Log4jServletContextListener[listenerCount]; + for (int idx = 0; idx < listenerCount; idx++) { + final Log4jServletContextListener listener = new Log4jServletContextListener(); + listeners[idx] = listener; + + listener.contextInitialized(event); + if (idx == 0) { + then(initializer).should().start(); + then(initializer).should().setLoggerContext(); + } else { + then(initializer).shouldHaveNoMoreInteractions(); + } + } - this.listener.contextDestroyed(this.event); + for (int idx = listenerCount - 1; idx >= 0; idx--) { + final Log4jServletContextListener listener = listeners[idx]; - then(initializer).should().clearLoggerContext(); - then(initializer).should().stop(DEFAULT_STOP_TIMEOUT, DEFAULT_STOP_TIMEOUT_TIMEUNIT); + listener.contextDestroyed(event); + if (idx == 0) { + then(initializer).should().clearLoggerContext(); + then(initializer).should().stop(DEFAULT_STOP_TIMEOUT, DEFAULT_STOP_TIMEOUT_TIMEUNIT); + } else { + then(initializer).shouldHaveNoMoreInteractions(); + } + } } @Test public void testInitFailure() throws Exception { willThrow(new IllegalStateException(Strings.EMPTY)).given(initializer).start(); + final Log4jServletContextListener listener = new Log4jServletContextListener(); - try { - this.listener.contextInitialized(this.event); - fail("Expected a RuntimeException."); - } catch (final RuntimeException e) { - assertEquals("Failed to initialize Log4j properly.", e.getMessage(), "The message is not correct."); - } + assertThrows(RuntimeException.class, () -> listener.contextInitialized(this.event), + "Failed to initialize Log4j properly."); } @Test @@ -96,17 +123,12 @@ public class Log4jServletContextListenerTest { } private void ensureInitializingFailsWhenAuthShutdownIsEnabled() { - try { - this.listener.contextInitialized(this.event); - fail("Expected a RuntimeException."); - } catch (final RuntimeException e) { - final String expectedMessage = - "Do not use " + Log4jServletContextListener.class.getSimpleName() + " when " - + Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED + " is true. Please use " - + Log4jShutdownOnContextDestroyedListener.class.getSimpleName() + " instead of " - + Log4jServletContextListener.class.getSimpleName() + "."; - - assertEquals(expectedMessage, e.getMessage(), "The message is not correct"); - } + final Log4jServletContextListener listener = new Log4jServletContextListener(); + final String message = "Do not use " + Log4jServletContextListener.class.getSimpleName() + " when " + + Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED + " is true. Please use " + + Log4jShutdownOnContextDestroyedListener.class.getSimpleName() + " instead of " + + Log4jServletContextListener.class.getSimpleName() + "."; + + assertThrows(RuntimeException.class, () -> listener.contextInitialized(event), message); } } diff --git a/src/changelog/.3.x.x/1782_allow_multiple_servletcontextlistener_instances.xml b/src/changelog/.3.x.x/1782_allow_multiple_servletcontextlistener_instances.xml new file mode 100644 index 0000000000..50f3f61988 --- /dev/null +++ b/src/changelog/.3.x.x/1782_allow_multiple_servletcontextlistener_instances.xml @@ -0,0 +1,28 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- + ~ 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. + --> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xmlns="http://logging.apache.org/log4j/changelog"; + xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.0.xsd"; + type="fixed"> + <issue id="1782" link="https://github.com/apache/logging-log4j2/issues/1782"/> + <author name="Christian Seewald" id="github:cseewald"/> + <author id="github:ppkarwasz"/> + <description format="asciidoc"> + Only shutdown Log4j after last `Log4jServletContextListener` is executed. + </description> +</entry>
