Ticket #90 (closed defect: fixed)

Opened 3 months ago

Last modified 2 months ago

Reference to None leaked in module globals.

Reported by: robertwb Assigned to:
Priority: major Milestone: 0.10
Component: Memory Leak Version:
Keywords: Cc:

Description 

On Oct 1, 2008, at 11:13 AM, Lisandro Dalcin wrote:

Consider the following code inside a one-line pyx file:

cdef object someint = 7

Then Cython generates the following inside the module init function:

  /*--- Global init code ---*/
  __pyx_v_9refleaks2_someint = Py_None; Py_INCREF(Py_None);

  /* "/u/dalcinl/Devel/Cython/sandbox/refleaks2.pyx":1
 * cdef object someint = 7             # <<<<<<<<<<<<<<
 *
 */
  Py_INCREF(__pyx_int_7);
  __pyx_v_9refleaks2_someint = __pyx_int_7;


Clearly, Py_None references are being leaked. All this is because of
bad interaction between this two methods:

* ModuleNode.generate_global_init_code(...) (in ModuleNode.py)
* FinalOptimizePhase.visit_SingleAssignmentNode(...) (in Optimize.py)

We should fix this with FinalOptimizePhase.visit_SingleAssignmentNode

Attachments

fixnoneleaks.diff (0.8 kB) - added by dalcinl on 10/15/2008 09:27:12 AM.
A possible fix, waiting for Dag's review
fixnoneleaks_alternative.diff (1.2 kB) - added by dagss on 10/15/2008 09:56:52 AM.
dagss_attempt2.diff (0.9 kB) - added by dagss on 10/15/2008 11:06:46 AM.
GLOBALS.diff (2.4 kB) - added by dalcinl on 10/16/2008 09:31:14 AM.

Change History

10/15/2008 09:27:12 AM changed by dalcinl

  • attachment fixnoneleaks.diff added.

A possible fix, waiting for Dag's review

10/15/2008 09:54:24 AM changed by dagss

The patch should work -- however it seems a bit fragile.

I'm attaching an alternative version, which rather modifies how the initalization happens. This should also be very slightly faster.

10/15/2008 09:56:52 AM changed by dagss

  • attachment fixnoneleaks_alternative.diff added.

10/15/2008 10:49:59 AM changed by dagss

Ideally this testcase should be added, which makes my patch crash: 

def f():
    print c

f()
cdef object c = 34

10/15/2008 11:06:46 AM changed by dagss

  • attachment dagss_attempt2.diff added.

10/15/2008 11:10:31 AM changed by dagss

OK, my last attempt is up as a new patch on the ticket:

<http://trac.cython.org/cython_trac/ticket/90>

It works in this way: Assignment nodes which are created from cdef statements in the module scope doesn't get the "first" attribute set to True (as it is not guaranteed that the cdef statement is indeed the first assignment, indeed they are implicitly always set to None first).

I now realize what made my worry about your patch: It works ok, but it corrects something which was set wrongly in the first place. This should fix the root cause instead.

10/16/2008 09:31:14 AM changed by dalcinl

  • attachment GLOBALS.diff added.

10/17/2008 01:16:10 AM changed by dagss

  • status changed from new to closed.
  • resolution set to fixed.

By Lisandro Dalcin: http://hg.cython.org/cython-devel/rev/b23f31007194

11/08/2008 02:40:59 PM changed by robertwb

  • milestone changed from wishlist to 0.10.