Lukasz Lenart created WW-5687:
---------------------------------
Summary: Clear the conversion and validator caches on
Dispatcher.cleanup() (WW-5537 defence-in-depth follow-on)
Key: WW-5687
URL: https://issues.apache.org/jira/browse/WW-5687
Project: Struts 2
Issue Type: Improvement
Reporter: Lukasz Lenart
Follow-on from WW-5537 (Closed/Fixed, shipped 6.9.0 and 7.2.1), which built the
{{InternalDestroyable}} cleanup framework. This item was identified during that
work but never filed, and is still not on {{main}} as of 2026-08-23.
h3. What it is, and what it is not
{{StrutsTypeConverterHolder}} (fields {{mappings}}, {{defaultMappings}},
{{unknownMappingsInternal}}) and the {{ActionValidatorManager}} implementations
(fields {{validatorCache}}, {{validatorFileCache}}) hold strong references to
application {{Class}} objects and to converter/validator instances.
These are container-scoped singletons with no static, thread, or otherwise
external references — they are reachable only through the container and are
collected together with it. They therefore do _not_ independently pin the
webapp classloader; that is determined by whatever retains the container, which
WW-5537 already addressed.
_This is not a leak fix._ It is defence-in-depth, consistent with the other
destroyables: clearing these caches during {{Dispatcher.cleanup()}} bounds
retained heap in the window before the container graph is collected, and costs
nothing if it already has been.
h3. The registration trap
The obvious implementation — make the two classes implement
{{InternalDestroyable}} and add a second {{<bean type="...InternalDestroyable"
.../>}} line for each — _does not work_, and this is the reason the item is
worth a ticket rather than a five-line patch.
The Struts container keys factories by (type, name), so declaring the same
class under two bean types produces two independent singletons. Verified
empirically against {{main}}:
{code}FileManager(system) = 1910936570 |
InternalDestroyable(defaultFileManager) = 807322507 | SAME? false
RootAccessor(struts) = 1495608502 | InternalDestroyable(compoundRootAccessor)
= 835773569 | SAME? false
{code}
{{Dispatcher.destroyInternalBeans()}} discovers destroyables with
{{container.getInstanceNames(InternalDestroyable.class)}} and then
{{container.getInstance(InternalDestroyable.class, name)}} — so it would
receive a freshly built holder whose caches are empty, and clear nothing.
The existing dual-registered destroyables are unaffected by this, because what
they clear is static: {{DefaultFileManager.clearCache()}} and
{{CompoundRootAccessor.clearCache()}} are both {{static}} and clear {{static}}
fields, so any instance will do. The caches in this ticket are per-instance,
which is what breaks the pattern.
h3. Suggested implementation
Follow the adapter pattern already used for the other six destroyables
({{ComponentCacheDestroyable}}, {{OgnlCacheDestroyable}},
{{ScopeInterceptorCacheDestroyable}}, {{FreemarkerCacheDestroyable}},
{{DebugUtilsCacheDestroyable}}, {{FinalizableReferenceQueueDestroyable}}): add
small {{InternalDestroyable}} beans that resolve the _real_ singleton from the
container and clear it, rather than making the holders themselves the
destroyable.
Note that {{AnnotationActionValidatorManager extends
DefaultActionValidatorManager}} and is the default binding ({{name="struts"}});
{{DefaultActionValidatorManager}} is bound separately as
{{name="no-annotations"}}. Whatever is done must cover the instance actually in
use, not just the base class.
h3. Verification
A test that asserts the caches are non-empty before {{Dispatcher.cleanup()}}
and empty afterwards — and, importantly, that fails if the destroyable clears a
different instance than the one the framework uses. A naive registration passes
any test that only checks "destroy() was called".
h3. Provenance
The original plan text lived only in unpushed local commit {{8019e6994}}
({{docs/superpowers/plans/2026-03-23-WW-5537-classloader-leak-fixes.md}}, Task
5b), on a branch deleted 2026-08-23. Its Step 3 prescribed the naive dual
registration described above, which the identity probe shows would be a no-op —
that correction is the main thing this ticket adds over the original note.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
