Allow multiple Log4jServletContextListener registrations

This closes apache#1782.
ppkarwasz · Sep 11, 2023 · d620881 · d620881
1 parent 05afc58
commit d620881
Show file tree

Hide file tree

Showing 5 changed files with 154 additions and 14 deletions.
diff --git a/...j-jakarta-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java b/...j-jakarta-web/src/main/java/org/apache/logging/log4j/web/Log4jServletContextListener.java
@@ -36,6 +36,8 @@
  */
 public class Log4jServletContextListener implements ServletContextListener {
 
+    static final String START_COUNT_ATTR = Log4jServletContextListener.class.getName() + ".START_COUNT";
+
     private static final int DEFAULT_STOP_TIMEOUT = 30;
     private static final TimeUnit DEFAULT_STOP_TIMEOUT_TIMEUNIT = TimeUnit.SECONDS;
 
@@ -47,10 +49,36 @@ 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 (getAndIncrementCount() != 0) {
+            LOGGER.debug("Skipping Log4j context initialization, since {} is registered multiple times.",
+                    getClass().getSimpleName());
+            return;
+        }
+        LOGGER.info("Log4j context initialization by {}.", getClass().getSimpleName());
 
         if ("true".equalsIgnoreCase(servletContext.getInitParameter(
                 Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED))) {
@@ -72,10 +100,15 @@ public void contextInitialized(final ServletContextEvent event) {
     @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.");
+        LOGGER.info("Log4j context shutdown by {}.", getClass().getSimpleName());
 
         this.initializer.clearLoggerContext(); // the application is finished
         // shutting down now

diff --git a/...karta-web/src/test/java/org/apache/logging/log4j/web/Log4jServletContextListenerTest.java b/...karta-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 jakarta.servlet.ServletContext;
 import jakarta.servlet.ServletContextEvent;
 
@@ -26,11 +28,15 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
-import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.BDDMockito.eq;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+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 {
@@ -46,21 +52,38 @@ public class Log4jServletContextListenerTest {
 
     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);
+
+        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);
     }
 
     @Test
     public void testInitAndDestroy() throws Exception {
-        this.listener.contextInitialized(this.event);
+        listener.contextInitialized(event);
 
         then(initializer).should().start();
         then(initializer).should().setLoggerContext();
 
-        this.listener.contextDestroyed(this.event);
+        listener.contextInitialized(event);
+
+        then(initializer).shouldHaveNoMoreInteractions();
+
+        listener.contextDestroyed(event);
+
+        then(initializer).shouldHaveNoMoreInteractions();
+
+        listener.contextDestroyed(event);
 
         then(initializer).should().clearLoggerContext();
         then(initializer).should().stop();

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
@@ -36,6 +36,8 @@
  */
 public class Log4jServletContextListener implements ServletContextListener {
 
+    static final String START_COUNT_ATTR = Log4jServletContextListener.class.getName() + ".START_COUNT";
+
     private static final int DEFAULT_STOP_TIMEOUT = 30;
     private static final TimeUnit DEFAULT_STOP_TIMEOUT_TIMEUNIT = TimeUnit.SECONDS;
 
@@ -47,10 +49,36 @@ 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 (getAndIncrementCount() != 0) {
+            LOGGER.debug("Skipping Log4j context initialization, since {} is registered multiple times.",
+                    getClass().getSimpleName());
+            return;
+        }
+        LOGGER.info("Log4j context initialization by {}.", getClass().getSimpleName());
 
         if ("true".equalsIgnoreCase(servletContext.getInitParameter(
                 Log4jWebSupport.IS_LOG4J_AUTO_SHUTDOWN_DISABLED))) {
@@ -72,10 +100,15 @@ public void contextInitialized(final ServletContextEvent event) {
     @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.");
+        LOGGER.info("Log4j context shutdown by {}.", getClass().getSimpleName());
 
         this.initializer.clearLoggerContext(); // the application is finished
         // shutting down now

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
@@ -16,6 +16,8 @@
  */
 package org.apache.logging.log4j.web;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 import javax.servlet.ServletContext;
 import javax.servlet.ServletContextEvent;
 
@@ -26,11 +28,15 @@
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 
-import static org.junit.jupiter.api.Assertions.*;
-import static org.mockito.BDDMockito.eq;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
+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 {
@@ -46,21 +52,38 @@ public class Log4jServletContextListenerTest {
 
     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);
+
+        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);
     }
 
     @Test
     public void testInitAndDestroy() throws Exception {
-        this.listener.contextInitialized(this.event);
+        listener.contextInitialized(event);
 
         then(initializer).should().start();
         then(initializer).should().setLoggerContext();
 
-        this.listener.contextDestroyed(this.event);
+        listener.contextInitialized(event);
+
+        then(initializer).shouldHaveNoMoreInteractions();
+
+        listener.contextDestroyed(event);
+
+        then(initializer).shouldHaveNoMoreInteractions();
+
+        listener.contextDestroyed(event);
 
         then(initializer).should().clearLoggerContext();
         then(initializer).should().stop();

diff --git a/src/changelog/.2.x.x/1782_allow_multiple_servletcontextlistener_instances.xml b/src/changelog/.2.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>