Repository navigation
Conversation
With struts.freemarker.mru.max.strong.size=0 (default) the FreeMarker template cache stayed on the default SoftCacheStorage, which has no entry limit. Its key includes the locale, which for LocaleProvider actions derives from the request, so distinct request locales grew it without a count bound. Use a SoftCacheStorage that is emptied once it holds 10000 entries. It stays concurrent, so lookups remain lock-free (unlike MruCacheStorage, which TemplateCache synchronizes on). The strong:N path is unchanged.
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
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.
Fixes WW-5832
With
struts.freemarker.mru.max.strong.size=0(the default)FreemarkerManagerleaves FreeMarker's template cache on its defaultSoftCacheStorage, which has no entry limit. The cache key includes the locale. ForLocaleProvideractions (e.g.ActionSupport)FreemarkerResult.deduceLocale()returns the action's locale, which by default comes fromrequest.getLocale()and is not validated unlessstruts.locale.validateRequestLocaleis enabled (defaultfalse). Distinct request locales therefore add template cache entries without a count bound. Same shape as WW-5668 (text caches) and WW-5716 (Tiles locale caches).The cost is retention and re-parsing, not an out-of-memory error: entries are soft references and are cleared under heap pressure, which also evicts the templates legitimate requests need, and every locale re-reads and re-parses the template.
Change
mruMaxStrongSizeis0, install aSoftCacheStoragesubclass that is emptied once it holds 10000 entries (the same cap asstruts.i18n.cacheMaxSize). Entries stay soft references, and an evicted template is just loaded again on next use. The bound is approximate under concurrency.soft:10000: that createsMruCacheStorage(0, 10000), which is not aConcurrentCacheStorage, soTemplateCachewould take a lock on every lookup; the defaultSoftCacheStorageavoids that (see WW-3766).mruMaxStrongSize > 0->strong:N) is unchanged.10000 entries is templates x locales: core ships 131 theme templates (at most 45 in one theme), so an application would need on the order of a hundred locales across a few hundred templates to reach it.
How verified
FreemarkerManagerTest#testTemplateCacheIsBoundedByDefaultloads one template under 10100 distinct locales and asserts the cache stays within the limit and the storage is still concurrent. With the bounded-storage branch removed it fails on the size assertion; with the change it passes.FreemarkerManagerTest(8),FreeMarkerResultTest(6) andI18nInterceptorTest(28) pass, and the fullcoresuite runs 3626 tests with 0 failures and 2 skipped.Also checked against a running Struts Boot app with an
ActionSupportaction rendering a FreeMarker template and 12,000 requests with 12,000 distinctAccept-Languageheaders: onmainthe template cache grew from 1 to 12,001 entries (DefaultSoftCacheStorage); with this change it is aBoundedSoftCacheStorage, emptied at 10000 and holding 2,002 entries after the same run. All 12,000 responses were correct and the server logged no errors.Not changed
struts.locale.validateRequestLocaleand its default,AcceptLanguageLocaleHandler, and thedefault.propertiescomment are untouched.LocaleProvider) useconfiguration.getLocale()indeduceLocale(); I did not test them.