WW-5537 fix(core): resolve classloader/memory leaks during Tomcat hot deployment

apps/showcase/pom.xml

@@ -121,6 +121,10 @@
             <groupId>org.apache.logging.log4j</groupId>
             <artifactId>log4j-slf4j-impl</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.logging.log4j</groupId>
+            <artifactId>log4j-web</artifactId>
+        </dependency>
 
         <dependency>
             <groupId>opensymphony</groupId>

core/src/main/java/com/opensymphony/xwork2/inject/util/FinalizableReferenceQueue.java

@@ -26,7 +26,7 @@
  *
  * @author Bob Lee (crazybob@google.com)
  */
-class FinalizableReferenceQueue extends ReferenceQueue<Object> {
+public class FinalizableReferenceQueue extends ReferenceQueue<Object> {
 
   private static final Logger logger =
       Logger.getLogger(FinalizableReferenceQueue.class.getName());
@@ -45,22 +45,49 @@ void deliverBadNews(Throwable t) {
     logger.log(Level.SEVERE, "Error cleaning up after reference.", t);
   }
 
+  private volatile Thread drainThread;
+
   void start() {
     Thread thread = new Thread("FinalizableReferenceQueue") {
       @Override
       public void run() {
-        while (true) {
+        while (!Thread.currentThread().isInterrupted()) {
           try {
             cleanUp(remove());
-          } catch (InterruptedException e) { /* ignore */ }
+          } catch (InterruptedException e) {
+            break;
+          }
         }
       }
     };
     thread.setDaemon(true);
     thread.start();
+    this.drainThread = thread;
+  }
+
+  /**
+   * Stops the background drain thread and releases the singleton instance,
+   * preventing the webapp classloader from being pinned after undeploy.
+   */
+  public static synchronized void stopAndClear() {
+    if (instance instanceof FinalizableReferenceQueue) {
+      FinalizableReferenceQueue queue = (FinalizableReferenceQueue) instance;
+      Thread t = queue.drainThread;
+      if (t != null) {
+        t.interrupt();
+        try {
+          t.join(5000);
+        } catch (InterruptedException ignored) {
+          Thread.currentThread().interrupt();
+        }
+        t.setContextClassLoader(null);
+        queue.drainThread = null;
+      }
+    }
+    instance = null;
   }
 
-  static ReferenceQueue<Object> instance = createAndStart();
+  static volatile ReferenceQueue<Object> instance = createAndStart();
 
   static FinalizableReferenceQueue createAndStart() {
     FinalizableReferenceQueue queue = new FinalizableReferenceQueue();

core/src/main/java/com/opensymphony/xwork2/ognl/accessor/CompoundRootAccessor.java

@@ -33,6 +33,7 @@
 import org.apache.logging.log4j.Logger;
 import org.apache.struts2.StrutsConstants;
 import org.apache.struts2.StrutsException;
+import org.apache.struts2.dispatcher.InternalDestroyable;
 
 import java.beans.IntrospectionException;
 import java.beans.PropertyDescriptor;
@@ -53,7 +54,7 @@
  * @author Rainer Hermanns
  * @version $Revision$
  */
-public class CompoundRootAccessor implements RootAccessor {
+public class CompoundRootAccessor implements RootAccessor, InternalDestroyable {
 
     /**
      * Used by OGNl to generate bytecode
@@ -74,6 +75,22 @@ public String getSourceSetter(OgnlContext context, Object target, Object index)
     private final static Logger LOG = LogManager.getLogger(CompoundRootAccessor.class);
     private final static Class[] EMPTY_CLASS_ARRAY = new Class[0];
     private static final Map<MethodCall, Boolean> invalidMethods = new ConcurrentHashMap<>();
+
+    /**
+     * Clears the cached invalid methods map to prevent classloader leaks on hot redeploy.
+     */
+    public static void clearCache() {
+        invalidMethods.clear();
+    }
+
+    /**
+     * @since 6.9.0
+     */
+    @Override
+    public void destroy() {
+        clearCache();
+    }
+
     private boolean devMode;
     private boolean disallowCustomOgnlMap;
 

core/src/main/java/com/opensymphony/xwork2/util/fs/DefaultFileManager.java

@@ -21,6 +21,7 @@
 import com.opensymphony.xwork2.FileManager;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
+import org.apache.struts2.dispatcher.InternalDestroyable;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -33,7 +34,7 @@
 /**
  * Default implementation of {@link FileManager}
  */
-public class DefaultFileManager implements FileManager {
+public class DefaultFileManager implements FileManager, InternalDestroyable {
 
     private static Logger LOG = LogManager.getLogger(DefaultFileManager.class);
 
@@ -43,6 +44,22 @@ public class DefaultFileManager implements FileManager {
     protected static final Map<String, Revision> files = Collections.synchronizedMap(new HashMap<String, Revision>());
     private static final List<URL> lazyMonitoredFilesCache = Collections.synchronizedList(new ArrayList<URL>());
 
+    /**
+     * Clears both the files and lazy monitored files caches to prevent classloader leaks on hot redeploy.
+     */
+    public static void clearCache() {
+        files.clear();
+        lazyMonitoredFilesCache.clear();
+    }
+
+    /**
+     * @since 6.9.0
+     */
+    @Override
+    public void destroy() {
+        clearCache();
+    }
+
     protected boolean reloadingConfigs = false;
 
     public DefaultFileManager() {

core/src/main/java/org/apache/struts2/components/Component.java

@@ -68,6 +68,13 @@ public class Component {
      */
     protected static ConcurrentMap<Class<?>, Collection<String>> standardAttributesMap = new ConcurrentHashMap<>();
 
+    /**
+     * Clears the cached standard attributes map to prevent classloader leaks on hot redeploy.
+     */
+    public static void clearStandardAttributesMap() {
+        standardAttributesMap.clear();
+    }
+
     protected boolean devMode = false;
     protected boolean escapeHtmlBody = false;
     protected ValueStack stack;

core/src/main/java/org/apache/struts2/dispatcher/ComponentCacheDestroyable.java

@@ -0,0 +1,36 @@
+/*
+ * 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.struts2.dispatcher;
+
+import org.apache.struts2.components.Component;
+
+/**
+ * Clears {@link Component}'s static standard attributes cache to prevent
+ * classloader leaks on hot redeploy. Wrapper is needed because {@code Component}
+ * requires constructor arguments that prevent direct container instantiation.
+ *
+ * @since 6.9.0
+ */
+public class ComponentCacheDestroyable implements InternalDestroyable {
+
+    @Override
+    public void destroy() {
+        Component.clearStandardAttributesMap();
+    }
+}

core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java

@@ -21,27 +21,70 @@
 import com.opensymphony.xwork2.inject.Container;
 
 /**
- * Simple class to hold Container instance per thread to minimise number of attempts
- * to read configuration and build each time a new configuration.
+ * Per-thread cache for the Container instance, minimising repeated reads from
+ * {@link com.opensymphony.xwork2.config.ConfigurationManager}.
  * <p>
- * As ContainerHolder operates just per thread (which means per request) there is no need
- * to check if configuration changed during the same request. If changed between requests,
- * first call to store Container in ContainerHolder will be with the new configuration.
+ * WW-5537: Uses a ThreadLocal for per-request isolation with a volatile generation
+ * counter for cross-thread invalidation during app undeploy. When
+ * {@link #invalidateAll()} is called, all threads see the updated generation on their
+ * next {@link #get()} and return {@code null}, forcing a fresh read from
+ * ConfigurationManager. This prevents classloader leaks caused by idle pool threads
+ * retaining stale Container references after hot redeployment.
  */
 class ContainerHolder {
 
-    private static final ThreadLocal<Container> instance = new ThreadLocal<>();
+    private static final ThreadLocal<CachedContainer> instance = new ThreadLocal<>();
+
+    /**
+     * Incremented on each {@link #invalidateAll()} call. Threads compare their cached
+     * generation against this value to detect staleness.
+     */
+    private static volatile long generation = 0;
 
     public static void store(Container newInstance) {
-        instance.set(newInstance);
+        instance.set(new CachedContainer(newInstance, generation));
     }
 
     public static Container get() {
-        return instance.get();
+        CachedContainer cached = instance.get();
+        if (cached == null) {
+            return null;
+        }
+        if (cached.generation != generation) {
+            instance.remove();
+            return null;
+        }
+        return cached.container;
     }
 
+    /**
+     * Clears the current thread's cached container reference.
+     * Used for per-request cleanup.
+     */
     public static void clear() {
         instance.remove();
     }
 
+    /**
+     * Invalidates all threads' cached container references by advancing the generation
+     * counter. Each thread will detect the stale generation on its next {@link #get()}
+     * call and clear its own ThreadLocal. Also clears the calling thread immediately.
+     * <p>
+     * Used during application undeploy ({@link Dispatcher#cleanup()}) to ensure idle
+     * pool threads do not pin the webapp classloader via retained Container references.
+     */
+    public static void invalidateAll() {
+        generation++;
+        instance.remove();
+    }
+
+    private static class CachedContainer {
+        final Container container;
+        final long generation;
+
+        CachedContainer(Container container, long generation) {
+            this.container = container;
+            this.generation = generation;
+        }
+    }
 }

core/src/main/java/org/apache/struts2/dispatcher/ContextAwareDestroyable.java

@@ -0,0 +1,53 @@
+/*
+ * 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.struts2.dispatcher;
+
+import javax.servlet.ServletContext;
+
+/**
+ * Extension of {@link InternalDestroyable} for components that require
+ * {@link ServletContext} during cleanup (e.g. clearing servlet-scoped caches).
+ *
+ * <p>During {@link Dispatcher#cleanup()}, the discovery loop checks each
+ * {@code InternalDestroyable} bean: if it implements this subinterface,
+ * {@link #destroy(ServletContext)} is called instead of {@link #destroy()}.</p>
+ *
+ * @since 6.9.0
+ * @see InternalDestroyable
+ * @see Dispatcher#cleanup()
+ */
+public interface ContextAwareDestroyable extends InternalDestroyable {
+
+    /**
+     * Releases state that requires access to the {@link ServletContext}.
+     *
+     * @param servletContext the current servlet context, may be {@code null}
+     *                       if the Dispatcher was created without one
+     */
+    void destroy(ServletContext servletContext);
+
+    /**
+     * Default no-op — {@link Dispatcher} calls
+     * {@link #destroy(ServletContext)} instead when it recognises this type.
+     */
+    @Override
+    default void destroy() {
+        // no-op: context-aware variant is the real entry point
+    }
+}