Copilot commented on code in PR #2868:
URL: https://github.com/apache/groovy/pull/2868#discussion_r3930197170
##########
subprojects/groovy-servlet/src/test/groovy/groovy/servlet/TemplateServletTest.groovy:
##########
@@ -69,6 +69,34 @@ class TemplateServletTest {
assert responseData.status == null
}
+ @Test
+ void test_compiled_template_is_cached_across_garbage_collection() {
+ def templateFile = new File(temporaryFolder, 'cached.gsp').tap { write
'hello' }
+ def url = templateFile.toURI().toURL()
+ servlet.init(mockServletConfigForUrlResource(url))
+
+ def first = servlet.getTemplate(url)
+ forceGarbageCollection()
+ def second = servlet.getTemplate(url)
+
+ assert second.is(first)
+ }
Review Comment:
This test asserts template identity across a forced GC (`second.is(first)`),
but the implementation now caches via `SoftReference<TemplateCacheEntry>` and
the TemplateServlet Javadoc explicitly says callers must not rely on being
given the same instance twice. Since soft references may be cleared at any GC
(policy/JVM dependent), this assertion can be flaky. Consider asserting the
intended regression instead: that the cache key survives GC (i.e., the cache is
not a WeakHashMap of weak keys), by reflectively reading the private `cache`
and checking it still contains `url.toString()` after
`forceGarbageCollection()`.
##########
subprojects/groovy-servlet/src/main/java/groovy/servlet/TemplateServlet.java:
##########
@@ -165,9 +166,11 @@ public String toString() {
}
/**
- * Simple file name to template cache map.
+ * File name to template cache, safe for concurrent use. Entries are held
softly, so a
+ * template survives ordinary collection but can be reclaimed under memory
pressure and
+ * compiled again on the next request.
*/
- private final Map<String, TemplateCacheEntry> cache;
+ private final Map<String, SoftReference<TemplateCacheEntry>> cache;
Review Comment:
PR description states templates are retained for the servlet's lifetime once
the cache is moved to ConcurrentHashMap, but the code now stores
`SoftReference<TemplateCacheEntry>` values (and the Javadoc documents reclaim
under memory pressure). Please reconcile the PR description with the
implemented behavior (either update the description, or switch back to strong
cache values if lifetime retention is the goal).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
