Skip to content

GROOVY-12142: Stop pinning container class loaders: remove PIC-Cleane… - #2798

Open
paulk-asert wants to merge 1 commit into
apache:masterfrom
paulk-asert:groovy12142
Open

GROOVY-12142: Stop pinning container class loaders: remove PIC-Cleane…#2798
paulk-asert wants to merge 1 commit into
apache:masterfrom
paulk-asert:groovy12142

Conversation

@paulk-asert

Copy link
Copy Markdown
Contributor

…r and DFA-cache-cleaner threads, restore the ClassValue escape hatch

A Groovy copy deployed per webapp (the common Tomcat/WEB-INF/lib topology) permanently pinned its class loader, growing metaspace on every parallel (re)deployment. Three independent pins:

  1. PIC-Cleaner: linking any indy call site started a never-terminating daemon thread from CacheableCallSite's static initializer, pinning the defining loader and capturing the creating context's protection domains. Stale PIC entries are now swept inline — both callers already hold the lruCache monitor and the cache is bounded (8), so the sweep is trivial; the groovy.indy.callsite.cleaner.inline flag (GROOVY-12092) is gone along with the thread it toggled, which also removes the parked-thread false positive it existed to avoid.

  2. DFA-cache-cleaner: each AtnManager started a never-terminating reference-queue thread whose catch-all swallowed even interrupts. The softly referenced AtnWrapper is a GC canary; its collection is now detected on the parse path when the soft reference reads null, clearing the shared DFA cache at the next parse instead of at GC time — no queue, no thread.

  3. java.lang.ClassValue associations on immortal (bootstrap) classes never release their value's class loader (JDK-8136353), so one dynamic dispatch on e.g. String pinned the loader. The groovy.use.classvalue=false escape hatch (present through 4.x, lost in the GROOVY-11520 collections cleanup) is restored, with the fallback rebuilt on ManagedIdentityConcurrentMap (~40 lines) rather than the removed deprecated collections. Default stays ClassValue.

With 1+2 fixed, a dropped loader is collectable either with the flag or with a shutdown sweep over ClassInfo.getAllClassInfo() calling ClassInfo.remove — both verified by classloader-churn harness against the rebuilt jar; out-of-the-box default config still requires one of the two (the JVM bug is not ours to fix).

…r and DFA-cache-cleaner threads, restore the ClassValue escape hatch

A Groovy copy deployed per webapp (the common Tomcat/WEB-INF/lib
topology) permanently pinned its class loader, growing metaspace on
every parallel (re)deployment. Three independent pins:

1. PIC-Cleaner: linking any indy call site started a never-terminating
   daemon thread from CacheableCallSite's static initializer, pinning
   the defining loader and capturing the creating context's protection
   domains. Stale PIC entries are now swept inline — both callers
   already hold the lruCache monitor and the cache is bounded (8), so
   the sweep is trivial; the groovy.indy.callsite.cleaner.inline flag
   (GROOVY-12092) is gone along with the thread it toggled, which also
   removes the parked-thread false positive it existed to avoid.

2. DFA-cache-cleaner: each AtnManager started a never-terminating
   reference-queue thread whose catch-all swallowed even interrupts.
   The softly referenced AtnWrapper is a GC canary; its collection is
   now detected on the parse path when the soft reference reads null,
   clearing the shared DFA cache at the next parse instead of at GC
   time — no queue, no thread.

3. java.lang.ClassValue associations on immortal (bootstrap) classes
   never release their value's class loader (JDK-8136353), so one
   dynamic dispatch on e.g. String pinned the loader. The
   groovy.use.classvalue=false escape hatch (present through 4.x,
   lost in the GROOVY-11520 collections cleanup) is restored, with the
   fallback rebuilt on ManagedIdentityConcurrentMap (~40 lines) rather
   than the removed deprecated collections. Default stays ClassValue.

With 1+2 fixed, a dropped loader is collectable either with the flag
or with a shutdown sweep over ClassInfo.getAllClassInfo() calling
ClassInfo.remove — both verified by classloader-churn harness against
the rebuilt jar; out-of-the-box default config still requires one of
the two (the JVM bug is not ours to fix).
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.94737% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.1412%. Comparing base (142130d) to head (7442977).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
...aus/groovy/reflection/GroovyClassValueFactory.java 50.0000% 1 Missing and 1 partial ⚠️
.../parser/antlr4/internal/atnmanager/AtnManager.java 85.7143% 0 Missing and 1 partial ⚠️
...codehaus/groovy/vmplugin/v8/CacheableCallSite.java 0.0000% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##               master      #2798        +/-   ##
==================================================
+ Coverage     70.1064%   70.1412%   +0.0348%     
- Complexity      35774      35819        +45     
==================================================
  Files            1561       1563         +2     
  Lines          132373     132390        +17     
  Branches        24332      24353        +21     
==================================================
+ Hits            92802      92860        +58     
+ Misses          31170      31133        -37     
+ Partials         8401       8397         -4     
Files with missing lines Coverage Δ
...us/groovy/reflection/GroovyClassValueMapBased.java 100.0000% <100.0000%> (ø)
.../parser/antlr4/internal/atnmanager/AtnManager.java 71.7949% <85.7143%> (+17.9487%) ⬆️
...codehaus/groovy/vmplugin/v8/CacheableCallSite.java 96.6667% <0.0000%> (+19.1667%) ⬆️
...aus/groovy/reflection/GroovyClassValueFactory.java 40.0000% <50.0000%> (-10.0000%) ⬇️

... and 14 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@testlens-app

testlens-app Bot commented Aug 17, 2026

Copy link
Copy Markdown

✅ All tests passed ✅

🏷️ Commit: 7442977
▶️ Tests: 0 executed
⚪️ Checks: 31/31 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants