Skip to content

Commit 0a8b111

Browse files
lukaszlenartclaude
andauthored
WW-5537 fix(core): resolve classloader/memory leaks during Tomcat hot deployment (#1631)
* WW-5537 fix(core): resolve classloader/memory leaks during Tomcat hot deployment Introduce InternalDestroyable interface with container-based discovery to clean up static caches, daemon threads, and shared references that pin the webapp classloader after undeploy. This prevents OutOfMemoryError (Metaspace) on repeated hot deployments. Changes: - Add InternalDestroyable/ContextAwareDestroyable interfaces for cleanup hooks - Clear OGNL, Component, ScopeInterceptor, DefaultFileManager static caches - Stop FinalizableReferenceQueue daemon thread and null its classloader - Clear FreeMarker template/introspection caches from ServletContext - Replace ContainerHolder ThreadLocal with volatile to prevent thread-pool leaks - Clear static dispatcherListeners list on Dispatcher cleanup - Add JSONCacheDestroyable for json plugin cache cleanup - Register all destroyables via struts-beans.xml / struts-plugin.xml Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * WW-5537 chore(showcase): add log4j-web for proper Log4j2 lifecycle in Servlet container Without log4j-web, Log4j2 SoftReferences delay classloader GC after undeploy. The log4j-web module provides Log4jServletContextListener which ensures proper Log4j2 shutdown during ServletContext destruction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * WW-5537 fix(core): use ThreadLocal with generation counter in ContainerHolder Replace the volatile shared reference with a ThreadLocal backed by a volatile generation counter. Per-request clear() only affects the current thread (safe for concurrent requests and tests). On undeploy, invalidateAll() advances the generation counter so idle pool threads detect staleness on next access and self-clear, preventing classloader leaks without breaking test isolation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 696ee73 commit 0a8b111

File tree

23 files changed

+865
-32
lines changed

23 files changed

+865
-32
lines changed

apps/showcase/pom.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,10 @@
121121
<groupId>org.apache.logging.log4j</groupId>
122122
<artifactId>log4j-slf4j-impl</artifactId>
123123
</dependency>
124+
<dependency>
125+
<groupId>org.apache.logging.log4j</groupId>
126+
<artifactId>log4j-web</artifactId>
127+
</dependency>
124128

125129
<dependency>
126130
<groupId>opensymphony</groupId>

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

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
*
2727
* @author Bob Lee (crazybob@google.com)
2828
*/
29-
class FinalizableReferenceQueue extends ReferenceQueue<Object> {
29+
public class FinalizableReferenceQueue extends ReferenceQueue<Object> {
3030

3131
private static final Logger logger =
3232
Logger.getLogger(FinalizableReferenceQueue.class.getName());
@@ -45,22 +45,49 @@ void deliverBadNews(Throwable t) {
4545
logger.log(Level.SEVERE, "Error cleaning up after reference.", t);
4646
}
4747

48+
private volatile Thread drainThread;
49+
4850
void start() {
4951
Thread thread = new Thread("FinalizableReferenceQueue") {
5052
@Override
5153
public void run() {
52-
while (true) {
54+
while (!Thread.currentThread().isInterrupted()) {
5355
try {
5456
cleanUp(remove());
55-
} catch (InterruptedException e) { /* ignore */ }
57+
} catch (InterruptedException e) {
58+
break;
59+
}
5660
}
5761
}
5862
};
5963
thread.setDaemon(true);
6064
thread.start();
65+
this.drainThread = thread;
66+
}
67+
68+
/**
69+
* Stops the background drain thread and releases the singleton instance,
70+
* preventing the webapp classloader from being pinned after undeploy.
71+
*/
72+
public static synchronized void stopAndClear() {
73+
if (instance instanceof FinalizableReferenceQueue) {
74+
FinalizableReferenceQueue queue = (FinalizableReferenceQueue) instance;
75+
Thread t = queue.drainThread;
76+
if (t != null) {
77+
t.interrupt();
78+
try {
79+
t.join(5000);
80+
} catch (InterruptedException ignored) {
81+
Thread.currentThread().interrupt();
82+
}
83+
t.setContextClassLoader(null);
84+
queue.drainThread = null;
85+
}
86+
}
87+
instance = null;
6188
}
6289

63-
static ReferenceQueue<Object> instance = createAndStart();
90+
static volatile ReferenceQueue<Object> instance = createAndStart();
6491

6592
static FinalizableReferenceQueue createAndStart() {
6693
FinalizableReferenceQueue queue = new FinalizableReferenceQueue();

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import org.apache.logging.log4j.Logger;
3434
import org.apache.struts2.StrutsConstants;
3535
import org.apache.struts2.StrutsException;
36+
import org.apache.struts2.dispatcher.InternalDestroyable;
3637

3738
import java.beans.IntrospectionException;
3839
import java.beans.PropertyDescriptor;
@@ -53,7 +54,7 @@
5354
* @author Rainer Hermanns
5455
* @version $Revision$
5556
*/
56-
public class CompoundRootAccessor implements RootAccessor {
57+
public class CompoundRootAccessor implements RootAccessor, InternalDestroyable {
5758

5859
/**
5960
* Used by OGNl to generate bytecode
@@ -74,6 +75,22 @@ public String getSourceSetter(OgnlContext context, Object target, Object index)
7475
private final static Logger LOG = LogManager.getLogger(CompoundRootAccessor.class);
7576
private final static Class[] EMPTY_CLASS_ARRAY = new Class[0];
7677
private static final Map<MethodCall, Boolean> invalidMethods = new ConcurrentHashMap<>();
78+
79+
/**
80+
* Clears the cached invalid methods map to prevent classloader leaks on hot redeploy.
81+
*/
82+
public static void clearCache() {
83+
invalidMethods.clear();
84+
}
85+
86+
/**
87+
* @since 6.9.0
88+
*/
89+
@Override
90+
public void destroy() {
91+
clearCache();
92+
}
93+
7794
private boolean devMode;
7895
private boolean disallowCustomOgnlMap;
7996

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.opensymphony.xwork2.FileManager;
2222
import org.apache.logging.log4j.LogManager;
2323
import org.apache.logging.log4j.Logger;
24+
import org.apache.struts2.dispatcher.InternalDestroyable;
2425

2526
import java.io.IOException;
2627
import java.io.InputStream;
@@ -33,7 +34,7 @@
3334
/**
3435
* Default implementation of {@link FileManager}
3536
*/
36-
public class DefaultFileManager implements FileManager {
37+
public class DefaultFileManager implements FileManager, InternalDestroyable {
3738

3839
private static Logger LOG = LogManager.getLogger(DefaultFileManager.class);
3940

@@ -43,6 +44,22 @@ public class DefaultFileManager implements FileManager {
4344
protected static final Map<String, Revision> files = Collections.synchronizedMap(new HashMap<String, Revision>());
4445
private static final List<URL> lazyMonitoredFilesCache = Collections.synchronizedList(new ArrayList<URL>());
4546

47+
/**
48+
* Clears both the files and lazy monitored files caches to prevent classloader leaks on hot redeploy.
49+
*/
50+
public static void clearCache() {
51+
files.clear();
52+
lazyMonitoredFilesCache.clear();
53+
}
54+
55+
/**
56+
* @since 6.9.0
57+
*/
58+
@Override
59+
public void destroy() {
60+
clearCache();
61+
}
62+
4663
protected boolean reloadingConfigs = false;
4764

4865
public DefaultFileManager() {

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ public class Component {
6868
*/
6969
protected static ConcurrentMap<Class<?>, Collection<String>> standardAttributesMap = new ConcurrentHashMap<>();
7070

71+
/**
72+
* Clears the cached standard attributes map to prevent classloader leaks on hot redeploy.
73+
*/
74+
public static void clearStandardAttributesMap() {
75+
standardAttributesMap.clear();
76+
}
77+
7178
protected boolean devMode = false;
7279
protected boolean escapeHtmlBody = false;
7380
protected ValueStack stack;
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.struts2.dispatcher;
20+
21+
import org.apache.struts2.components.Component;
22+
23+
/**
24+
* Clears {@link Component}'s static standard attributes cache to prevent
25+
* classloader leaks on hot redeploy. Wrapper is needed because {@code Component}
26+
* requires constructor arguments that prevent direct container instantiation.
27+
*
28+
* @since 6.9.0
29+
*/
30+
public class ComponentCacheDestroyable implements InternalDestroyable {
31+
32+
@Override
33+
public void destroy() {
34+
Component.clearStandardAttributesMap();
35+
}
36+
}

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

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,27 +21,70 @@
2121
import com.opensymphony.xwork2.inject.Container;
2222

2323
/**
24-
* Simple class to hold Container instance per thread to minimise number of attempts
25-
* to read configuration and build each time a new configuration.
24+
* Per-thread cache for the Container instance, minimising repeated reads from
25+
* {@link com.opensymphony.xwork2.config.ConfigurationManager}.
2626
* <p>
27-
* As ContainerHolder operates just per thread (which means per request) there is no need
28-
* to check if configuration changed during the same request. If changed between requests,
29-
* first call to store Container in ContainerHolder will be with the new configuration.
27+
* WW-5537: Uses a ThreadLocal for per-request isolation with a volatile generation
28+
* counter for cross-thread invalidation during app undeploy. When
29+
* {@link #invalidateAll()} is called, all threads see the updated generation on their
30+
* next {@link #get()} and return {@code null}, forcing a fresh read from
31+
* ConfigurationManager. This prevents classloader leaks caused by idle pool threads
32+
* retaining stale Container references after hot redeployment.
3033
*/
3134
class ContainerHolder {
3235

33-
private static final ThreadLocal<Container> instance = new ThreadLocal<>();
36+
private static final ThreadLocal<CachedContainer> instance = new ThreadLocal<>();
37+
38+
/**
39+
* Incremented on each {@link #invalidateAll()} call. Threads compare their cached
40+
* generation against this value to detect staleness.
41+
*/
42+
private static volatile long generation = 0;
3443

3544
public static void store(Container newInstance) {
36-
instance.set(newInstance);
45+
instance.set(new CachedContainer(newInstance, generation));
3746
}
3847

3948
public static Container get() {
40-
return instance.get();
49+
CachedContainer cached = instance.get();
50+
if (cached == null) {
51+
return null;
52+
}
53+
if (cached.generation != generation) {
54+
instance.remove();
55+
return null;
56+
}
57+
return cached.container;
4158
}
4259

60+
/**
61+
* Clears the current thread's cached container reference.
62+
* Used for per-request cleanup.
63+
*/
4364
public static void clear() {
4465
instance.remove();
4566
}
4667

68+
/**
69+
* Invalidates all threads' cached container references by advancing the generation
70+
* counter. Each thread will detect the stale generation on its next {@link #get()}
71+
* call and clear its own ThreadLocal. Also clears the calling thread immediately.
72+
* <p>
73+
* Used during application undeploy ({@link Dispatcher#cleanup()}) to ensure idle
74+
* pool threads do not pin the webapp classloader via retained Container references.
75+
*/
76+
public static void invalidateAll() {
77+
generation++;
78+
instance.remove();
79+
}
80+
81+
private static class CachedContainer {
82+
final Container container;
83+
final long generation;
84+
85+
CachedContainer(Container container, long generation) {
86+
this.container = container;
87+
this.generation = generation;
88+
}
89+
}
4790
}
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.struts2.dispatcher;
20+
21+
import javax.servlet.ServletContext;
22+
23+
/**
24+
* Extension of {@link InternalDestroyable} for components that require
25+
* {@link ServletContext} during cleanup (e.g. clearing servlet-scoped caches).
26+
*
27+
* <p>During {@link Dispatcher#cleanup()}, the discovery loop checks each
28+
* {@code InternalDestroyable} bean: if it implements this subinterface,
29+
* {@link #destroy(ServletContext)} is called instead of {@link #destroy()}.</p>
30+
*
31+
* @since 6.9.0
32+
* @see InternalDestroyable
33+
* @see Dispatcher#cleanup()
34+
*/
35+
public interface ContextAwareDestroyable extends InternalDestroyable {
36+
37+
/**
38+
* Releases state that requires access to the {@link ServletContext}.
39+
*
40+
* @param servletContext the current servlet context, may be {@code null}
41+
* if the Dispatcher was created without one
42+
*/
43+
void destroy(ServletContext servletContext);
44+
45+
/**
46+
* Default no-op — {@link Dispatcher} calls
47+
* {@link #destroy(ServletContext)} instead when it recognises this type.
48+
*/
49+
@Override
50+
default void destroy() {
51+
// no-op: context-aware variant is the real entry point
52+
}
53+
}

0 commit comments

Comments
 (0)