Repository navigation
Don't free KLU factors in the finalizer pass at exit - #46
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #46 +/- ##
==========================================
+ Coverage 59.02% 59.15% +0.13%
==========================================
Files 2 2
Lines 593 595 +2
==========================================
+ Hits 350 352 +2
Misses 243 243 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ViralBShah
force-pushed
the
skip-free-at-exit
branch
from
October 30, 2026 19:30
8aab246 to
2a2ec09
Compare
At exit Julia runs all finalizers, including those of objects still in use by tasks on other threads, so a finalizer could free the Numeric and Symbolic objects out from under a running klu_solve. Skip the frees once the process is exiting, and leave that memory to the OS. Fixes #31 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ViralBShah
force-pushed
the
skip-free-at-exit
branch
from
October 30, 2026 19:59
2a2ec09 to
ffa8788
Compare
The subprocess's output is the only evidence of what crashed, and this test exists to catch a crash that may be specific to a platform. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
Small review fix in 3a171d1: the exit test keeps the subprocess stderr, so a platform-specific crash leaves evidence in CI, and the comment is shorter. Also checked on Linux, which the description lists as untested: the #31 scenario crashed 7/10 runs on |
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 #31. Refs #38.
The bug
At exit,
jl_atexit_hookrunsBase._atexitand then runs all finalizers, including those of objects that are still reachable, while tasks on other threads may still be running. TheKLUFactorizationfinalizer then frees theNumericandSymbolicobjects under an in-flightklu_solve, which segfaults.The reproducer in #31 makes no copies. Each task calls
klu(A)itself, and the script ends without waiting for its tasks. That explains the reporter's two observations: both@syncand changing the finalizer make the crash go away. A trace of the finalizer logged frees of up to 7 objects still in use after the main task finished.The fix
An
atexithook sets_EXITING, and the finalizer returns early once it is set. Hooks run before the finalizer pass. The memory is left to the OS, and the process is exiting anyway. PowerNetworkMatrices.jl made the same change in its own wrapper in 8b8fb959.Results
Locally on macOS arm64 with Julia 1.13:
main-t 8-t 4The new test runs the scenario in a subprocess, because the crash happens during process exit. The full suite passes at
-t 4.Windows and Linux were not tested locally. The crash log in #31 has the same
in expression starting at none:0signature as these runs.Overlap with #42
#42 also changes the finalizer, so whichever merges second needs a small conflict resolution. #42 stops this crash too, but this fix keeps libklu calls running in parallel.
🤖 Generated with Claude Code