potential cause for #557: cython missing dictionary deallocation

93 views
Skip to first unread message

mabshoff

unread,
Oct 18, 2007, 12:04:46 AM10/18/07
to sage-devel
Hello,

for every module Cython generates "runtime support code" that is place
at the end of the converted file. Each one of those has an
__Pyx_Import function that gets called from

PyMODINIT_FUNC init$MODULENAME(void)

static PyObject *__Pyx_Import(PyObject *name, PyObject *from_list) {
PyObject *__import__ = 0;
PyObject *empty_list = 0;
PyObject *module = 0;
PyObject *global_dict = 0;
PyObject *empty_dict = 0;
PyObject *list;
__import__ = PyObject_GetAttrString(__pyx_b, "__import__");
if (!__import__)
goto bad;
if (from_list)
list = from_list;
else {
empty_list = PyList_New(0);
if (!empty_list)
goto bad;
list = empty_list;
}
global_dict = PyModule_GetDict(__pyx_m);
if (!global_dict)
goto bad;
empty_dict = PyDict_New();
if (!empty_dict)
goto bad;
module = PyObject_CallFunction(__import__, "OOOO",
name, global_dict, empty_dict, list);
bad:
Py_XDECREF(empty_list);
Py_XDECREF(__import__);
Py_XDECREF(empty_dict);
return module;
}

In that function PyMODINIT_FUNC init$MODULENAME(void) we also call
__Pyx_ImportType quite often:

static PyTypeObject *__Pyx_ImportType(char *module_name, char
*class_name,
long size)
{
PyObject *py_module_name = 0;
PyObject *py_class_name = 0;
PyObject *py_name_list = 0;
PyObject *py_module = 0;
PyObject *result = 0;

py_module_name = PyString_FromString(module_name);
if (!py_module_name)
goto bad;
py_class_name = PyString_FromString(class_name);
if (!py_class_name)
goto bad;
py_name_list = PyList_New(1);
if (!py_name_list)
goto bad;
Py_INCREF(py_class_name);
if (PyList_SetItem(py_name_list, 0, py_class_name) < 0)
goto bad;
py_module = __Pyx_Import(py_module_name, py_name_list);
if (!py_module)
goto bad;
result = PyObject_GetAttr(py_module, py_class_name);
if (!result)
goto bad;
if (!PyType_Check(result)) {
PyErr_Format(PyExc_TypeError,
"%s.%s is not a type object",
module_name, class_name);
goto bad;
}
if (((PyTypeObject *)result)->tp_basicsize != size) {
PyErr_Format(PyExc_ValueError,
"%s.%s does not appear to be the correct type object",
module_name, class_name);
goto bad;
}
goto done;
bad:
Py_XDECREF(result);
result = 0;
done:
Py_XDECREF(py_module_name);
Py_XDECREF(py_class_name);
Py_XDECREF(py_name_list);
return (PyTypeObject *)result;
}

As one can see certain PyObject have refcounts greater 0 when
initialization is successful. But I cannot find a cleanup function
that would decrease the reference count upon exit. Consequently
python's garbage collector never frees those dictionaries, which in
most cases doesn't matter because we are exiting python anyway [some
people claim that one shouldn't care about still reachable memory,
because cleaning it up the nice way just slows down the termination of
a process because the system will reap the heap and stack
automatically]. But it pollutes the memcheck log, which is why I care
about this.

The usual way to call those functions for each module would be to
register an atexit function, see

http://docs.python.org/lib/module-atexit.html

So if anybody want to write am autogenerated cleanup function for
Cython and register it via atexit that person would be very welcome :)

Cheers,
Michael

Robert Bradshaw

unread,
Oct 18, 2007, 12:16:10 AM10/18/07
to sage-...@googlegroups.com
No cleanup function is ever generated, but atexit sounds perfect. I'm
the obvious person to implement this, I'll see if I can find the time
to do so soon.

- Robert

mabshoff

unread,
Oct 18, 2007, 12:20:46 AM10/18/07
to sage-devel

On Oct 18, 6:16 am, Robert Bradshaw <rober...@math.washington.edu>
wrote:

Hi Robert,

> No cleanup function is ever generated, but atexit sounds perfect.

Good that you seem to agree with my analysis. I ran across it be sheer
accident while looking at some linker issue that roed asked me about.

> I'm the obvious person to implement this,

Pretty much 100% what I just claimed in IRC, but cwitty might look at
it during Bug Day 4.

> I'll see if I can find the time to do so soon.
>

Let's hope so.

> - Robert
>

Cheers,

Michael

Robert Bradshaw

unread,
Oct 24, 2007, 3:01:50 PM10/24/07
to sage-...@googlegroups.com
I've implemented a function and hook to do this in the new version of
Cython, but I'm not sure how well it will work in practice on the
entire SAGE library. It will decref local variable and all, but then
if anything (e.g. other destructors) ever try and use any of the
module again it will crash with high probability (because the
expected variables will no longer be set).

- Robert

mabshoff

unread,
Oct 24, 2007, 3:39:15 PM10/24/07
to sage-devel

On Oct 24, 9:01 pm, Robert Bradshaw <rober...@math.washington.edu>
wrote:

Robert,

> I've implemented a function and hook to do this in the new version of
> Cython,

Cool.

> but I'm not sure how well it will work in practice on the
> entire SAGE library. It will decref local variable and all, but then
> if anything (e.g. other destructors) ever try and use any of the
> module again it will crash with high probability (because the
> expected variables will no longer be set).
>

Well, if it crashes upon exit I would consider that a bug and we can
hopefully fix them.

If you have an inofficial spkg I would really like to take it for a
spin.

> - Robert

Cheers,

Michael

<SNIP>

Robert Bradshaw

unread,
Oct 24, 2007, 3:56:51 PM10/24/07
to sage-...@googlegroups.com
On Oct 24, 2007, at 12:39 PM, mabshoff wrote:

> On Oct 24, 9:01 pm, Robert Bradshaw <rober...@math.washington.edu>
> wrote:
>
> Robert,
>
>> I've implemented a function and hook to do this in the new version of
>> Cython,
>
> Cool.
>
>> but I'm not sure how well it will work in practice on the
>> entire SAGE library. It will decref local variable and all, but then
>> if anything (e.g. other destructors) ever try and use any of the
>> module again it will crash with high probability (because the
>> expected variables will no longer be set).
>>
>
> Well, if it crashes upon exit I would consider that a bug and we can
> hopefully fix them.

There are three levels of cleanup--each more aggressive than the
previous, but even the most basic level will potentially render any
of the module-level functions unusable. Hopefully we can find the
minimum set of things to decref so that the python interpreter can
take care of the rest.

> If you have an inofficial spkg I would really like to take it for a
> spin.

I'm hoping to have time to package it all up tonight and make sure
SAGE still builds fine. If not, at the very least I'll post an
unofficial package.

- Robert

Robert Bradshaw

unread,
Oct 25, 2007, 3:06:52 AM10/25/07
to sage-...@googlegroups.com
See http://sage.math.washington.edu/home/robertwb/cython/ All tests
pass with cleanup level 1 (now default). The command line option

--cleanup n

sets the level (0 <= n <= 3). Things crash on quit (in SAGE, though
not with simple examples) with level 3. This may or may not fix #557,
but is a start (but at the very least should make the memcheck logs
somewhat cleaner). We can play with it from here).

- Robert

mabshoff

unread,
Oct 25, 2007, 5:25:30 AM10/25/07
to sage-devel

On Oct 25, 9:06 am, Robert Bradshaw <rober...@math.washington.edu>
wrote:

Hi Robert,

> Seehttp://sage.math.washington.edu/home/robertwb/cython/All tests


> pass with cleanup level 1 (now default). The command line option
>
> --cleanup n
>
> sets the level (0 <= n <= 3). Things crash on quit (in SAGE, though
> not with simple examples) with level 3. This may or may not fix #557,
> but is a start (but at the very least should make the memcheck logs
> somewhat cleaner). We can play with it from here).

Excellent, I will play with this tonight and report back.

mabshoff

unread,
Oct 25, 2007, 1:03:22 PM10/25/07
to sage-devel

On Oct 25, 11:25 am, mabshoff <Michael.Absh...@fsmath.mathematik.uni-

Hmm, with 2.8.9 I get the following error when using

http://sage.math.washington.edu/home/robertwb/cython/cython-0.9.6.8.spkg


Building sage/matrix/misc.c because it depends on sage/matrix/
misc.pyx.
touch sage/matrix/misc.pyx; cython --embed-positions --incref-local-
binop -I/tmp/Work-mabshoff/sage-2.8.9/devel/sage-main -o sage/matrix/
misc.c sage/matrix/misc.pyx

Error converting Pyrex file to C:
------------------------------------------------------------
...
0, 0, 0)
cdef mpz_t* L_row
cdef mod_int* A_row
for i from 0 <= i < A._nrows:
L_row = L._matrix[i]
A_row = A._matrix[i]
^
------------------------------------------------------------

/tmp/Work-mabshoff/sage-2.8.9/devel/sage-main/sage/matrix/misc.pyx:
54:25: Cannot assign type 'sage.matrix.matrix_modn_dense.mod_int *' to
'sage.matrix.misc.mod_int *'
sage: Error running cython.
sage: There was an error installing modified sage library code.

I don't think misc.pyx has changed in a while. Any ideas?

Robert Bradshaw

unread,
Oct 25, 2007, 2:04:27 PM10/25/07
to sage-...@googlegroups.com

Did you apply the accompanying .hg patch to sage-main?

mabshoff

unread,
Oct 25, 2007, 3:14:54 PM10/25/07
to sage-devel

On Oct 25, 8:04 pm, Robert Bradshaw <rober...@math.washington.edu>
wrote:

Hi Robert,

> Did you apply the accompanying .hg patch to sage-main?

nope, but that fixed the problem. It didn't seem obvious, so I ignored
that bundle.

With the old Cython:

==26784== LEAK SUMMARY:
==26784== definitely lost: 0 bytes in 0 blocks.
==26784== possibly lost: 251,078 bytes in 625 blocks.
==26784== still reachable: 36,000,918 bytes in 20,368 blocks.
==26784== suppressed: 0 bytes in 0 blocks.

With the new Cython and n=1 I get:

==31741== LEAK SUMMARY:
==31741== definitely lost: 0 bytes in 0 blocks.
==31741== possibly lost: 253,574 bytes in 633 blocks.
==31741== still reachable: 36,244,988 bytes in 20,359 blocks.
==31741== suppressed: 0 bytes in 0 blocks.

So there has been a slight decrease in the number of blocks that are
still reachable or possibly lost, even thought the amount of memory
increased. I will try with n=2 to see if anything changes. How do I
change the default by the way?

Cheers,

Michael

Robert Bradshaw

unread,
Oct 25, 2007, 3:34:30 PM10/25/07
to sage-...@googlegroups.com
On Oct 25, 2007, at 12:14 PM, mabshoff wrote:

> On Oct 25, 8:04 pm, Robert Bradshaw <rober...@math.washington.edu>
> wrote:
>> On Oct 25, 2007, at 10:03 AM, mabshoff wrote:
>>
>>> On Oct 25, 11:25 am, mabshoff
>>> <Michael.Absh...@fsmath.mathematik.uni-
>>> dortmund.de> wrote:
>>>> On Oct 25, 9:06 am, Robert Bradshaw <rober...@math.washington.edu>

>> Did you apply the accompanying .hg patch to sage-main?
>
> nope, but that fixed the problem. It didn't seem obvious, so I ignored
> that bundle.

There's often a few tweaks that need to be done to the SAGE codebase
to get it to work with the new cython...that one there was fixing a
hack that didn't work anymore.

> With the old Cython:
>
> ==26784== LEAK SUMMARY:
> ==26784== definitely lost: 0 bytes in 0 blocks.
> ==26784== possibly lost: 251,078 bytes in 625 blocks.
> ==26784== still reachable: 36,000,918 bytes in 20,368 blocks.
> ==26784== suppressed: 0 bytes in 0 blocks.
>
> With the new Cython and n=1 I get:
>
> ==31741== LEAK SUMMARY:
> ==31741== definitely lost: 0 bytes in 0 blocks.
> ==31741== possibly lost: 253,574 bytes in 633 blocks.
> ==31741== still reachable: 36,244,988 bytes in 20,359 blocks.
> ==31741== suppressed: 0 bytes in 0 blocks.
>
> So there has been a slight decrease in the number of blocks that are
> still reachable or possibly lost, even thought the amount of memory
> increased.

More stuff is interned with the new cython (e.g. python int constants
are pre-computed rather than constructed/destructed on the fly
throughout the code) as well as other changes. I'm curious how n=0
vs. n=1 behaves, as well as, of course, n=2+.

> I will try with n=2 to see if anything changes. How do I
> change the default by the way?

Try adding "--clean 2" to line 1008 of process_cython_file() in
setup.py.

- Robert

mabshoff

unread,
Oct 25, 2007, 10:46:12 PM10/25/07
to sage-devel

On Oct 25, 9:34 pm, Robert Bradshaw <rober...@math.washington.edu>

Hi Robert,


>
> Try adding "--clean 2" to line 1008 of process_cython_file() in
> setup.py.
>

that doesn't work yet, because the parsing of the command line doesn't
seem to comprehend --clean yet, so I hardcoded the value I wanted for
now.

With n=2 I get the following which might be the cause for the memory
corruption:

==14064== Invalid free() / delete / delete[]
==14064== at 0x4A1B74A: free (vg_replace_malloc.c:320)
==14064== by 0xB98B0D8:
__pyx_f_4sage_5rings_7integer_fast_tp_dealloc (integer.c:13607)
==14064== by 0x4B0022: collect (gcmodule.c:714)
==14064== by 0x4B0373: PyGC_Collect (gcmodule.c:1265)
==14064== by 0x4A612C: Py_Finalize (pythonrun.c:387)
==14064== by 0x4A5C7A: handle_system_exit (pythonrun.c:1616)
==14064== by 0x4A5E78: PyErr_PrintEx (pythonrun.c:1062)
==14064== by 0x4A6686: PyRun_SimpleFileExFlags (pythonrun.c:976)
==14064== by 0x412319: Py_Main (main.c:134)
==14064== by 0x4FD94C9: (below main) (in /lib/libc-2.3.6.so)
==14064== Address 0x73287d8 is 0 bytes inside a block of size 8
free'd
==14064== at 0x4A1B74A: free (vg_replace_malloc.c:320)
==14064== by 0xB98B0D8:
__pyx_f_4sage_5rings_7integer_fast_tp_dealloc (integer.c:13607)
==14064== by 0xB9847E2: cleanup (integer.c:15198)
==14064== by 0x415522: PyObject_Call (abstract.c:1860)
==14064== by 0x481ACA: PyEval_EvalFrameEx (ceval.c:3844)
==14064== by 0x484F3A: PyEval_EvalCodeEx (ceval.c:2831)
==14064== by 0x4CE527: function_call (funcobject.c:517)
==14064== by 0x415522: PyObject_Call (abstract.c:1860)
==14064== by 0x47C850: PyEval_CallObjectWithKeywords (ceval.c:3433)
==14064== by 0x4A60BC: Py_Finalize (pythonrun.c:1589)
==14064== by 0x4A5C7A: handle_system_exit (pythonrun.c:1616)
==14064== by 0x4A5E78: PyErr_PrintEx (pythonrun.c:1062)


> - Robert

Carl Witty was also wondering whether your new Cython.spkg was ready
for 2.8.10 or if we should wait.

Cheers,

Michael

Robert Bradshaw

unread,
Oct 25, 2007, 11:18:20 PM10/25/07
to sage-...@googlegroups.com
On Oct 25, 2007, at 7:46 PM, mabshoff wrote:

> On Oct 25, 9:34 pm, Robert Bradshaw <rober...@math.washington.edu>
> wrote:
> Hi Robert,
>>
>> Try adding "--clean 2" to line 1008 of process_cython_file() in
>> setup.py.
>>
>
> that doesn't work yet, because the parsing of the command line doesn't
> seem to comprehend --clean yet, so I hardcoded the value I wanted for
> now.

Sorry, that should have been --cleanup 2, but hardcoding works too.

This last one might be due to funny stuff we do with the fast alloc/
dealloc stuff. Try compiling just the integer file with --cleanup 1
and see if that fixes it. Perhaps we could incref
__pyx_v_4sage_5rings_7integer_global_dummy_Integer manually so it's
never actually collected. Or it could be something more subtle.

> Carl Witty was also wondering whether your new Cython.spkg was ready
> for 2.8.10 or if we should wait.

I believe so, it compiles all of SAGE and passes all doctests. Stefan
(the other main Cython guy) is having trouble getting his code to
work though.

- Robert


mabshoff

unread,
Oct 26, 2007, 12:35:09 AM10/26/07
to sage-devel

On Oct 26, 5:18 am, Robert Bradshaw <rober...@math.washington.edu>
wrote:

Hello Robert,

>
> This last one might be due to funny stuff we do with the fast alloc/
> dealloc stuff. Try compiling just the integer file with --cleanup 1
> and see if that fixes it.

Yep, for n=2 and n=3 there isn't anything funny going on. Ironically
valgrind pretty much reports the same amount of still reachable
memory:

==17336== LEAK SUMMARY:
==17336== definitely lost: 0 bytes in 0 blocks.
==17336== possibly lost: 253,574 bytes in 633 blocks.
==17336== still reachable: 36,239,900 bytes in 20,342 blocks.
==17336== suppressed: 0 bytes in 0 blocks.


> Perhaps we could incref
> __pyx_v_4sage_5rings_7integer_global_dummy_Integer manually so it's
> never actually collected. Or it could be something more subtle.
>
> > Carl Witty was also wondering whether your new Cython.spkg was ready
> > for 2.8.10 or if we should wait.
>
> I believe so, it compiles all of SAGE and passes all doctests. Stefan
> (the other main Cython guy) is having trouble getting his code to
> work though.
>

ok.

One more thing: I compiled python --pydebug and I get the following
crash related to nteger_fast_tp_dealloc, so it seems that something
very odd is going on there:

----------------------------------------------------------------------
| SAGE Version 2.8.9.alpha0, Release Date: 2007-10-23 |
| Type notebook() for the GUI, and license() for information. |
----------------------------------------------------------------------
sage:
Exiting SAGE (CPU time 0m0.07s, Wall time 0m2.15s).

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1208604992 (LWP 20053)]
0x0078341b in free () from /lib/libc.so.6
(gdb) bt
#0 0x0078341b in free () from /lib/libc.so.6
#1 0x00ea2f38 in __pyx_f_7integer_fast_tp_dealloc
(__pyx_v_o=0x91c046c) at sage/rings/integer.c:15544
#2 0x08083bdf in dict_dealloc (mp=0x91b7334) at Objects/dictobject.c:
847
#3 0x08083bdf in dict_dealloc (mp=0x8cf7474) at Objects/dictobject.c:
847
#4 0x080d9659 in _PyImport_Fini () at Python/import.c:229
#5 0x080e4da4 in Py_Finalize () at Python/pythonrun.c:419
#6 0x080e48ea in handle_system_exit () at Python/pythonrun.c:1616
#7 0x080e4ae5 in PyErr_PrintEx (set_sys_last_vars=1) at Python/
pythonrun.c:1062
#8 0x080e5303 in PyRun_SimpleFileExFlags (fp=0x0,
filename=0xbfa27cb3 "/tmp/Work/sage-2.8.9.rc1/local/bin/sage-gdb-
pythonstartup", closeit=0, flags=0xbfa268e8)
at Python/pythonrun.c:976
#9 0x080571a6 in Py_Main (argc=0, argv=0xbfa269b4) at Modules/main.c:
134
#10 0x08056432 in main (argc=Cannot access memory at address 0x1
) at ./Modules/python.c:23
(gdb) quit

I will try with a "clean build" to see if I can get to the bottom of
this.

Cheers,

Michael

> - Robert

Robert Bradshaw

unread,
Oct 26, 2007, 12:49:46 AM10/26/07
to sage-...@googlegroups.com
On Oct 25, 2007, at 9:35 PM, mabshoff wrote:

> Yep, for n=2 and n=3 there isn't anything funny going on. Ironically
> valgrind pretty much reports the same amount of still reachable
> memory:
>
> ==17336== LEAK SUMMARY:
> ==17336== definitely lost: 0 bytes in 0 blocks.
> ==17336== possibly lost: 253,574 bytes in 633 blocks.
> ==17336== still reachable: 36,239,900 bytes in 20,342 blocks.
> ==17336== suppressed: 0 bytes in 0 blocks.

I was hoping for more than that. Perhaps we could manually decref the
module's dictionary, but that might be dangerous... Perhaps there's
other cleanup to do too.

> One more thing: I compiled python --pydebug and I get the following
> crash related to nteger_fast_tp_dealloc, so it seems that something
> very odd is going on there:

Yes, there are a lot of funny things going on in that class for
speed...in particular, to run in debug mode, you need to uncomment
the PyObject_INIT line of fast_tp_new() in integer.pyx. You might
want to talk to Martin Albrecht.

- Robert

Robert Bradshaw

unread,
Oct 26, 2007, 8:26:29 PM10/26/07
to sage-...@googlegroups.com
On Oct 25, 2007, at 9:35 PM, mabshoff wrote:

>>> Carl Witty was also wondering whether your new Cython.spkg was ready
>>> for 2.8.10 or if we should wait.
>>
>> I believe so, it compiles all of SAGE and passes all doctests. Stefan
>> (the other main Cython guy) is having trouble getting his code to
>> work though.
>>
>
> ok.

BTW, I just uploaded a new spkg with one more tiny fix (has to do
with error reporting).

- Robert

mabshoff

unread,
Oct 28, 2007, 4:29:17 AM10/28/07
to sage-devel

On Oct 27, 1:26 am, Robert Bradshaw <rober...@math.washington.edu>
wrote:


> On Oct 25, 2007, at 9:35 PM, mabshoff wrote:
>

> >>> Carl Witty was also wondering whether your newCython.spkg was ready


> >>> for 2.8.10 or if we should wait.
>
> >> I believe so, it compiles all of SAGE and passes all doctests. Stefan

> >> (the other mainCythonguy) is having trouble getting his code to
> >> work though.
>
> > ok.
>

Hi Robert,

> BTW, I just uploaded a new spkg with one more tiny fix (has to do
> with error reporting).
>

For 2.8.10.alpha1+Craig's fix and python compiled using "--without-
pymalloc" I get with the default cleanup level 1:

==6569== LEAK SUMMARY:
==6569== definitely lost: 181,030 bytes in 2,926 blocks.
==6569== possibly lost: 266,925 bytes in 747 blocks.
==6569== still reachable: 29,184,388 bytes in 179,707 blocks.
==6569== suppressed: 0 bytes in 0 blocks.

With cleanup level 3 in general and cleanup level 1 just for
integer.pyx I get

==23818== LEAK SUMMARY:
==23818== definitely lost: 181,117 bytes in 2,927 blocks.
==23818== possibly lost: 266,838 bytes in 746 blocks.
==23818== still reachable: 29,157,453 bytes in 179,383 blocks.
==23818== suppressed: 0 bytes in 0 blocks.

So my conclusion is that you are on the right way with the cleanup
code. Hopefully once we can compile integer.pyx with cleanup level 3
and not crash the amount of still reachable memory should decrease
tremendously because integer.pyx should just about be referenced in
every extension we have.

> - Robert

Cheers,

Michael

Robert Bradshaw

unread,
Oct 28, 2007, 4:48:30 AM10/28/07
to sage-...@googlegroups.com
On Oct 28, 2007, at 1:29 AM, mabshoff wrote:

> For 2.8.10.alpha1+Craig's fix and python compiled using "--without-
> pymalloc" I get with the default cleanup level 1:
>
> ==6569== LEAK SUMMARY:
> ==6569== definitely lost: 181,030 bytes in 2,926 blocks.
> ==6569== possibly lost: 266,925 bytes in 747 blocks.
> ==6569== still reachable: 29,184,388 bytes in 179,707 blocks.
> ==6569== suppressed: 0 bytes in 0 blocks.
>
> With cleanup level 3 in general and cleanup level 1 just for
> integer.pyx I get
>
> ==23818== LEAK SUMMARY:
> ==23818== definitely lost: 181,117 bytes in 2,927 blocks.
> ==23818== possibly lost: 266,838 bytes in 746 blocks.
> ==23818== still reachable: 29,157,453 bytes in 179,383 blocks.
> ==23818== suppressed: 0 bytes in 0 blocks.
>
> So my conclusion is that you are on the right way with the cleanup
> code.

What is our goal (i.e. what is a clean python run?) It's a bit better
but it seems like we've barely made a dent in it--are the
dictionaries we were looking at decrefing earlier still around? (They
could be holding onto a lot of stuff.)

> Hopefully once we can compile integer.pyx with cleanup level 3
> and not crash

Try the below patch--this will prevent the double-free from the
global dummy integer. There might be a better fix, as this will make
it so that this integer object is never freed.

> the amount of still reachable memory should decrease
> tremendously because integer.pyx should just about be referenced in
> every extension we have.

It doesn't matter how many things reference integer.pyx, only how
many things are referenced by integer.pyx (which isn't too much, but
may include such things as element.pyx).

- Robert


no-collect-integer.patch

mabshoff

unread,
Oct 28, 2007, 5:17:16 AM10/28/07
to sage-devel
On Oct 28, 9:48 am, Robert Bradshaw <rober...@math.washington.edu>
wrote:

> On Oct 28, 2007, at 1:29 AM, mabshoff wrote:
>

Hi Robert,

>
>
> > For 2.8.10.alpha1+Craig's fix and python compiled using "--without-
> > pymalloc" I get with the default cleanup level 1:
>
> > ==6569== LEAK SUMMARY:
> > ==6569== definitely lost: 181,030 bytes in 2,926 blocks.
> > ==6569== possibly lost: 266,925 bytes in 747 blocks.
> > ==6569== still reachable: 29,184,388 bytes in 179,707 blocks.
> > ==6569== suppressed: 0 bytes in 0 blocks.
>
> > With cleanup level 3 in general and cleanup level 1 just for
> > integer.pyx I get
>
> > ==23818== LEAK SUMMARY:
> > ==23818== definitely lost: 181,117 bytes in 2,927 blocks.
> > ==23818== possibly lost: 266,838 bytes in 746 blocks.
> > ==23818== still reachable: 29,157,453 bytes in 179,383 blocks.
> > ==23818== suppressed: 0 bytes in 0 blocks.
>

With the patch valgrind says:

==3482== LEAK SUMMARY:
==3482== definitely lost: 181,235 bytes in 2,929 blocks.
==3482== possibly lost: 266,720 bytes in 744 blocks.
==3482== still reachable: 29,157,648 bytes in 179,395 blocks.
==3482== suppressed: 0 bytes in 0 blocks.

So the number are slightly worst.

> > So my conclusion is that you are on the right way with the cleanup
> > code.
>
> What is our goal (i.e. what is a clean python run?) It's a bit better
> but it seems like we've barely made a dent in it--are the
> dictionaries we were looking at decrefing earlier still around? (They
> could be holding onto a lot of stuff.)
>
> > Hopefully once we can compile integer.pyx with cleanup level 3
> > and not crash
>
> Try the below patch--this will prevent the double-free from the
> global dummy integer. There might be a better fix, as this will make
> it so that this integer object is never freed.

with the patch everything compiles and runs fine with cleanup level 3.

>
> > the amount of still reachable memory should decrease
> > tremendously because integer.pyx should just about be referenced in
> > every extension we have.
>
> It doesn't matter how many things reference integer.pyx, only how
> many things are referenced by integer.pyx (which isn't too much, but
> may include such things as element.pyx).
>

Ok, but I plan to actually play with this using only a couple small
modules independent of Sage in order to understand the problem better.

The only valgrind complaint I get is the following:

==3482== Invalid free() / delete / delete[]
==3482== at 0x4A1B74A: free (vg_replace_malloc.c:320)
==3482== by 0x43FE9A: dict_dealloc (dictobject.c:847)
==3482== by 0x43FE9A: dict_dealloc (dictobject.c:847)
==3482== by 0x499E5B: _PyImport_Fini (import.c:229)
==3482== by 0x4A5D66: Py_Finalize (pythonrun.c:419)
==3482== by 0x4A58AA: handle_system_exit (pythonrun.c:1616)
==3482== by 0x4A5AA8: PyErr_PrintEx (pythonrun.c:1062)
==3482== by 0x4A62B6: PyRun_SimpleFileExFlags (pythonrun.c:976)
==3482== by 0x412319: Py_Main (main.c:134)
==3482== by 0x4FD94C9: (below main) (in /lib/libc-2.3.6.so)
==3482== Address 0xb2de9f8 is 32 bytes inside a block of size 80
alloc'd
==3482== at 0x4A1BB35: malloc (vg_replace_malloc.c:207)
==3482== by 0x4B0079: _PyObject_GC_Malloc (gcmodule.c:1321)
==3482== by 0x454C29: PyType_GenericAlloc (typeobject.c:454)
==3482== by 0x975D160:
__pyx_tp_new_4sage_9structure_7element_Element (element.c:14429)
==3482== by 0xAC89AAA: __pyx_tp_new_4sage_5rings_7integer_Integer
(integer.c:14228)
==3482== by 0x458D92: type_call (typeobject.c:422)
==3482== by 0x415542: PyObject_Call (abstract.c:1860)
==3482== by 0x47C480: PyEval_CallObjectWithKeywords (ceval.c:3433)
==3482== by 0xAC85823: initinteger (integer.c:15100)
==3482== by 0x49E17D: _PyImport_LoadDynamicModule (importdl.c:53)
==3482== by 0x49C08D: import_submodule (import.c:2394)
==3482== by 0x49C550: load_next (import.c:2214)

It might be related. I also get a lot of the following:

==3482== 45 bytes in 1 blocks are definitely lost in loss record 525
of 10,255
==3482== at 0x4A1BB35: malloc (vg_replace_malloc.c:207)
==3482== by 0x44D68B: PyString_FromString (stringobject.c:130)
==3482== by 0x13B4DF15: __Pyx_ImportType (real_double_vector.c:
3786)
==3482== by 0x13B52202: initreal_double_vector
(real_double_vector.c:3255)
==3482== by 0x49E17D: _PyImport_LoadDynamicModule (importdl.c:53)
==3482== by 0x49C08D: import_submodule (import.c:2394)
==3482== by 0x49C550: load_next (import.c:2214)
==3482== by 0x49C7AD: import_module_level (import.c:2002)
==3482== by 0x49CBF4: PyImport_ImportModuleLevel (import.c:2066)
==3482== by 0x47BEE8: builtin___import__ (bltinmodule.c:47)
==3482== by 0x415542: PyObject_Call (abstract.c:1860)
==3482== by 0x47C480: PyEval_CallObjectWithKeywords (ceval.c:3433)

> - Robert
>

Cheers,

Michael

> no-collect-integer.patch
> 1KDownload

Robert Bradshaw

unread,
Oct 28, 2007, 5:33:52 AM10/28/07
to sage-...@googlegroups.com

Hmm... strange. I wonder if the dummy integer was getting decrefed
somewhere else. Also, how deterministic are these numbers?

>>> Hopefully once we can compile integer.pyx with cleanup level 3
>>> and not crash
>>
>> Try the below patch--this will prevent the double-free from the
>> global dummy integer. There might be a better fix, as this will make
>> it so that this integer object is never freed.
>
> with the patch everything compiles and runs fine with cleanup level 3.

Good, though it looks like not all issues are resolved (as seen below).

>>> the amount of still reachable memory should decrease
>>> tremendously because integer.pyx should just about be referenced in
>>> every extension we have.
>>
>> It doesn't matter how many things reference integer.pyx, only how
>> many things are referenced by integer.pyx (which isn't too much, but
>> may include such things as element.pyx).
>>
>
> Ok, but I plan to actually play with this using only a couple small
> modules independent of Sage in order to understand the problem better.

That sound like a good idea.

This looks like it has something to do with the very first integer
created, though I'm not totally sure how to read the log.

> It might be related. I also get a lot of the following:
>
> ==3482== 45 bytes in 1 blocks are definitely lost in loss record 525
> of 10,255
> ==3482== at 0x4A1BB35: malloc (vg_replace_malloc.c:207)
> ==3482== by 0x44D68B: PyString_FromString (stringobject.c:130)
> ==3482== by 0x13B4DF15: __Pyx_ImportType (real_double_vector.c:
> 3786)
> ==3482== by 0x13B52202: initreal_double_vector
> (real_double_vector.c:3255)
> ==3482== by 0x49E17D: _PyImport_LoadDynamicModule (importdl.c:53)
> ==3482== by 0x49C08D: import_submodule (import.c:2394)
> ==3482== by 0x49C550: load_next (import.c:2214)
> ==3482== by 0x49C7AD: import_module_level (import.c:2002)
> ==3482== by 0x49CBF4: PyImport_ImportModuleLevel (import.c:2066)
> ==3482== by 0x47BEE8: builtin___import__ (bltinmodule.c:47)
> ==3482== by 0x415542: PyObject_Call (abstract.c:1860)
> ==3482== by 0x47C480: PyEval_CallObjectWithKeywords (ceval.c:3433)

I see what's going wrong here, try applying the attached patch. This
is probably all over the place.

- Robert

__Pyx_ImportModule-decref-fix.patch

mabshoff

unread,
Oct 28, 2007, 6:26:38 AM10/28/07
to sage-devel

On Oct 28, 10:33 am, Robert Bradshaw <rober...@math.washington.edu>
wrote:


> On Oct 28, 2007, at 2:17 AM, mabshoff wrote:
>

Hi Robert,

The amount of memory leaked can vary a little, but the number of
blocks is usually constant.

>
> >>> Hopefully once we can compile integer.pyx with cleanup level 3
> >>> and not crash
>
> >> Try the below patch--this will prevent the double-free from the
> >> global dummy integer. There might be a better fix, as this will make
> >> it so that this integer object is never freed.
>
> > with the patch everything compiles and runs fine with cleanup level 3.
>
> Good, though it looks like not all issues are resolved (as seen below).
>

Well, it is certainly a step by step process and we are making
progress, so I am happy.

> >>> the amount of still reachable memory should decrease
> >>> tremendously because integer.pyx should just about be referenced in
> >>> every extension we have.
>
> >> It doesn't matter how many things reference integer.pyx, only how
> >> many things are referenced by integer.pyx (which isn't too much, but
> >> may include such things as element.pyx).
>
> > Ok, but I plan to actually play with this using only a couple small
> > modules independent of Sage in order to understand the problem better.
>
> That sound like a good idea.

Yep, all I now need is the time to actually do it.

Yep, the patch fixes that nicely. The valgrind log shrank from 8.2MB
to 5.8MB and we get:

==6724== LEAK SUMMARY:
==6724== definitely lost: 0 bytes in 0 blocks.
==6724== possibly lost: 266,534 bytes in 741 blocks.
==6724== still reachable: 29,157,606 bytes in 179,398 blocks.
==6724== suppressed: 0 bytes in 0 blocks.

So: very nice catch. I will try to run the code with a python using "--
with-pymalloc" and see what happens there.

> - Robert
>

Cheers,

Michael

> __Pyx_ImportModule-decref-fix.patch
> 1KDownload
>
>

mabshoff

unread,
Oct 28, 2007, 6:43:10 AM10/28/07
to sage-devel

On Oct 28, 11:26 am, mabshoff <Michael.Absh...@fsmath.mathematik.uni-

Okay, this doesn't go too well:

----------------------------------------------------------------------
| SAGE Version 2.8.10.alpha1, Release Date: 2007-10-27 |


| Type notebook() for the GUI, and license() for information. |
----------------------------------------------------------------------
sage:

Exiting SAGE (CPU time 0m0.05s, Wall time 0m1.51s).
*** glibc detected *** free(): invalid pointer: 0x00002b21905bd6e0 ***

Program received signal SIGABRT, Aborted.
[Switching to Thread 47423054237536 (LWP 29300)]
0x00002b218a2c607b in raise () from /lib/libc.so.6
(gdb) bt
#0 0x00002b218a2c607b in raise () from /lib/libc.so.6
#1 0x00002b218a2c784e in abort () from /lib/libc.so.6
#2 0x00002b218a2fc6e9 in __libc_message () from /lib/libc.so.6
#3 0x00002b218a303253 in _int_free () from /lib/libc.so.6
#4 0x00002b218a3032de in free () from /lib/libc.so.6
#5 0x000000000043fc2b in dict_dealloc (mp=0x9a1c20) at Objects/
dictobject.c:847
#6 0x000000000043fc2b in dict_dealloc (mp=0x65b160) at Objects/
dictobject.c:847
#7 0x000000000049a22c in _PyImport_Fini () at Python/import.c:229
#8 0x00000000004a6137 in Py_Finalize () at Python/pythonrun.c:419
#9 0x00000000004a5c7b in handle_system_exit () at Python/pythonrun.c:
1616
#10 0x00000000004a5e79 in PyErr_PrintEx (set_sys_last_vars=1) at
Python/pythonrun.c:1062
#11 0x00000000004a6687 in PyRun_SimpleFileExFlags (fp=0x0,
filename=<value optimized out>, closeit=0, flags=0x7fff20dc5d60)
at Python/pythonrun.c:976
#12 0x000000000041231a in Py_Main (argc=<value optimized out>,
argv=<value optimized out>) at Modules/main.c:134
#13 0x00002b218a2b34ca in __libc_start_main () from /lib/libc.so.6
#14 0x000000000041163a in _start () at ../sysdeps/x86_64/elf/start.S:
113

I am recompiling with lower cleanup level and see if it makes a
difference.

Reply all
Reply to author
Forward
0 new messages