[
https://issues.apache.org/jira/browse/WW-5687?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Lukasz Lenart updated WW-5687:
------------------------------
Fix Version/s: 7.4.0
> 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
> Priority: Minor
> Fix For: 7.4.0
>
>
> 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)
