Fix Sub::Quote weak reference cleanup#759
Closed
fglock wants to merge 3 commits into
Closed
Conversation
- Modified WeakRefRegistry.clearWeakRefsTo() to always clear weak refs to CODE objects - Added includeCode parameter to allow conditional CODE ref clearing - Added clearUnreachableCodeWeakRefs() method to clean up unreachable CODE refs - Added clear_unreachable_code_weak_refs() function to Scalar::Util - Modified ScalarUtil.isweak() and weaken() to call clearUnreachableCodeWeakRefs() for CODE refs This fixes leaks.t tests which were failing because weak refs to CODE objects were never cleared, preventing Sub::Quote::CLONE from cleaning up expired entries. Note: hints.t still has 2 failing tests due to a known limitation with caller(0) returning empty warning bits in main script context (documented in lexical-warnings.md).
The OOM error in sub-defer.t and sub-quote.t is a pre-existing issue that occurs even without any changes to WeakRefRegistry or ScalarUtil. This fix only changes clearWeakRefsTo to always include CODE refs, which fixes leaks.t without exacerbating the OOM issue.
Documents the changes made in PR #759 to fix Sub::Quote leaks.t tests, the investigation into the OOM error in sub-defer.t and sub-quote.t, and 6 potential system redesign options to resolve the OOM issue. Recommended approach is to bundle PerlOnJava-tuned versions of Sub::Defer/Sub::Quote as the short-term solution.
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.
Fix Sub::Quote leaks.t tests by clearing weak refs to CODE objects
Problem
The Sub::Quote leaks.t tests were failing because weak references to CODE objects were never cleared, preventing Sub::Quote::CLONE from cleaning up expired entries.
Solution
Modified WeakRefRegistry.clearWeakRefsTo() to always clear weak refs to CODE objects by passing
trueinstead offalseto the includeCode parameter. This allows Sub::Quote::CLONE to properly clean up expired entries.Test Results
OOM Investigation
The OOM error in sub-defer.t and sub-quote.t was investigated extensively:
Root cause identified:
The OOM occurs in
ScalarRefRegistry.forceGcAndSnapshot()at line 130, which is called byReachabilityWalker.sweepWeakRefs()at line 1120 during cleanup. The forceGcAndSnapshot method runs 3 passes of GC (each calling System.gc() 5 times with 10ms sleep) to determine which objects are still reachable. With the many CODE refs and weak refs created by Sub::Defer/Sub::Quote, this GC forcing mechanism runs out of memory even with 4GB heap.The Sub::Defer/Sub::Quote tests create many CODE refs and weak refs, and during cleanup, the reachability walker's GC forcing mechanism causes OOM. This is a fundamental part of the selective refcounting system and not easily fixable without redesigning that system.
Potential fixes (not implemented in this PR):
Files Changed