Skip to content

Require Julia 1.12 and fix memory-safety and failure-path problems - #59

Merged
ViralBShah merged 1 commit into
mainfrom
klu-owns-vectors-julia-1.12
Oct 31, 2026
Merged

ViralBShah merged 1 commit into
mainfrom
klu-owns-vectors-julia-1.12

Conversation

@ViralBShah

@ViralBShah ViralBShah commented Oct 31, 2026 •

Copy link
Copy Markdown
Member

Follows a review of the package for structure and soundness. This PR set the version to 0.7.0, but these changes are to be released as 0.6.1, in a follow-up that also stops exporting solve!.

Changes to the accepted element types are in #60, so klu, klu! and ldiv! accept the same types after this PR as before it.

Behaviour changes

  • Julia 1.12 is now required. SparseArrays and LinearAlgebra compat move with it. Test stays at 1.10, because that stdlib is still v1.11.0 on Julia 1.12 and 1.13.
  • Failed factorizations release their factors. After klu! or klu_factor! throws, K \ b, rcond, condest and det throw until a factorization succeeds.
  • Fields are const. Only _symbolic, _numeric, _perms and nzval can still be assigned.
  • An earlier failure no longer blocks a retry. klu_factor! used to rethrow a stale error, even after a temporary lack of memory.
  • solve! with check=false no longer throws when it factors on demand.
  • klu rejects empty matrices with a clear ArgumentError, as lu does.
  • K.symbolic and K.numeric are no longer listed by propertynames(K), because they hold raw pointers into libklu's memory. They can still be accessed.

Memory safety

  • Out-of-bounds reads: a factorization shares vectors with the caller, so dropzeros!(A) or resize! could leave K.nzval too short, and libklu read past its end. Operations that read these vectors now check the lengths first. Sharing itself is unchanged, to avoid using more memory, and the docs now say what the caller must not do.
  • Shared pattern vectors: the pattern check in klu! shifted colptr and rowval in place, which other factorizations sharing them could see while inside libklu.
  • Permutations: klu_analyze! validated the caller's P and Q, then used them, so another task could change them in between. It now validates and uses copies.
  • Pointer assignment: assigning the current _numeric or _symbolic pointer freed it and left it dangling. Replacing _symbolic now also releases the numeric factors that depend on it.

Memory leak

deserialize doesn't call the constructor, so a deserialized factorization had no finalizer and never freed the libklu objects it made on demand. The finalizer is now registered when the first of them is attached.

Peak memory growth over 60 rounds of solving with a 300,000 × 300,000 tridiagonal matrix and running GC:

Before After
Ordinary factorization 246 MiB 246 MiB
Deserialized factorization 2892 MiB 266 MiB

Failure paths and locking

  • Wrong answers after a failure: when klu! stopped partway it left a mix of old and new factors, and a failed klu_factor! kept the earlier factors, so K \ b returned finite wrong answers. rcond and condest could then also make issuccess return true.
  • Unlocked reads: issuccess, nnz and show now read under the lock, and show prints factors from one factorization.
  • klu_sort: its result is now checked.
  • klu_refactor! dropped keyword arguments.

Other changes

  • init_suitesparse is now called from _common rather than __init__, as the stdlib solvers do on entry.
  • Adds logabsdet, and so det and logdet, including for K' and transpose(K).
  • SingularException reports a column with no pivot, and KLU_INVALID has a clearer message.
  • ldiv! accepts a single row with any column stride.
  • The docs for the scale setting now describe rows rather than columns.
  • Removes the AdjointFact and TransposeFact aliases and the unused @isok macro.
  • Adds Serialization as a test dependency.

Testing

On Apple silicon macOS, the test suite passes on Julia 1.12.7 and 1.13.3, and Aqua and the docs build pass on 1.12.7. Linux and Windows are left to CI.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 31, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.29%. Comparing base (146612b) to head (260e88d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
- Coverage   97.31%   97.29%   -0.03%     
==========================================
  Files           1        1              
  Lines         373      407      +34     
==========================================
+ Hits          363      396      +33     
- Misses         10       11       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ViralBShah ViralBShah changed the title Require Julia 1.12, make factorizations own their vectors, and fix failure paths Require Julia 1.12 and fix soundness problems in failure paths and locking Oct 31, 2026
Bump the version to 0.7.0, as several of these change behaviour.

Julia 1.12 no longer sets up SuiteSparse when SparseArrays loads, so call init_suitesparse from
_common, as its own solvers do on entry, rather than from __init__.

Memory safety:
- A factorization shares vectors with the caller, so dropzeros!(A) or resize! could leave K.nzval
  too short, and libklu read past its end. Operations that read these vectors now check the
  lengths first. Sharing stays, to avoid using more memory, and the docs now say what the caller
  must not do.
- The pattern check in klu! shifted colptr and rowval in place, which other factorizations sharing
  them could see while inside libklu.
- klu_analyze! validated the caller's permutations and then used them, so another task could change
  them in between. It now validates and uses copies.
- Assigning the current libklu pointer freed it and left it dangling, and replacing the symbolic
  pointer left numeric factors that depended on it.
- Fields that don't need to change are const, which leaves _symbolic, _numeric, _perms and nzval.

Memory leak:
- deserialize does not call the constructor, so a deserialized factorization had no finalizer, and
  never freed the libklu objects it made on demand. Register the finalizer when the first of them
  is attached instead.

Failure paths:
- When klu! stopped partway it left a mix of old and new factors, and a failed klu_factor! kept the
  earlier factors, so K \ b returned finite wrong answers. Both now release them, so later use
  throws until a factorization succeeds.
- rcond and condest keep the status, as they could make issuccess return true.
- An earlier failure no longer stops klu_factor! from retrying.
- solve! with check=false doesn't throw when it factors on demand.
- Check the result of klu_sort.
- klu rejects empty matrices with a clear error, and KLU_INVALID has a clearer message.
- SingularException reports a column with no pivot.

Locking:
- issuccess, nnz and show read state under the lock, and show prints factors from one factorization.

Also:
- klu_refactor! accepts keywords
- add logabsdet, and so det and logdet, including for K' and transpose(K)
- ldiv! accepts a single row with any column stride
- K.symbolic and K.numeric, which hold raw pointers, are no longer listed as public
- the docs for the scale setting describe rows rather than columns
- remove the AdjointFact and TransposeFact aliases, and the unused `@isok` macro

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ViralBShah
ViralBShah force-pushed the klu-owns-vectors-julia-1.12 branch from 575be64 to 260e88d Compare October 31, 2026 14:58
@ViralBShah ViralBShah changed the title Require Julia 1.12 and fix soundness problems in failure paths and locking Require Julia 1.12 and fix memory-safety and failure-path problems Oct 31, 2026
@ViralBShah
ViralBShah merged commit 7f28afe into main Oct 31, 2026
7 of 8 checks passed
@ViralBShah
ViralBShah deleted the klu-owns-vectors-julia-1.12 branch October 31, 2026 19:15
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.

1 participant