diff --git a/apps/showcase/pom.xml b/apps/showcase/pom.xml index e54572c7aa..0754eaeace 100644 --- a/apps/showcase/pom.xml +++ b/apps/showcase/pom.xml @@ -119,6 +119,10 @@ org.apache.logging.log4j log4j-slf4j-impl + + org.apache.logging.log4j + log4j-web + org.sitemesh diff --git a/core/src/main/java/org/apache/struts2/components/Component.java b/core/src/main/java/org/apache/struts2/components/Component.java index c9cd1bd675..10e6373cc2 100644 --- a/core/src/main/java/org/apache/struts2/components/Component.java +++ b/core/src/main/java/org/apache/struts2/components/Component.java @@ -68,6 +68,14 @@ public class Component { */ protected static ConcurrentMap, Collection> standardAttributesMap = new ConcurrentHashMap<>(); + /** + * Clears the standard attributes cache to prevent classloader memory leaks during hot redeployment. + * The cache uses Class keys which pin the webapp classloader. + */ + public static void clearStandardAttributesMap() { + standardAttributesMap.clear(); + } + protected boolean devMode = false; protected boolean escapeHtmlBody = false; protected ValueStack stack; diff --git a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java index 70c62f7750..6c46fd2647 100644 --- a/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java +++ b/core/src/main/java/org/apache/struts2/config/impl/DefaultConfiguration.java @@ -249,6 +249,10 @@ public PackageConfig removePackageConfig(String packageName) { public void destroy() { packageContexts.clear(); loadedFileNames.clear(); + if (container != null) { + container.destroy(); + container = null; + } } @Override @@ -266,8 +270,7 @@ public void rebuildRuntimeConfiguration() { */ @Override public synchronized List reloadContainer(List providers) throws ConfigurationException { - packageContexts.clear(); - loadedFileNames.clear(); + destroy(); List packageProviders = new ArrayList<>(); ContainerProperties props = new ContainerProperties(); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ComponentCacheDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/ComponentCacheDestroyable.java new file mode 100644 index 0000000000..d581a333ed --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/ComponentCacheDestroyable.java @@ -0,0 +1,35 @@ +/* + * 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. + * + * @since 7.2.0 + */ +public class ComponentCacheDestroyable implements InternalDestroyable { + + @Override + public void destroy() { + Component.clearStandardAttributesMap(); + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java b/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java index 5e0b0ceb17..708d897f04 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/ContainerHolder.java @@ -20,28 +20,65 @@ import org.apache.struts2.inject.Container; +import java.util.concurrent.atomic.AtomicLong; + /** - * 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 org.apache.struts2.config.ConfigurationManager}. *

- * 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 an AtomicLong 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 instance = new ThreadLocal<>(); + private static final ThreadLocal instance = new ThreadLocal<>(); + + /** + * Incremented on each {@link #invalidateAll()} call. Threads compare their cached + * generation against this value to detect staleness. + */ + private static final AtomicLong generation = new AtomicLong(); public static void store(Container newInstance) { - instance.set(newInstance); + instance.set(new CachedContainer(newInstance, generation.get())); } public static Container get() { - return instance.get(); + CachedContainer cached = instance.get(); + if (cached == null) { + return null; + } + if (cached.generation() != generation.get()) { + 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. + *

+ * 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.incrementAndGet(); + instance.remove(); + } + + private record CachedContainer(Container container, long generation) {} } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ContextAwareDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/ContextAwareDestroyable.java new file mode 100644 index 0000000000..09e27214ce --- /dev/null +++ b/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 jakarta.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 7.2.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 + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/DebugUtilsCacheDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/DebugUtilsCacheDestroyable.java new file mode 100644 index 0000000000..358e109fe0 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/DebugUtilsCacheDestroyable.java @@ -0,0 +1,35 @@ +/* + * 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.util.DebugUtils; + +/** + * Clears {@link DebugUtils}'s static logged-keys cache to prevent memory leaks + * during hot redeployment. + * + * @since 7.2.0 + */ +public class DebugUtilsCacheDestroyable implements InternalDestroyable { + + @Override + public void destroy() { + DebugUtils.clearCache(); + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 5cbd66b569..34d6c13c87 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -441,37 +441,78 @@ public void setThreadAllowlist(ThreadAllowlist threadAllowlist) { * Releases all instances bound to this dispatcher instance. */ public void cleanup() { - // clean up ObjectFactory + destroyObjectFactory(); + + // clean up Dispatcher itself for this thread + instance.remove(); + servletContext.setAttribute(StrutsStatics.SERVLET_DISPATCHER, null); + + destroyDispatcherListeners(); + + destroyInterceptors(); + + destroyInternalBeans(); + + // WW-5537: Invalidate all threads' cached Container references to prevent + // classloader leaks from idle pool threads retaining stale references after undeploy. + ContainerHolder.invalidateAll(); + + //cleanup action context + ActionContext.clear(); + + // clean up configuration + configurationManager.destroyConfiguration(); + configurationManager = null; + } + + /** + * Destroys the {@link ObjectFactory} if it implements {@link ObjectFactoryDestroyable}. + * + * @since 7.2.0 + */ + protected void destroyObjectFactory() { if (objectFactory == null) { LOG.warn("Object Factory is null, something is seriously wrong, no clean up will be performed"); + return; } - if (objectFactory instanceof ObjectFactoryDestroyable) { + if (objectFactory instanceof ObjectFactoryDestroyable ofd) { try { - ((ObjectFactoryDestroyable) objectFactory).destroy(); + ofd.destroy(); } catch (Exception e) { - // catch any exception that may occur during destroy() and log it LOG.error("Exception occurred while destroying ObjectFactory [{}]", objectFactory.toString(), e); } } + } - // clean up Dispatcher itself for this thread - instance.remove(); - servletContext.setAttribute(StrutsStatics.SERVLET_DISPATCHER, null); - - // clean up DispatcherListeners + /** + * Notifies all registered {@link DispatcherListener}s that this dispatcher + * is being destroyed, then clears the listener list. + * + * @since 7.2.0 + */ + protected void destroyDispatcherListeners() { if (!dispatcherListeners.isEmpty()) { for (DispatcherListener l : dispatcherListeners) { l.dispatcherDestroyed(this); } + // WW-5537: Clear the static listener list to release references that may + // pin the webapp classloader after undeploy. + dispatcherListeners.clear(); } + } - // clean up all interceptors by calling their destroy() method + /** + * Destroys all interceptors registered in the current configuration. + * + * @since 7.2.0 + */ + protected void destroyInterceptors() { Set interceptors = new HashSet<>(); Collection packageConfigs = configurationManager.getConfiguration().getPackageConfigs().values(); for (PackageConfig packageConfig : packageConfigs) { for (Object config : packageConfig.getAllInterceptorConfigs().values()) { - if (config instanceof InterceptorStackConfig) { - for (InterceptorMapping interceptorMapping : ((InterceptorStackConfig) config).getInterceptors()) { + if (config instanceof InterceptorStackConfig isc) { + for (InterceptorMapping interceptorMapping : isc.getInterceptors()) { interceptors.add(interceptorMapping.getInterceptor()); } } @@ -480,16 +521,38 @@ public void cleanup() { for (Interceptor interceptor : interceptors) { interceptor.destroy(); } + } - // Clear container holder when application is unloaded / server shutdown - ContainerHolder.clear(); - - //cleanup action context - ActionContext.clear(); - - // clean up configuration - configurationManager.destroyConfiguration(); - configurationManager = null; + /** + * Discovers and invokes all {@link InternalDestroyable} beans registered + * in the container, clearing static caches and stopping daemon threads + * to prevent classloader leaks during hot redeployment (WW-5537). + * + *

Beans implementing {@link ContextAwareDestroyable} receive the + * {@link jakarta.servlet.ServletContext} via + * {@link ContextAwareDestroyable#destroy(jakarta.servlet.ServletContext)}.

+ * + * @since 7.2.0 + */ + protected void destroyInternalBeans() { + if (configurationManager != null && configurationManager.getConfiguration() != null) { + Container container = configurationManager.getConfiguration().getContainer(); + Set destroyableNames = container.getInstanceNames(InternalDestroyable.class); + for (String name : destroyableNames) { + try { + InternalDestroyable destroyable = container.getInstance(InternalDestroyable.class, name); + if (destroyable instanceof ContextAwareDestroyable cad) { + cad.destroy(servletContext); + } else { + destroyable.destroy(); + } + } catch (Exception e) { + LOG.warn("Error during internal cleanup [{}]", name, e); + } + } + } else { + LOG.warn("ConfigurationManager is null during cleanup, InternalDestroyable beans will not be invoked"); + } } private void init_FileManager() throws ClassNotFoundException { diff --git a/core/src/main/java/org/apache/struts2/dispatcher/FinalizableReferenceQueueDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/FinalizableReferenceQueueDestroyable.java new file mode 100644 index 0000000000..cac8114bc9 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/FinalizableReferenceQueueDestroyable.java @@ -0,0 +1,35 @@ +/* + * 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.inject.util.FinalizableReferenceQueue; + +/** + * Adapter that exposes {@link FinalizableReferenceQueue#stopAndClear()} as an + * {@link InternalDestroyable} bean. + * + * @since 7.2.0 + */ +public class FinalizableReferenceQueueDestroyable implements InternalDestroyable { + + @Override + public void destroy() { + FinalizableReferenceQueue.stopAndClear(); + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/FreemarkerCacheDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/FreemarkerCacheDestroyable.java new file mode 100644 index 0000000000..16899cdb2a --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/FreemarkerCacheDestroyable.java @@ -0,0 +1,56 @@ +/* + * 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 freemarker.ext.beans.BeansWrapper; +import freemarker.template.Configuration; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.apache.struts2.views.freemarker.FreemarkerManager; + +import jakarta.servlet.ServletContext; + +/** + * WW-5537: Clears FreeMarker's template and class introspection caches + * stored in {@link ServletContext} during application undeploy, preventing + * classloader leaks. + * + * @since 7.2.0 + */ +public class FreemarkerCacheDestroyable implements ContextAwareDestroyable { + + private static final Logger LOG = LogManager.getLogger(FreemarkerCacheDestroyable.class); + + @Override + public void destroy(ServletContext servletContext) { + if (servletContext == null) { + return; + } + Object fmConfig = servletContext.getAttribute(FreemarkerManager.CONFIG_SERVLET_CONTEXT_KEY); + if (fmConfig instanceof Configuration cfg) { + cfg.clearTemplateCache(); + cfg.clearEncodingMap(); + if (cfg.getObjectWrapper() instanceof BeansWrapper bw) { + bw.clearClassIntrospectionCache(); + } + servletContext.removeAttribute(FreemarkerManager.CONFIG_SERVLET_CONTEXT_KEY); + LOG.debug("FreeMarker configuration cleaned up"); + } + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/InternalDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/InternalDestroyable.java new file mode 100644 index 0000000000..2207d4d58a --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/InternalDestroyable.java @@ -0,0 +1,45 @@ +/* + * 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; + +/** + * Internal framework interface for components that hold static state + * (caches, daemon threads, etc.) requiring cleanup during application + * undeploy to prevent classloader leaks. + * + * <p>Implementations are registered as named beans in {@code struts-beans.xml} + * (or plugin descriptors) with type {@code InternalDestroyable}. During + * {@link Dispatcher#cleanup()}, all registered implementations are discovered + * via {@code Container.getInstanceNames(InternalDestroyable.class)} and + * invoked automatically.</p> + * + * <p>This is not part of the public user API. For user/plugin lifecycle + * callbacks, use {@link DispatcherListener} instead.</p> + * + * @since 7.2.0 + * @see Dispatcher#cleanup() + */ +public interface InternalDestroyable { + + /** + * Releases static state held by this component. Called once during + * {@link Dispatcher#cleanup()}. + */ + void destroy(); +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/OgnlCacheDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/OgnlCacheDestroyable.java new file mode 100644 index 0000000000..f19afdcdb6 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/OgnlCacheDestroyable.java @@ -0,0 +1,38 @@ +/* + * 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.ognl.OgnlUtil; + +import java.beans.Introspector; + +/** + * Clears OGNL runtime caches and JDK introspection caches that hold + * {@code Class} references, preventing classloader leaks on hot redeploy. + * + * @since 7.2.0 + */ +public class OgnlCacheDestroyable implements InternalDestroyable { + + @Override + public void destroy() { + OgnlUtil.clearRuntimeCache(); + Introspector.flushCaches(); + } +} diff --git a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java index 09df2c15f3..eb37b4812d 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/PrepareOperations.java @@ -77,6 +77,7 @@ public void cleanupRequest(final HttpServletRequest request) { } finally { ActionContext.clear(); Dispatcher.clearInstance(); + ContainerHolder.clear(); devModeOverride.remove(); } }); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/ScopeInterceptorCacheDestroyable.java b/core/src/main/java/org/apache/struts2/dispatcher/ScopeInterceptorCacheDestroyable.java new file mode 100644 index 0000000000..26005b58d8 --- /dev/null +++ b/core/src/main/java/org/apache/struts2/dispatcher/ScopeInterceptorCacheDestroyable.java @@ -0,0 +1,35 @@ +/* + * 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.interceptor.ScopeInterceptor; + +/** + * Clears {@link ScopeInterceptor}'s static locks map to prevent classloader + * leaks on hot redeploy. + * + * @since 7.2.0 + */ +public class ScopeInterceptorCacheDestroyable implements InternalDestroyable { + + @Override + public void destroy() { + ScopeInterceptor.clearLocks(); + } +} diff --git a/core/src/main/java/org/apache/struts2/inject/Container.java b/core/src/main/java/org/apache/struts2/inject/Container.java index cc20cf17bf..38513777d6 100644 --- a/core/src/main/java/org/apache/struts2/inject/Container.java +++ b/core/src/main/java/org/apache/struts2/inject/Container.java @@ -130,4 +130,13 @@ public interface Container extends Serializable { * Removes the scope strategy for the current thread. */ void removeScopeStrategy(); + + /** + * Releases all internal resources held by this container, including caches, + * factory maps, and thread-local state. This allows the webapp classloader + * to be garbage collected after hot redeployment. + * + * @since 7.2.0 + */ + void destroy(); } diff --git a/core/src/main/java/org/apache/struts2/inject/ContainerImpl.java b/core/src/main/java/org/apache/struts2/inject/ContainerImpl.java index a690b3c384..fc53bd0e6a 100644 --- a/core/src/main/java/org/apache/struts2/inject/ContainerImpl.java +++ b/core/src/main/java/org/apache/struts2/inject/ContainerImpl.java @@ -651,6 +651,25 @@ interface Injector extends Serializable { void inject(InternalContext context, Object o); } + /** + * Clears all internal caches, factory maps, and ThreadLocals to release + * Class references that would otherwise pin the webapp classloader after undeploy. + *

+ * The {@code injectors} and {@code constructors} ReferenceCache maps hold + * {@code Class} keys and reflection accessor objects ({@code Method}, + * {@code Constructor}) whose JDK-generated {@code DelegatingClassLoader} + * instances retain the webapp classloader as their parent. + * + * @since 7.2.0 + */ + @Override + public void destroy() { + injectors.clear(); + constructors.clear(); + localContext.remove(); + localScopeStrategy.remove(); + } + static class MissingDependencyException extends Exception { MissingDependencyException(String message) { super(message); diff --git a/core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java b/core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java index 6556000f76..07b7ececa9 100644 --- a/core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java +++ b/core/src/main/java/org/apache/struts2/inject/util/FinalizableReferenceQueue.java @@ -18,6 +18,7 @@ import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; +import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Level; import java.util.logging.Logger; @@ -26,11 +27,13 @@ * * @author Bob Lee (crazybob@google.com) */ -class FinalizableReferenceQueue extends ReferenceQueue { +public class FinalizableReferenceQueue extends ReferenceQueue { private static final Logger logger = Logger.getLogger(FinalizableReferenceQueue.class.getName()); + private final AtomicReference cleanupThread = new AtomicReference<>(); + private FinalizableReferenceQueue() {} void cleanUp(Reference reference) { @@ -49,18 +52,39 @@ 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) { + Thread.currentThread().interrupt(); + break; + } } } }; thread.setDaemon(true); thread.start(); + cleanupThread.set(thread); + } + + /** + * Stops the background cleanup thread to prevent classloader memory leaks during hot redeployment. + */ + void stop() { + Thread t = cleanupThread.getAndSet(null); + if (t != null) { + t.interrupt(); + try { + t.join(5000); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + } + t.setContextClassLoader(null); + } } - static ReferenceQueue instance = createAndStart(); + private static final AtomicReference> instance = + new AtomicReference<>(createAndStart()); static FinalizableReferenceQueue createAndStart() { FinalizableReferenceQueue queue = new FinalizableReferenceQueue(); @@ -72,6 +96,17 @@ static FinalizableReferenceQueue createAndStart() { * Gets instance. */ public static ReferenceQueue getInstance() { - return instance; + return instance.get(); + } + + /** + * Stops the cleanup thread and clears the instance to prevent classloader + * memory leaks during hot redeployment. + */ + public static void stopAndClear() { + ReferenceQueue q = instance.getAndSet(null); + if (q instanceof FinalizableReferenceQueue frq) { + frq.stop(); + } } } diff --git a/core/src/main/java/org/apache/struts2/interceptor/ScopeInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ScopeInterceptor.java index b899b7f84a..c654316c2d 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ScopeInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ScopeInterceptor.java @@ -263,6 +263,15 @@ static void unlock(Object o) { } } + /** + * Clears the locks map to prevent memory leaks during hot redeployment. + */ + public static void clearLocks() { + synchronized (locks) { + locks.clear(); + } + } + protected void after(ActionInvocation invocation, String result) throws Exception { Map session = ActionContext.getContext().getSession(); if ( session != null) { diff --git a/core/src/main/java/org/apache/struts2/mock/MockContainer.java b/core/src/main/java/org/apache/struts2/mock/MockContainer.java index 41fab15fe9..a03ec2d678 100644 --- a/core/src/main/java/org/apache/struts2/mock/MockContainer.java +++ b/core/src/main/java/org/apache/struts2/mock/MockContainer.java @@ -56,4 +56,9 @@ public void removeScopeStrategy() { } + @Override + public void destroy() { + // no-op in mock + } + } diff --git a/core/src/main/java/org/apache/struts2/ognl/accessor/CompoundRootAccessor.java b/core/src/main/java/org/apache/struts2/ognl/accessor/CompoundRootAccessor.java index d4cc814ddd..801be939bd 100644 --- a/core/src/main/java/org/apache/struts2/ognl/accessor/CompoundRootAccessor.java +++ b/core/src/main/java/org/apache/struts2/ognl/accessor/CompoundRootAccessor.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.ognl.accessor; +import org.apache.struts2.dispatcher.InternalDestroyable; import org.apache.struts2.inject.Inject; import org.apache.struts2.ognl.OgnlValueStack; import org.apache.struts2.util.CompoundRoot; @@ -57,7 +58,7 @@ * @author Rainer Hermanns * @version $Revision$ */ -public class CompoundRootAccessor implements RootAccessor { +public class CompoundRootAccessor implements RootAccessor, InternalDestroyable { /** * Used by OGNl to generate bytecode @@ -322,6 +323,18 @@ private Class[] getArgTypes(Object[] args) { } + public static void clearCache() { + invalidMethods.clear(); + } + + /** + * @since 7.2.0 + */ + @Override + public void destroy() { + clearCache(); + } + static class MethodCall { Class clazz; String name; diff --git a/core/src/main/java/org/apache/struts2/util/DebugUtils.java b/core/src/main/java/org/apache/struts2/util/DebugUtils.java index cc8ca5f5f4..6938bb948f 100644 --- a/core/src/main/java/org/apache/struts2/util/DebugUtils.java +++ b/core/src/main/java/org/apache/struts2/util/DebugUtils.java @@ -32,6 +32,13 @@ public final class DebugUtils { private static final Set IS_LOGGED = ConcurrentHashMap.newKeySet(); + /** + * Clears the logged-keys cache to prevent memory leaks during hot redeployment. + */ + public static void clearCache() { + IS_LOGGED.clear(); + } + public static void notifyDeveloperOfError(Logger log, Object action, String message) { if (action instanceof TextProvider tp) { message = tp.getText("devmode.notification", "Developer Notification:\n{0}", new String[]{message}); diff --git a/core/src/main/java/org/apache/struts2/util/fs/DefaultFileManager.java b/core/src/main/java/org/apache/struts2/util/fs/DefaultFileManager.java index 82bf06348a..764cb386a7 100644 --- a/core/src/main/java/org/apache/struts2/util/fs/DefaultFileManager.java +++ b/core/src/main/java/org/apache/struts2/util/fs/DefaultFileManager.java @@ -19,6 +19,7 @@ package org.apache.struts2.util.fs; import org.apache.struts2.FileManager; +import org.apache.struts2.dispatcher.InternalDestroyable; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -40,7 +41,7 @@ /** * Default implementation of {@link FileManager} */ -public class DefaultFileManager implements FileManager { +public class DefaultFileManager implements FileManager, InternalDestroyable { private static final Logger LOG = LogManager.getLogger(DefaultFileManager.class); @@ -56,6 +57,19 @@ public class DefaultFileManager implements FileManager { public DefaultFileManager() { } + public static void clearCache() { + files.clear(); + lazyMonitoredFilesCache.clear(); + } + + /** + * @since 7.2.0 + */ + @Override + public void destroy() { + clearCache(); + } + public void setReloadingConfigs(boolean reloadingConfigs) { if (reloadingConfigs && !this.reloadingConfigs) { //starting monitoring cached not-monitored files (lazy monitoring on demand because of performance) diff --git a/core/src/main/resources/struts-beans.xml b/core/src/main/resources/struts-beans.xml index 7c59a88da3..6141786919 100644 --- a/core/src/main/resources/struts-beans.xml +++ b/core/src/main/resources/struts-beans.xml @@ -260,4 +260,22 @@ + + + + + + + + + + diff --git a/core/src/test/java/org/apache/struts2/dispatcher/ContainerHolderTest.java b/core/src/test/java/org/apache/struts2/dispatcher/ContainerHolderTest.java new file mode 100644 index 0000000000..a11717dbc3 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/ContainerHolderTest.java @@ -0,0 +1,78 @@ +/* + * 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.inject.Container; +import org.junit.After; +import org.junit.Test; + +import java.util.concurrent.atomic.AtomicReference; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +public class ContainerHolderTest { + + @After + public void tearDown() { + ContainerHolder.clear(); + } + + @Test + public void storeAndGet() { + Container c = mock(Container.class); + ContainerHolder.store(c); + assertThat(ContainerHolder.get()).isSameAs(c); + } + + @Test + public void clearRemovesCurrentThread() { + ContainerHolder.store(mock(Container.class)); + ContainerHolder.clear(); + assertThat(ContainerHolder.get()).isNull(); + } + + @Test + public void invalidateAllMakesOtherThreadsSeeNull() throws Exception { + Container c = mock(Container.class); + + // Another thread stores a container + Thread t = new Thread(() -> ContainerHolder.store(c)); + t.start(); + t.join(); + + // Invalidate on main thread + ContainerHolder.invalidateAll(); + + // Other thread's cached value should now be stale + AtomicReference otherThreadResult = new AtomicReference<>(); + Thread t2 = new Thread(() -> otherThreadResult.set(ContainerHolder.get())); + t2.start(); + t2.join(); + + assertThat(otherThreadResult.get()).isNull(); + } + + @Test + public void invalidateAllClearsCallingThread() { + ContainerHolder.store(mock(Container.class)); + ContainerHolder.invalidateAll(); + assertThat(ContainerHolder.get()).isNull(); + } +} diff --git a/core/src/test/java/org/apache/struts2/dispatcher/DispatcherCleanupTest.java b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherCleanupTest.java new file mode 100644 index 0000000000..d55cb4438b --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/DispatcherCleanupTest.java @@ -0,0 +1,185 @@ +/* + * 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.ActionContext; +import org.apache.struts2.StrutsJUnit4InternalTestCase; +import org.apache.struts2.components.Component; +import org.apache.struts2.inject.Container; +import org.apache.struts2.ognl.accessor.CompoundRootAccessor; +import org.apache.struts2.util.DebugUtils; +import org.apache.struts2.util.fs.DefaultFileManager; +import org.junit.Test; + +import java.lang.reflect.Field; +import java.net.URI; +import java.net.URL; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.ConcurrentMap; + +import static java.util.Collections.emptyMap; +import static org.assertj.core.api.Assertions.assertThat; + +/** + * WW-5537: Verifies that Dispatcher.cleanup() properly clears all static state + * that could prevent classloader garbage collection during hot redeployment. + */ +public class DispatcherCleanupTest extends StrutsJUnit4InternalTestCase { + + @Test + public void cleanupDiscoversAllInternalDestroyableBeans() { + initDispatcher(emptyMap()); + + Container container = dispatcher.getConfigurationManager().getConfiguration().getContainer(); + Set names = container.getInstanceNames(InternalDestroyable.class); + + Set expected = new HashSet<>(Arrays.asList( + "componentCache", "compoundRootAccessor", "defaultFileManager", + "scopeInterceptorCache", "ognlCache", "finalizableReferenceQueue", + "freemarkerCache", "debugUtilsCache" + )); + assertThat(names).containsAll(expected); + } + + @Test + @SuppressWarnings("unchecked") + public void cleanupClearsComponentStandardAttributesMap() throws Exception { + initDispatcher(emptyMap()); + + Field mapField = Component.class.getDeclaredField("standardAttributesMap"); + mapField.setAccessible(true); + ConcurrentMap, Collection> map = + (ConcurrentMap, Collection>) mapField.get(null); + + map.put(String.class, new ArrayList<>()); + assertThat(map).isNotEmpty(); + + dispatcher.cleanup(); + + assertThat(map).isEmpty(); + } + + @Test + @SuppressWarnings("unchecked") + public void cleanupClearsCompoundRootAccessorCache() throws Exception { + initDispatcher(emptyMap()); + + Field field = CompoundRootAccessor.class.getDeclaredField("invalidMethods"); + field.setAccessible(true); + Map invalidMethods = (Map) field.get(null); + + invalidMethods.put("testKey", Boolean.TRUE); + assertThat(invalidMethods).isNotEmpty(); + + dispatcher.cleanup(); + + assertThat(invalidMethods).isEmpty(); + } + + @Test + public void cleanupClearsDefaultFileManagerFilesMap() throws Exception { + initDispatcher(emptyMap()); + + Field filesField = DefaultFileManager.class.getDeclaredField("files"); + filesField.setAccessible(true); + @SuppressWarnings("unchecked") + Map files = (Map) filesField.get(null); + + files.put("test-key", new Object()); + assertThat(files).isNotEmpty(); + + dispatcher.cleanup(); + + assertThat(files).isEmpty(); + } + + @Test + public void cleanupClearsDefaultFileManagerLazyCache() throws Exception { + initDispatcher(emptyMap()); + + Field lazyCacheField = DefaultFileManager.class.getDeclaredField("lazyMonitoredFilesCache"); + lazyCacheField.setAccessible(true); + @SuppressWarnings("unchecked") + List lazyCache = (List) lazyCacheField.get(null); + + lazyCache.add(new URI("file:///test").toURL()); + assertThat(lazyCache).isNotEmpty(); + + dispatcher.cleanup(); + + assertThat(lazyCache).isEmpty(); + } + + @Test + public void cleanupClearsDispatcherListeners() throws Exception { + initDispatcher(emptyMap()); + + Dispatcher.addDispatcherListener(new DispatcherListener() { + @Override + public void dispatcherInitialized(Dispatcher du) { + // intentionally empty — test only verifies listener list is cleared + } + @Override + public void dispatcherDestroyed(Dispatcher du) { + // intentionally empty — test only verifies listener list is cleared + } + }); + + dispatcher.cleanup(); + + Field listenersField = Dispatcher.class.getDeclaredField("dispatcherListeners"); + listenersField.setAccessible(true); + List listeners = (List) listenersField.get(null); + assertThat(listeners).isEmpty(); + } + + @Test + public void cleanupClearsThreadLocals() { + assertThat(Dispatcher.getInstance()).isNotNull(); + assertThat(ActionContext.getContext()).isNotNull(); + + dispatcher.cleanup(); + + assertThat(Dispatcher.getInstance()).isNull(); + assertThat(ActionContext.getContext()).isNull(); + } + + @Test + @SuppressWarnings("unchecked") + public void cleanupClearsDebugUtilsCache() throws Exception { + initDispatcher(emptyMap()); + + Field field = DebugUtils.class.getDeclaredField("IS_LOGGED"); + field.setAccessible(true); + Set isLogged = (Set) field.get(null); + + isLogged.add("test-key"); + assertThat(isLogged).isNotEmpty(); + + dispatcher.cleanup(); + + assertThat(isLogged).isEmpty(); + } +} diff --git a/core/src/test/java/org/apache/struts2/util/fs/DefaultFileManagerFactoryTest.java b/core/src/test/java/org/apache/struts2/util/fs/DefaultFileManagerFactoryTest.java index a09dc0cadd..55617b7c20 100644 --- a/core/src/test/java/org/apache/struts2/util/fs/DefaultFileManagerFactoryTest.java +++ b/core/src/test/java/org/apache/struts2/util/fs/DefaultFileManagerFactoryTest.java @@ -112,6 +112,10 @@ public void setScopeStrategy(Scope.Strategy scopeStrategy) { public void removeScopeStrategy() { } + public void destroy() { + // no-op in test dummy + } + } class DummyFileManager implements FileManager { diff --git a/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java b/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java index 22c1a2b9d5..4823f5c5a2 100644 --- a/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java +++ b/core/src/test/java/org/apache/struts2/views/util/DefaultUrlHelperTest.java @@ -393,5 +393,9 @@ public void removeScopeStrategy() { public void setScopeStrategy(Strategy scopeStrategy) { parent.setScopeStrategy(scopeStrategy); } + + public void destroy() { + parent.destroy(); + } } } diff --git a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java index fce42e6171..7b5d579599 100644 --- a/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java +++ b/plugins/convention/src/test/java/org/apache/struts2/convention/PackageBasedActionConfigBuilderTest.java @@ -1264,5 +1264,9 @@ public void removeScopeStrategy() { public void setScopeStrategy(Strategy scopeStrategy) { } + public void destroy() { + // no-op in test dummy + } + } } diff --git a/plugins/json/src/main/java/org/apache/struts2/json/JSONCacheDestroyable.java b/plugins/json/src/main/java/org/apache/struts2/json/JSONCacheDestroyable.java new file mode 100644 index 0000000000..032951aec1 --- /dev/null +++ b/plugins/json/src/main/java/org/apache/struts2/json/JSONCacheDestroyable.java @@ -0,0 +1,35 @@ +/* + * 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.json; + +import org.apache.struts2.dispatcher.InternalDestroyable; + +/** + * WW-5537: Clears JSON plugin's static BeanInfo caches when the Dispatcher is + * destroyed, preventing classloader leaks during hot redeployment. + * + * @since 7.2.0 + */ +public class JSONCacheDestroyable implements InternalDestroyable { + + @Override + public void destroy() { + StrutsJSONWriter.clearBeanInfoCaches(); + } +} diff --git a/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java b/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java index c75e54e0a1..3918004741 100644 --- a/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java +++ b/plugins/json/src/main/java/org/apache/struts2/json/StrutsJSONWriter.java @@ -76,6 +76,14 @@ public class StrutsJSONWriter implements JSONWriter { private static final ConcurrentMap, BeanInfo> BEAN_INFO_CACHE_IGNORE_HIERARCHY = new ConcurrentHashMap<>(); private static final ConcurrentMap, BeanInfo> BEAN_INFO_CACHE = new ConcurrentHashMap<>(); + /** + * Clears both BeanInfo caches to prevent classloader leaks on hot redeploy. + */ + public static void clearBeanInfoCaches() { + BEAN_INFO_CACHE_IGNORE_HIERARCHY.clear(); + BEAN_INFO_CACHE.clear(); + } + private final StringBuilder buf = new StringBuilder(); private final Deque stack = new ArrayDeque<>(); private boolean ignoreHierarchy = true; diff --git a/plugins/json/src/main/resources/struts-plugin.xml b/plugins/json/src/main/resources/struts-plugin.xml index 88451b39b4..3d246ff368 100644 --- a/plugins/json/src/main/resources/struts-plugin.xml +++ b/plugins/json/src/main/resources/struts-plugin.xml @@ -29,6 +29,8 @@ + diff --git a/plugins/rest/src/test/java/org/apache/struts2/rest/DefaultContentTypeHandlerManagerTest.java b/plugins/rest/src/test/java/org/apache/struts2/rest/DefaultContentTypeHandlerManagerTest.java index 5f54f63f08..432cdc9216 100644 --- a/plugins/rest/src/test/java/org/apache/struts2/rest/DefaultContentTypeHandlerManagerTest.java +++ b/plugins/rest/src/test/java/org/apache/struts2/rest/DefaultContentTypeHandlerManagerTest.java @@ -184,4 +184,9 @@ public void setScopeStrategy(Scope.Strategy scopeStrategy) { public void removeScopeStrategy() { } + + @Override + public void destroy() { + // no-op in test mock + } }