Request: New lint to catch potential memory leaks

[bgianfo]

Recently I found a bug in a cython code base which caused a memory leak. The issue was the author had written a __cdel__ instead of __del__ or __dealloc__. I think it would be valuable to have a rule that we could potentially opt into, which would allow us to find __cinit__ methods without a matching __dealloc__ implementation. I took a pass and didn't see anything like this documented in the current rule set.

[MarcoGorelli]

Thanks for idea!

Could you spell out more explicitly what the rule should be please? As in, could you please show

• a minimal code example which cython-lint should flag
• a minimal code example which is similar in nature but which this tool should not flag

Then I can add it

Thanks!

[bgianfo]

Sure, essentially I'm asking for the cython special function rules to be encoded in some linter rules. https://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#finalization-methods-dealloc-and-del

Here is some some code that correctly allocates and deallocates that memory when Example is garbage collected and destructed.

cdef cppclass DummyAllocation:
    DummyAllocation() except +

cdef class Example:
    cdef DummyAllocation* _allocation

    def __cinit__(self, *args, **kwargs):
        self._allocation = new DummyAllocation()

    def __dealloc__(self):
        del self._allocation

Here is the code where we would leak memory, where __dealloc__ is just missing, or as I mentioned originally maybe it's just named the wrong thing :smile::

cdef cppclass DummyAllocation:
    DummyAllocation() except +

cdef class Example:

    cdef DummyAllocation* _allocation

    def __cinit__(self, *args, **kwargs):
        self._allocation = new DummyAllocation()

Note that it is perfectly ok to have a __cinit__ method and not allocate anything, so this is a kind of awkward rule to enable by default, since unless you can actually detect an allocation happening (new / malloc / opening of a file descriptor...) there is no way to know if you actually NEED a __dealloc__, etc...

[bgianfo]

@MarcoGorelli do you think this would be reasonable to implement?

[MarcoGorelli]

happy to review a PR if you put one up