MAINT: Use ENUM_CHECK_NAME for avoiding memory leaks in _superluobject.c
#22071
Conversation
ENUM_CHECK_NAME for avoiding memory leaks in _superluobject.cENUM_CHECK_NAME for avoiding memory leaks in _superluobject.c
There was a problem hiding this comment.
Is there a way we could get some kind of measurable indication that memory leaks have genuinely been fixed here?
On the rebased version of this branch, using memory_profiler with an splu snippet
import numpy as np
from scipy.sparse import csc_array
from scipy.sparse.linalg import splu
for i in range(500000):
A = csc_array([[1,2,0,4], [1,0,0,1], [1,0,2,1], [2,2,1,0.]])
lu = splu(A)
b = np.array([1, 2, 3, 4])
x = lu.solve(b)
A.dot(x)
lu.perm_r
lu.perm_c
On main at time of writing:
They both look stable to me, but I'm also fairly ignorant of the method to reproduce the memory leak here given that we just have a static analyzer report to go off.
There was a problem hiding this comment.
This minor issue was introduced in commit 77e88d7, which predates the introduction of ENUM_CHECK_NAME in commit f414439 almost 2 years later. So that explains why ENUM_CHECK_NAME wasn't used from the start.
This LGTM, simplifies the code and is more correct.
Is there a way we could get some kind of measurable indication that memory leaks have genuinely been fixed here?
It's a tiny leak in an error handling path so you can't hit it in a loop unless you somehow manage to insert a wrong name and then loop over a try-except; it doesn't matter in the real world.
|
Thanks @czgdp1807,@Snape3058 and @tylerjereddy |
Reference issue
Closes #16234
@Snape3058 I think this change increases code safety. It uses already existing macro
ENUM_CHECK_NAMEwhich always doesPy_XDECREF(tmpobj)before returning. Regarding, manually destroyingtmpobj, I think it's not safe. That pattern is also not used elsewhere in SciPy.What does this implement/fix?
Additional information