#11339: Refcounting for Cython rings
-----------------------------------------------+----------------------------
Reporter: gagern | Owner: drkirkby
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-4.7.2
Component: algebra | Keywords: sd31
Work_issues: Rebase on Sage 4.7.2.alpha2. | Upstream: N/A
Reviewer: François Bissey, Steven Trogdon | Author: Volker Braun,
Martin von Gagern
Merged: | Dependencies:
-----------------------------------------------+----------------------------
Changes (by vbraun):
* status: needs_work => needs_review
Old description:
> As [https://github.com/cschwan/sage-on-
> gentoo/issues/51#issuecomment-1181259 described for the Sage on Gentoo
> project], there is a problem in how parts of sage use `__deallocate__` in
> their Cython code. I've actually traced an instance of
> [http://hg.sagemath.org/sage-
> main/file/361a4ad7d52c/sage/libs/singular/groebner_strategy.pyx#l134
> groebner_strategy.pyx] causing segmentation faults under Python 2.7.
>
> What happens is that the garbage collector invokes the
> [http://docs.python.org/c-api/typeobj.html#tp_clear tp_clear] function
> for the object. Its implementation is provided by Cython, and one of its
> effects is that every python reference will be set to `None`. A bit later
> on, [http://docs.python.org/c-api/typeobj.html#tp_dealloc tp_dealloc] is
> called and invokes the `__deallocate__` method. By that time, `_parent`
> is `None`, so accessing `_parent._ring` is a bad idea, and in this case
> it turns out to be null.
>
> Others have had similar problems before. There are
> [http://stackoverflow.com/questions/4501938/how-can-c-object-lifetimes-
> be-correctly-managed-in-cython crude workarounds] floating around. I
> guess a proper solution would be twaking cython to allow custom code in
> the tp_clear function. In other words, have a "magic" mehod `__clear__`
> similar to the magic `__deallocate__`. But I'll wait for comments here
> first, before taking this to cython upstream. Perhaps people with more
> experience have better solutions to offer. And I won't mind if you decide
> to take this to Cython devs yourselves.
>
> Apply:
> * [attachment:trac_11339_refcount_singular_rings.patch]
> * [attachment:trac_11339_refcount_singular_polynomials.patch]
New description:
= Ref counting for Singular rings =
Python makes no guarantees that destructors are called if circular
references are involved. This patch implements an extra Python proxy layer
that correctly refcounts Singular rings. In contrast to our earlier code,
it will release the singular rings once they are no longer in use. This
triggers various issues in Sage/libSingular that are dealt with in the
follow-up ticket #10903
= Historic discussion =
As [https://github.com/cschwan/sage-on-
gentoo/issues/51#issuecomment-1181259 described for the Sage on Gentoo
project], there is a problem in how parts of sage use `__deallocate__` in
their Cython code. I've actually traced an instance of
[http://hg.sagemath.org/sage-
main/file/361a4ad7d52c/sage/libs/singular/groebner_strategy.pyx#l134
groebner_strategy.pyx] causing segmentation faults under Python 2.7.
What happens is that the garbage collector invokes the
[http://docs.python.org/c-api/typeobj.html#tp_clear tp_clear] function for
the object. Its implementation is provided by Cython, and one of its
effects is that every python reference will be set to `None`. A bit later
on, [http://docs.python.org/c-api/typeobj.html#tp_dealloc tp_dealloc] is
called and invokes the `__deallocate__` method. By that time, `_parent` is
`None`, so accessing `_parent._ring` is a bad idea, and in this case it
turns out to be null.
Others have had similar problems before. There are
[http://stackoverflow.com/questions/4501938/how-can-c-object-lifetimes-be-
correctly-managed-in-cython crude workarounds] floating around. I guess a
proper solution would be twaking cython to allow custom code in the
tp_clear function. In other words, have a "magic" mehod `__clear__`
similar to the magic `__deallocate__`. But I'll wait for comments here
first, before taking this to cython upstream. Perhaps people with more
experience have better solutions to offer. And I won't mind if you decide
to take this to Cython devs yourselves.
= Apply =
Apply:
* [attachment:trac_11339_refcount_singular_rings.patch]
* [attachment:trac_11339_refcount_singular_polynomials.patch]
--
Comment:
I am setting this bug to Needs Review. The two patches here will correctly
refcount the Singular rings, and the reviewer should focus on that aspect.
They also make Sage crash because of issues in the libSingular interface.
The latter will be dealt with in #10903. Please report any crashes in Sage
on #10903, and not here.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11339#comment:75>
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.
