fix memory leak in cholmod

[stevengj]

Fix bug identified on discourse.

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.38%. Comparing base (896723a) to head (443c147).

Files with missing lines	Patch %	Lines
src/solvers/cholmod.jl	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #733      +/-   ##
==========================================
- Coverage   84.38%   84.38%   -0.01%     
==========================================
  Files          13       13              
  Lines        9372     9369       -3     
==========================================
- Hits         7909     7906       -3     
  Misses       1463     1463              

☔ 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.

[dkarrasch]

Can this be tested? Does this need backporting (v1.13 for sure, but v1.12 & LTS)?

[stevengj]

Presumably the existing tests from #547 (cc @ranocha) cover the code to make sure it works? Not sure how to test for a memory leak?

[bbrehm]

I verified presence of some memory leak via Base.gc_live_bytes(). That might work as a regression test.

I have not looked into the allocation path in SuiteSparse or valgrind. To be honest, I'm somewhat surprised that this allocation gets properly logged and accounted for, at all!

[ranocha]

Thanks a lot for the fix. I am sorry for introducing this bug two years ago.

The basic method should be tested, but I am not sure whether all branches will be tested completely.

This could be a explanation for

The current method still allocates some memory on my system since we do not provide the workspace vectors Handle_Y and Handle_E. We could store them in the CHOLMOD.Factor struct. I tried that, but they were still C_NULL after the call to solve2 (when initialized with C_NULL). Thus, re-using them is not working as I understand the docs. Thus, I have not added this complexity to this PR.

noticed in #547. I guess we should at least open an issue to check whether this works correctly with the fix from this PR, and consider adding this additional optimization in another PR.

[ViralBShah]

cc @rayegun

[stevengj]

Storing the workspace vectors in CHOLMOD.Factor is happening in #727