WW-5537 Resolve classloader/memory leaks during Tomcat hot deployment#1632
Open
lukaszlenart wants to merge 17 commits intomainfrom
Open
WW-5537 Resolve classloader/memory leaks during Tomcat hot deployment#1632lukaszlenart wants to merge 17 commits intomainfrom
lukaszlenart wants to merge 17 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…er null Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…estroyable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…alDestroyable discovery Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e pattern matching Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lukaszlenart
force-pushed
the
WW-5537-classloader-leak-fixes
branch
from
b56ea78 to
9097c07
Compare
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Container now exposes a destroy() method that clears factories, injectors, constructors, and ThreadLocals. This releases Class<?> keys and JDK DelegatingClassLoader instances that pin the webapp classloader. DefaultConfiguration.destroy() calls container.destroy() and reloadContainer() delegates to destroy() to avoid duplication. Also fixes JSONCacheDestroyable referencing non-existent DefaultJSONWriter (renamed to StrutsJSONWriter). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lukaszlenart
force-pushed
the
WW-5537-classloader-leak-fixes
branch
from
20b8476 to
1296e4f
Compare
…om reloadContainer factories must remain intact because existing code holds direct references to the Container after destroyConfiguration() and expects it to still resolve dependencies (e.g. during configuration reload). reloadContainer() reverted to clearing packageContexts/loadedFileNames directly — calling destroy() there nulled the container reference and cleared state needed during the bootstrap transition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The test failures were caused by factories.clear() in Container.destroy(), not by calling destroy() from reloadContainer(). Now that factories.clear() is removed, destroy() is safe to call here — it clears packageContexts, loadedFileNames, and the container's reflection caches in one place. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lukaszlenart
requested review from
aleksandr-m,
cnenning,
jogep,
kusalk,
rgielen,
sdutry and
yasserzamani
…y method comments - Replace volatile field with AtomicReference in FinalizableReferenceQueue for proper thread safety using getAndSet() - Add comments to empty destroy() implementations in test mocks - Replace deprecated new URL() with URI.toURL() in DispatcherCleanupTest - Add comments to empty listener methods in DispatcherCleanupTest Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Port of #1631 (Struts 6) to Struts 7, with Java 17 improvements.
Fixes classloader/memory leaks that cause
OutOfMemoryError(Metaspace) during Tomcat hot redeployment (WW-5537):InternalDestroyable/ContextAwareDestroyableinterfaces with container-based discovery so static caches, daemon threads, and shared references are cleaned up whenDispatcher.cleanup()runsFinalizableReferenceQueuedaemon thread with properjoin()and classloader cleanupContainerHolderplainThreadLocalwith generation-counter pattern (AtomicLong) for safe cross-thread invalidation during undeployJava 17 improvements over Struts 6 (Java 8) version
volatile longwith non-atomic++AtomicLong(thread-safe)recordinstanceofChanges
InternalDestroyable/ContextAwareDestroyableinterfacesContainerHolderThreadLocal → ThreadLocal + AtomicLong generation counterDispatcher.cleanup()refactored into focused destroy methods with container discoveryFinalizableReferenceQueue: volatile instance, join(5000), setContextClassLoader(null)ScopeInterceptor.clearLocks(): synchronized block for thread safetyPrepareOperations: per-requestContainerHolder.clear()JSONCacheDestroyableregistered instruts-plugin.xmllog4j-webfor proper Log4j2 lifecycle in servlet containerTest plan
DispatcherCleanupTest— 8 tests verifying InternalDestroyable discovery and all static caches cleared afterdispatcher.cleanup()ContainerHolderTest— 4 tests verifying store/get, clear, invalidateAll cross-thread and same-thread🤖 Generated with Claude Code