Memory leak related to return in try/finally

[robertwb]

tryfinally.pyx has a genuine leak reported by the refcount nanny:

dagss@boksen:~/cython/devel5$ python runtests.py --refnanny 
...
    RefnannyException: References leaked: 
      Acquired on lines: 390

At 2009-02-20T00:50:29Z @dagss added attachment 200.diff

Migrated from http://trac.cython.org/ticket/200

[robertwb]

@dagss changed priority from major to critical
commented

[robertwb]

@dagss changed description from

tryfinally.pyx has a genuine leak reported by the refcount nanny:

dagss@boksen:~/cython/devel5$ python runtests.py --refnanny --no-cpp --no-cleanup -vv tryfinally
Running tests against Cython 0.11.beta
Python 2.5.2 (r252:60911, Apr 21 2008, 11:17:30) 
[4.2.3 (Ubuntu 4.2.3-2ubuntu7)](GCC)

compiling (c) tryfinally ... ok
compiling (c) and running tryfinally ... tryfinally.c: In function ‘__pyx_pf_10tryfinally_try_return_temp’:
tryfinally.c:438: warning: ‘__pyx_r’ may be used uninitialized in this function
Doctest: tryfinally ... FAIL

======================================================================
FAIL: Doctest: tryfinally
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.5/doctest.py", line 2128, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for tryfinally
  File "/home/dagss/cython/devel5/BUILD/run/c/tryfinally.so", line 135, in tryfinally

----------------------------------------------------------------------
File "/home/dagss/cython/devel5/BUILD/run/c/tryfinally.so", line 154, in tryfinally
Failed example:
    try_return_cy()
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python2.5/doctest.py", line 1228, in __run
        compileflags, 1) in test.globs
      File "<doctest tryfinally[4]>", line 1, in <module>
        try_return_cy()
      File "tryfinally.pyx", line 40, in tryfinally.try_return_cy (tryfinally.c:419)
      File "refnanny.pyx", line 90, in refnanny.__Pyx_Refnanny_FinishContext (refnanny.c:1941)
      File "refnanny.pyx", line 49, in refnanny.RefnannyContext.end (refnanny.c:1441)
    RefnannyException: References leaked: 
      Acquired on lines: 390

to

tryfinally.pyx has a genuine leak reported by the refcount nanny:

dagss@boksen:~/cython/devel5$ python runtests.py --refnanny 
...
    RefnannyException: References leaked: 
      Acquired on lines: 390

commented

This bug has apparently been in Cython for some time. With code like

try:
  return x
finally:
  return y

then x is leaked, because ReturnStatNode assumes that __pyx_r is empty.

The fix is of course easy; assign __pyx_r to None initially and decref it on assignments. However, this adds an additional decref to every single function with object return values.

This is another thing that longer-term would be solved by control flow analysis.

[robertwb]

scoder commented

Yes, starting off with None was my first thought, too. And I find it ok as a quick fix for 0.11. Correct code first.

[robertwb]

@dagss changed owner to dagss
commented

Fix attached, this can be closed when I get to push it or someone else applies it.

[robertwb]

@dagss changed owner from dagss to robertwb
commented

[robertwb]

scoder changed resolution to fixed
status from new to closed
commented

I assume that this can be fixed now that the patch is in.