Fix per-proof memory leak: clear JavaParserFacade cache on dispose#3854
Merged
Conversation
Disposed proofs were never collected: live heap grew ~56.6 MB per load/prove/dispose cycle of the same obligation in one JVM (linearly), which is why RunAllProofs forks a JVM per group. Root cause is javaparser's JavaParserFacade.instances, a static WeakHashMap<TypeSolver, JavaParserFacade> whose value (new JavaParserFacade(typeSolver)) strongly references its own weak key. A weak key that stays strongly reachable from its own value never becomes weakly reachable, so the entry is never evicted. KeY's JavaParserFactory type solvers transitively reference the owning Services, so each disposed proof's whole proof tree was retained through this cache. A proof registers several short-lived type-solver keys (the DynamicTypeSolver wrapper plus every CombinedTypeSolver from rebuild()), and the fork exposes no per-key removal, so JavaParserFactory.dispose() drops the cache wholesale via the public clearInstances(), synchronized on the JavaParserFacade monitor that get() uses (the backing map is a plain WeakHashMap and clearInstances() is itself unsynchronized). The facade is a load-time-only resolution cache, rebuilt lazily, so this is safe. Proof.dispose() invokes it via the new nullable accessor Services.getJavaServiceOrNull() (avoids the assert in getJavaService() for pure first-order proofs). Measured with MtRetentionProbe (8 reps, symmArray): +56.6 MB/proof before, +0.1 MB/proof after.
8bc530c to
2bf95ab
Compare
Test-only diagnostics for the dispose leak, all gated behind -Dkey.mt.retention=true so they never run in normal CI: - MtRetentionProbe: loads/proves/disposes an obligation N times, forces GC each cycle and reports live MB per rep, a verdict, and (with .histo) the top live-instance growth. Serves as a regression guard for the dispose leak. - HeapRootFinder: MAT-free, in-process "path to GC roots". Enumerates loaded classes via the gcClassHistogram diagnostic command, BFS-walks the object graph from their static fields and from every thread's ThreadLocalMap, excludes weak/soft referents, and prints the shortest strong reference chain to a target object. This is how the JavaParserFacade.instances root was identified after JFR/OldObjectSample only reached "VM Global / Global Object Handle". build.gradle forwards -Dkey.mt.retention* and -PstressHeap to the test JVM, and adds --add-opens for java.base packages when -Dkey.mt.retention.root=true (the root finder reflects into JDK collection internals).
2bf95ab to
e15580c
Compare
wadoon
approved these changes
Jun 21, 2026
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.
Intended Change
This bug was found thanks to stress testing the MT PRs on 128 core :-)
Repeatedly loading, proving and disposing the same obligation in a single JVM leaked roughly a
whole proof tree per cycle — about +56.6 MB per proof, growing linearly — so a disposed
Proofwas never garbage-collected. This is long-standing (it is why
RunAllProofsforks a JVM per group)and is not multithreading-specific.
Root cause
javaparser's
JavaParserFacadekeeps a process-global cache:Each value (
new JavaParserFacade(typeSolver)) strongly references its own weak key. A weakkey that stays strongly reachable from its own value can never become weakly reachable, so the
entry is never evicted. KeY's
JavaParserFactorytype solvers (DynamicTypeSolver,LogicalTypeSolver) are non-static inner classes that transitively reference the owningServices,so every disposed proof's entire proof tree was pinned through this cache.
The exact GC-root chain (found with the in-process root finder added here):
Fix
A single proof registers several short-lived type-solver keys (the
DynamicTypeSolverwrapperplus every
CombinedTypeSolverproduced byrebuild()), and the fork exposes no per-key removal, soJavaParserFactory.dispose()drops the cache wholesale via the publicJavaParserFacade.clearInstances(). It synchronizes on theJavaParserFacadeclass monitor thatget()uses, because the backing map is a plainWeakHashMapandclearInstances()is itselfunsynchronized. The facade is a pure resolution cache that is only populated while parsing Java
during loading and is rebuilt lazily on the next
get, so clearing it on dispose is safe.Proof.dispose()invokes this via the new nullable accessorServices.getJavaServiceOrNull(), whichavoids the assertion in
getJavaService()for pure first-order proofs.Result
Measured with
MtRetentionProbe(8 load/prove/dispose cycles ofsymmArray,-Xmx2g):All eight proofs still load, prove and close after the fix, confirming that clearing the cache
between proofs does not break symbol resolution.
Type of pull request
Ensuring quality
verdict: flat (dispose releases per-proof state)(+0.1 MB/proof); before the fix the samecommand reports
RETENTION(+56.6 MB/proof).Additional information and contact(s)
This change is orthogonal to the multithreading PRs (those merely mitigate the same leak for the
stress battery by forking one JVM per proof); this is the underlying fix.
Relationship to the JavaParser fork fix (wadoon/key-javaparser#7)
The true root cause is in our JavaParser fork and is addressed there by
wadoon/key-javaparser#7, which holds the
JavaParserFacade.instancescache values weakly so theself-cycle can no longer pin a
TypeSolver. The two fixes are complementary:org.key-project.proofjavaartifact and forcesimmediate release on
dispose(). It is the fix we can ship today.consumer (release happens lazily, a GC cycle or two after a proof is dropped).
What to do once wadoon/key-javaparser#7 is released and the
JP_VERSIONdependency is bumped:this
dispose()-timeclearInstances()becomes belt-and-suspenders and can be relaxed. Concretely:Bump
JP_VERSIONin the build to the fork release containing <deleted> #7 and confirm the leak stays flat(re-run
MtRetentionProbe; it should remainflatwithout theclearInstances()call).Then either:
JavaParserFactory.dispose()and its call inProof.dispose()entirely (rely on thefork's weak values), or
dispose()but switch it from the globalclearInstances()to a targeted eviction ofjust this factory's type-solver entries — gentler for the IDE / multi-proof case, where clearing
the whole shared cache on every proof disposal is wasteful.
The global
clearInstances()is used here only because the published fork exposes no per-keyremoval and a proof registers several short-lived type-solver keys; <deleted> #7 removes the need for it.
Services.getJavaServiceOrNull()can stay regardless (it is a generally useful nullable accessor).Two opt-in, test-only diagnostics are included as a regression guard and for future leak hunts, both
gated behind
-Dkey.mt.retention=trueso they never run in normal CI:MtRetentionProbe— quantifies live-heap growth across disposed proofs.HeapRootFinder— a MAT-free, in-process "path to GC roots" (enumerates loaded classes via thegcClassHistogramdiagnostic command, BFS from static fields and every thread'sThreadLocalMap,excluding weak/soft referents). JFR's
OldObjectSampleonly reached "VM Global / Global ObjectHandle", which is why this was needed.
The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.