#13896: Fix cython's gc_track and gc_untrack
------------------------------------------------------------------+---------
Reporter: nbruin |
Owner: rlm
Type: defect |
Status: new
Priority: blocker |
Milestone: sage-5.6
Component: memleak |
Resolution:
Keywords: | Work
issues:
Report Upstream: Reported upstream. Developers acknowledge bug. |
Reviewers:
Authors: | Merged
in:
Dependencies: |
Stopgaps:
------------------------------------------------------------------+---------
Old description:
> In a long sage-devel thread we eventually found
> [https://groups.google.com/group/sage-devel/msg/1d05a46b9b5fa8e2?hl=en in
> this message] that a GC during a weakref callback on a Cython class can
> lead to double deallocation of that class. In Python's
> [http://svn.python.org/projects/python/trunk/Include/object.h
> Objects/typeobject.h], line 1024 and onwards, there are some comments
> that indicate that earlier version of Python were bitten by this problem
> too. The solution is to insert the appropriate `PyObject_GC_Untrack` and
> `PyObject_GC_Track` in cython's deallocation code. This is best fixed in
> cython itself.
New description:
In a long sage-devel thread we eventually found
[https://groups.google.com/group/sage-devel/msg/1d05a46b9b5fa8e2?hl=en in
this message] that a GC during a weakref callback on a Cython class can
lead to double deallocation of that class. In Python's
[http://svn.python.org/projects/python/trunk/Objects/typeobject.c
Objects/typeobject.c], line 1024 and onwards, there are some comments
that indicate that earlier version of Python were bitten by this problem
too. The solution is to insert the appropriate `PyObject_GC_Untrack` and
`PyObject_GC_Track` in cython's deallocation code. This is best fixed in
cython itself.
--
Comment (by nbruin):
Apologies. I saw I linked to the wrong file.
[http://svn.python.org/projects/python/trunk/Include/object.h
Include/object.h] also has some interesting information, but it looks like
it is a bit out-of-date on some bits. In particular, if you look at the
actual use of the TRASHCAN macros:
{{{
PyObject_GC_UnTrack(self);
++_PyTrash_delete_nesting;
Py_TRASHCAN_SAFE_BEGIN(self);
--_PyTrash_delete_nesting;
...
endlabel:
++_PyTrash_delete_nesting;
Py_TRASHCAN_SAFE_END(self);
--_PyTrash_delete_nesting;
}}}
with the explanation a little lower:
{{{
Q. Why the bizarre (net-zero) manipulation of
_PyTrash_delete_nesting around the trashcan macros?
A. Some base classes (e.g. list) also use the trashcan mechanism.
The following scenario used to be possible:
- suppose the trashcan level is one below the trashcan limit
- subtype_dealloc() is called
- the trashcan limit is not yet reached, so the trashcan level
is incremented and the code between trashcan begin and end is
executed
- this destroys much of the object's contents, including its
slots and __dict__
- basedealloc() is called; this is really list_dealloc(), or
some other type which also uses the trashcan macros
- the trashcan limit is now reached, so the object is put on the
trashcan's to-be-deleted-later list
- basedealloc() returns
- subtype_dealloc() decrefs the object's type
- subtype_dealloc() returns
- later, the trashcan code starts deleting the objects from its
to-be-deleted-later list
- subtype_dealloc() is called *AGAIN* for the same object
- at the very least (if the destroyed slots and __dict__ don't
cause problems) the object's type gets decref'ed a second
time, which is *BAD*!!!
The remedy is to make sure that if the code between trashcan
begin and end in subtype_dealloc() is called, the code between
trashcan begin and end in basedealloc() will also be called.
This is done by decrementing the level after passing into the
trashcan block, and incrementing it just before leaving the
block.
But now it's possible that a chain of objects consisting solely
of objects whose deallocator is subtype_dealloc() will defeat
the trashcan mechanism completely: the decremented level means
that the effective level never reaches the limit.
Therefore, we
*increment* the level *before* entering the trashcan block, and
matchingly decrement it after leaving. This means the trashcan
code will trigger a little early, but that's no big deal.
}}}
It's probably better to leave out the trashcan for now. It seems like
rather tricky code and I'm not sure it's part of the official Python C-API
(it might be something internal, just like they use some macros themselves
they find unsafe for use in extension modules)
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13896#comment:6>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
