Reference to None leaked in module globals.

[robertwb]

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

At 2008-10-15T16:27:12Z @dalcinl added attachment fixnoneleaks.diff

At 2008-10-15T16:56:52Z @dagss added attachment fixnoneleaks_alternative.diff

At 2008-10-15T18:06:46Z @dagss added attachment dagss_attempt2.diff

At 2008-10-16T16:31:14Z @dalcinl added attachment GLOBALS.diff

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

[robertwb]

@dagss commented

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.

[robertwb]

@dagss commented

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

def f():
    print c

f()
cdef object c = 34

[robertwb]

@dagss commented

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.

[robertwb]

@dagss changed resolution to fixed
status from new to closed
commented

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

[robertwb]

@robertwb changed milestone from wishlist to 0.10
commented