[issue24469] Py2.x int free list can grow without bounds

[Roundup Robot]

Roundup Robot added the comment:

New changeset fd0842f34602 by Serhiy Storchaka in branch '2.7':
Issue #24469: Fixed memory leak caused by int subclasses without overridden
https://hg.python.org/cpython/rev/fd0842f34602

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Guido van Rossum]

Guido van Rossum added the comment:

OK, SGTM.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It seems to me, that all builtin and extension types set tp_free to 
PyObject_Del, PyObject_GC_Del, or 0. The int class is the only exception.

int_free() was introduced in 200559fcc664:

> Make sure that tp_free frees the int the same way as tp_dealloc would.
> This fixes the problem that Barry reported on python-dev:
>>>> 23000 .__class__ = bool
> crashes in the deallocator.  This was because int inherited tp_free
> from object, which uses the default allocator.

The above example no longer works:

>>> 23000 .__class__ = bool
Traceback (most recent call last):
  File "", line 1, in 
TypeError: __class__ assignment: only for heap types

I think int_free should be removed and tp_free should be reverted to 0 (as in 
float type).

--
assignee:  -> serhiy.storchaka
keywords: +patch
stage:  -> patch review
Added file: http://bugs.python.org/file45679/int-tp_free-2.7.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Stefan Behnel]

New submission from Stefan Behnel:

A Cython user noticed a memory leak when C-inheriting from int.

http://thread.gmane.org/gmane.comp.python.cython.devel/15689

The Cython code to reproduce this is simply this:

cdef class ExtendedInt(int): pass
 
for j in xrange(1000):
ExtendedInt(j)

The problem is due to the free-list of the int type. It uses this code for 
deallocation:


static void
int_dealloc(PyIntObject *v)
{
if (PyInt_CheckExact(v)) {
Py_TYPE(v) = (struct _typeobject *)free_list;
free_list = v;
}
else
Py_TYPE(v)-tp_free((PyObject *)v);
}

static void
int_free(PyIntObject *v)
{
Py_TYPE(v) = (struct _typeobject *)free_list;
free_list = v;
}


Now, when C-inheriting from PyInt_Type without providing an own tp_free 
implementation, PyType_Ready() will inherit the supertype's tp_free slot, which 
means that int_dealloc() above will end up calling int_free() in all cases, not 
only for the exact-int case. Thus, whether or not it's exactly int or a 
subtype, the object will always be added to the free-list on deallocation.

However, in the subtype case, the free-list is actually ignored by int_new() 
and int_subtype_new(), so that as long as the user code only creates tons of 
int subtype instances and not plain int instances, the freelist will keep 
growing without bounds by pushing dead subtype objects onto it that are never 
reused.

There are two problems here:

1) int subtypes should not be added to the freelist, or at least not unless 
they have exactly the same struct size as PyIntObject (which the ExtendedInt 
type above does but other subtypes may not)

2) if a free-list is used, it should be used in all instantiation cases, not 
just in PyInt_FromLong().

Fixing 1) by adding a type check to int_free() would be enough to fix the 
overall problem. Here's a quickly hacked up change that seems to work for me:


static void
int_free(PyIntObject *v)
{
if (PyInt_CheckExact(v)) {
Py_TYPE(v) = (struct _typeobject *)free_list;
free_list = v;
}
else if (Py_TYPE(v)-tp_flags  Py_TPFLAGS_HAVE_GC)
PyObject_GC_Del(v);  // untested by probably necessary
else
PyObject_Del(v);
}


--
components: Interpreter Core
messages: 245492
nosy: scoder
priority: normal
severity: normal
status: open
title: Py2.x int free list can grow without bounds
type: crash
versions: Python 2.7

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Antoine Pitrou]

Antoine Pitrou added the comment:

Apparently, this is because of 200559fcc664. The fix is weird, though: why 
duplicate code instead of moving it into tp_free?

I'd rather let Guido and Barry discuss this, I'm not willing to touch the 2.x 
int type anymore.

--
nosy: +barry, gvanrossum

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Antoine Pitrou]

Changes by Antoine Pitrou pit...@free.fr:


--
nosy:  -pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Guido van Rossum]

Guido van Rossum added the comment:

1) The intended solution is to require that int subclasses override tp_free.

2) I don't see any constructors that don't call PyInt_FromLong() -- what am I 
missing?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Antoine Pitrou]

Antoine Pitrou added the comment:

Now that's weird. Why is the current int_free() doing the same as 
int_dealloc()? Because some people call tp_free directly?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Stefan Behnel]

Changes by Stefan Behnel sco...@users.sourceforge.net:


--
nosy: +pitrou, serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Stefan Behnel]

Stefan Behnel added the comment:

 1) The intended solution is to require that int subclasses override tp_free.

In the way I showed? By calling either PyObject_GC_Del() or PyObject_Del()
directly? I'll happily do that (Py2.7 and Py2 int being dead ends makes
that pretty future proof), but it's neither obvious nor very clean.

 2) I don't see any constructors that don't call PyInt_FromLong() -- what am I 
 missing?

Not sure what you mean. int_new() immediately calls int_subtype_new(),
which then calls type-tp_alloc(). No call to PyInt_FromLong() involved.


static PyObject *
int_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
PyObject *x = NULL;
int base = -909;
static char *kwlist[] = {x, base, 0};

if (type != PyInt_Type)
return int_subtype_new(type, args, kwds); /* Wimp out */
...

static PyObject *
int_subtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
...
tmp = int_new(PyInt_Type, args, kwds);
if (tmp == NULL)
return NULL;
if (!PyInt_Check(tmp)) {
ival = PyLong_AsLong(tmp);
if (ival == -1  PyErr_Occurred()) {
Py_DECREF(tmp);
return NULL;
}
} else {
ival = ((PyIntObject *)tmp)-ob_ival;
}

newobj = type-tp_alloc(type, 0);
if (newobj == NULL) {
Py_DECREF(tmp);
return NULL;
}
((PyIntObject *)newobj)-ob_ival = ival;
Py_DECREF(tmp);
return newobj;
}


--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24469] Py2.x int free list can grow without bounds

[Guido van Rossum]

Guido van Rossum added the comment:

(1) Just look at the examples of other builtin types with a similar structure 
but no free_list.

(2) I guess the intention is for classes that subclass int to also override 
tp_alloc.

Note that much of this code is written pretty much assuming that subclasses are 
created using class statements (in a regular Python module, not Cython) which 
take care of all these details. That doesn't mean Cython is wrong to try this, 
but it does mean there isn't a lot of documentation, and it also means I don't 
think the thing you reported qualifies as a bug in CPython.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24469
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com