Commits (12)
......@@ -207,6 +207,7 @@ cdef extern from "singular/Singular/libsingular.h":
n_Procs_s* cf # coefficient field/ring
int ref
int sage_ref
# return total degree of p
......
......@@ -24,6 +24,8 @@ cdef singular_ring* access_singular_ring(r) except <singular_ring*> -1
cdef class RingWrap:
cdef singular_ring *_ring
cdef int *_ring_ref
cdef class Resolution:
cdef syStrategy *_resolution
......
......@@ -102,6 +102,7 @@ from sage.libs.singular.decl cimport *
from sage.libs.singular.option import opt_ctx
from sage.libs.singular.polynomial cimport singular_vector_maximal_component, singular_polynomial_check
from sage.libs.singular.singular cimport sa2si, si2sa, si2sa_intvec
from sage.libs.singular.ring cimport singular_ring_delete, singular_ring_reference
from sage.libs.singular.singular import error_messages
......@@ -112,6 +113,8 @@ from sage.misc.misc import get_verbose
from sage.structure.sequence import Sequence, Sequence_generic
from sage.rings.polynomial.multi_polynomial_sequence import PolynomialSequence, PolynomialSequence_generic
from cysignals.memory cimport sig_calloc
cdef poly* sage_vector_to_poly(v, ring *r) except <poly*> -1:
"""
......@@ -156,8 +159,7 @@ cdef class RingWrap:
return "<RingWrap>"
def __dealloc__(self):
if self._ring!=NULL:
self._ring.ref -= 1
singular_ring_delete(self._ring, self._ring_ref)
def ngens(self):
"""
......@@ -828,7 +830,6 @@ cdef class Converter(SageObject):
Append the ring ``r`` to the list.
"""
cdef ring *_r = access_singular_ring(r)
_r.ref+=1
return self._append(<void *>_r, RING_CMD)
cdef leftv *append_matrix(self, mat) except NULL:
......@@ -1060,9 +1061,8 @@ cdef class LibraryCallHandler(BaseCallHandler):
cdef leftv* handle_call(self, Converter argument_list, ring *_ring=NULL):
if _ring != currRing: rChangeCurrRing(_ring)
cdef bint error = iiMake_proc(self.proc_idhdl, NULL, argument_list.args)
cdef leftv * res
if not error:
if not iiMake_proc(self.proc_idhdl, NULL, argument_list.args):
res = <leftv*> omAllocBin(sleftv_bin)
res.Init()
res.Copy(&iiRETURNEXPR)
......@@ -1191,8 +1191,13 @@ cdef class SingularFunction(SageObject):
currRingHdl = ggetid("my_awesome_sage_ring")
if currRingHdl == NULL:
currRingHdl = enterid("my_awesome_sage_ring", 0, RING_CMD, &IDROOT, 1)
currRingHdl.data.uring = <ring *>omAlloc0Bin(sip_sring_bin)
currRingHdl.data.uring.ref += 1
if currRing:
currRingHdl.data.uring = currRing
else:
currRingHdl.data.uring = <ring *>omAlloc0Bin(sip_sring_bin)
# deletion would result in a segfault, thus, we
# prevent it from deletion:
currRingHdl.data.uring.ref += 1
cdef BaseCallHandler get_call_handler(self):
"""
......@@ -1488,7 +1493,6 @@ cdef inline call_function(SingularFunction self, tuple args, object R, bint sign
global myynest
global error_messages
cdef ring *si_ring
if isinstance(R, MPolynomialRing_libsingular):
si_ring = (<MPolynomialRing_libsingular>R)._ring
......@@ -1498,9 +1502,7 @@ cdef inline call_function(SingularFunction self, tuple args, object R, bint sign
if si_ring != currRing: rChangeCurrRing(si_ring)
if currRingHdl.data.uring!= currRing:
currRingHdl.data.uring.ref -= 1
currRingHdl.data.uring = currRing # ref counting?
currRingHdl.data.uring.ref += 1
currRingHdl.data.uring = currRing
cdef Converter argument_list = Converter(args, R, attributes)
......@@ -1883,9 +1885,9 @@ def list_of_functions(packages=False):
cdef inline RingWrap new_RingWrap(ring* r):
cdef RingWrap ring_wrap_result = RingWrap.__new__(RingWrap)
ring_wrap_result._ring = r
ring_wrap_result._ring.ref += 1
ring_wrap_result._ring_ref = <int*>sig_calloc(1, sizeof(int))
ring_wrap_result._ring = singular_ring_reference(r, ring_wrap_result._ring_ref)
ring_wrap_result._ring.ref = 1 # to prevent Singular from deleting it prematurely
return ring_wrap_result
......
......@@ -8,6 +8,7 @@ from sage.rings.polynomial.plural cimport NCPolynomialRing_plural, NCPolynomial_
cdef class GroebnerStrategy(SageObject):
cdef skStrategy *_strat
cdef ring *_parent_ring
cdef int *_parent_ring_ref
cdef MPolynomialRing_libsingular _parent
cdef object _ideal
......@@ -15,6 +16,8 @@ cdef class GroebnerStrategy(SageObject):
cdef class NCGroebnerStrategy(SageObject):
cdef skStrategy *_strat
cdef ring *_parent_ring
cdef int *_parent_ring_ref
cdef NCPolynomialRing_plural _parent
cdef object _ideal
......
......@@ -110,7 +110,8 @@ cdef class GroebnerStrategy(SageObject):
cdef MPolynomialRing_libsingular R = <MPolynomialRing_libsingular>L.ring()
self._parent = R
self._parent_ring = singular_ring_reference(R._ring)
self._parent_ring_ref = R._ring_ref
self._parent_ring = singular_ring_reference(R._ring, self._parent_ring_ref)
if not R.term_order().is_global():
raise NotImplementedError("The local case is not implemented yet.")
......@@ -178,8 +179,7 @@ cdef class GroebnerStrategy(SageObject):
rChangeCurrRing(oldRing)
else:
delete_skStrategy(self._strat)
if self._parent_ring:
singular_ring_delete(self._parent_ring)
singular_ring_delete(self._parent_ring, self._parent_ring_ref)
def _repr_(self):
"""
......@@ -352,6 +352,9 @@ cdef class NCGroebnerStrategy(SageObject):
cdef NCPolynomialRing_plural R = <NCPolynomialRing_plural>L.ring()
self._parent = R
assert R._ring_ref, "Underlying ring for the Groebner strategy needs to have a refcount"
self._parent_ring_ref = R._ring_ref
self._parent_ring = singular_ring_reference(R._ring, self._parent_ring_ref)
if not R.term_order().is_global():
raise NotImplementedError("The local case is not implemented yet.")
......@@ -394,6 +397,8 @@ cdef class NCGroebnerStrategy(SageObject):
sage: strat = NCGroebnerStrategy(I)
sage: del strat # indirect doctest
"""
# WARNING: the Cython class self._parent is no longer accessible!
# see http://trac.sagemath.org/sage_trac/ticket/11339
cdef ring *oldRing = NULL
if self._strat:
omfree(self._strat.sevS)
......@@ -407,13 +412,14 @@ cdef class NCGroebnerStrategy(SageObject):
omfree(self._strat.fromQ)
id_Delete(&self._strat.Shdl, self._parent._ring)
if self._parent._ring != currRing:
if self._parent_ring != currRing:
oldRing = currRing
rChangeCurrRing(self._parent._ring)
rChangeCurrRing(self._parent_ring)
delete_skStrategy(self._strat)
rChangeCurrRing(oldRing)
else:
delete_skStrategy(self._strat)
singular_ring_delete(self._parent_ring, self._parent_ring_ref)
def _repr_(self):
"""
......
......@@ -19,7 +19,7 @@ from sage.libs.singular.decl cimport poly, ring
cdef int singular_polynomial_check(poly *p, ring *r) except -1
cdef int singular_polynomial_add (poly **ret, poly *p, poly *q, ring *r)
cdef int singular_polynomial_call (poly **ret, poly *p, ring *r, list args, poly *(*get_element)(object))
cdef int singular_polynomial_cmp (poly *p, poly *q, ring *r)
cdef int singular_polynomial_cmp (poly *p, poly *q, ring *r) except -2
cdef int singular_polynomial_rmul (poly **ret, poly *p, RingElement q, ring *r)
cdef int singular_polynomial_mul (poly **ret, poly *p, poly *q, ring *r) except -1
cdef int singular_polynomial_sub (poly **ret, poly *p, poly *q, ring *r)
......
......@@ -224,7 +224,7 @@ cdef int singular_polynomial_call(poly **ret, poly *p, ring *r, list args, poly
return 0
cdef int singular_polynomial_cmp(poly *p, poly *q, ring *r):
cdef int singular_polynomial_cmp(poly *p, poly *q, ring *r) except -2:
"""
Compare two Singular elements ``p`` and ``q`` in ``r``.
......@@ -250,6 +250,8 @@ cdef int singular_polynomial_cmp(poly *p, poly *q, ring *r):
"""
cdef number *h
cdef int ret = 0
assert r, "A valid ring must be provided"
assert r.ref >= 0, "This ring has %d references and thus has previously been deleted"%(r.ref)
if r != currRing:
rChangeCurrRing(r)
......
......@@ -17,21 +17,27 @@ from sage.libs.singular.decl cimport ring
# To work with singular rings, you need to balance singular_ring_new with
# singular_ring_delete or singular_ring_reference with
# singular_ring_delete. That is, either use one of the two patterns:
# singular_ring_delete; the latter will, when appropriate, also deallocate
# the <int*> that is being used for the reference count.
# That is, either use one of the two patterns:
#
# cdef class myclass_new():
# cdef ring* myring;
# cdef ring *myring;
# cdef int *ref
# cdef __cinit__():
# self.myring = singular_ring_new(...)
# self.ref = <int*>sig_malloc(sizeof(int))
# self.myring = singular_ring_reference(singular_ring_new(...), self.ref)
# cdef __dealloc__():
# singular_ring_delete(self.myring)
# singular_ring_delete(self.myring, self.ref)
#
# cdef class myclass_reference():
# cdef ring* refring;
# cdef __cinit__(ring* some_ring):
# self.refring = singular_ring_reference(some_ring)
# cdef ring *refring
# cdef int *ref
# cdef __cinit__(ring *some_ring, int *refcount_for_some_ring):
# self.ref = refcount_for_some_ring
# self.refring = singular_ring_reference(some_ring, self.ref)
# cdef __dealloc__():
# singular_ring_delete(self.refring)
# singular_ring_delete(self.refring, self.ref)
#
# You must not refer to Python/Cython classes in the Cython
# destructor, the following is INVALID:
......@@ -49,10 +55,8 @@ from sage.libs.singular.decl cimport ring
cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL
# reference an existing ring
cdef ring *singular_ring_reference(ring *existing_ring) except NULL
cdef ring *singular_ring_reference(ring *existing_ring, int *refcount) except NULL
# carefully delete a ring once its refcount is zero
cdef void singular_ring_delete(ring *doomed)
# Used internally for reference counting
cdef wrap_ring(ring* R)
# carefully delete a ring once its refcount is zero, in that case
# also deallocating refcount
cdef int singular_ring_delete(ring *doomed, int *refcount) except 1
......@@ -6,6 +6,8 @@ AUTHORS:
- Martin Albrecht (2009-07): initial implementation
- Kwankyu Lee (2010-06): added matrix term order support
- Simon King (2019-01): more efficient refcounting for libsingular rings
"""
#*****************************************************************************
# Copyright (C) 2009 Martin Albrecht <malb@informatik.uni-bremen.de>
......@@ -15,6 +17,7 @@ AUTHORS:
#*****************************************************************************
from sage.cpython.string cimport str_to_bytes
from cysignals.memory cimport sig_free
from sage.libs.gmp.types cimport __mpz_struct
from sage.libs.gmp.mpz cimport mpz_init_set_ui, mpz_init_set
......@@ -372,15 +375,8 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
if (_ring is NULL):
raise ValueError("Failed to allocate Singular ring.")
_ring.ShortOut = 0
rChangeCurrRing(_ring)
wrapped_ring = wrap_ring(_ring)
if wrapped_ring in ring_refcount_dict:
raise ValueError('newly created ring already in dictionary??')
ring_refcount_dict[wrapped_ring] = 1
rComplete(_ring, 1)
_ring.ShortOut = 0
......@@ -392,150 +388,39 @@ cdef ring *singular_ring_new(base_ring, n, names, term_order) except NULL:
return _ring
#############################################################################
ring_refcount_dict = defaultdict(int)
cdef class ring_wrapper_Py(object):
r"""
Python object wrapping the ring pointer.
This is useful to store ring pointers in Python containers.
cdef int ring_reference_count = 0
You must not construct instances of this class yourself, use
:func:`wrap_ring` instead.
EXAMPLES::
sage: from sage.libs.singular.ring import ring_wrapper_Py
sage: ring_wrapper_Py
<type 'sage.libs.singular.ring.ring_wrapper_Py'>
def total_ring_reference_count():
"""
Return the total number of all references to libsingular rings.
cdef ring* _ring
def __cinit__(self):
"""
The Cython constructor.
EXAMPLES::
sage: from sage.libs.singular.ring import ring_wrapper_Py
sage: t = ring_wrapper_Py(); t
The ring pointer 0x0
These are just wrappers around a pointer, so it isn't really meaningful
to pickle them::
sage: TestSuite(t).run(skip='_test_pickling')
"""
self._ring = NULL
def __hash__(self):
"""
Return a hash value so that instances can be used as dictionary keys.
OUTPUT:
Integer.
EXAMPLES::
sage: from sage.libs.singular.ring import ring_wrapper_Py
sage: t = ring_wrapper_Py()
sage: t.__hash__()
0
"""
return <long>(self._ring)
def __repr__(self):
"""
Return a string representation.
OUTPUT:
String.
EXAMPLES::
sage: from sage.libs.singular.ring import ring_wrapper_Py
sage: t = ring_wrapper_Py()
sage: t
The ring pointer 0x0
sage: t.__repr__()
'The ring pointer 0x0'
"""
return 'The ring pointer '+hex(self.__hash__())
# This could be written using __eq__ but that does not work
# due to https://github.com/cython/cython/issues/2019
def __richcmp__(ring_wrapper_Py self, other, int op):
"""
Equality comparison between two ``ring_wrapper_Py`` instances,
for use when hashing.
INPUT:
This is for testing and debugging only.
- ``right`` -- a :class:`ring_wrapper_Py`
OUTPUT:
True if both ``ring_wrapper_Py`` wrap the same pointer.
EXAMPLES::
sage: from sage.libs.singular.ring import (ring_wrapper_Py,
....: currRing_wrapper)
sage: t = ring_wrapper_Py()
sage: t == t
True
sage: P.<x,y,z> = QQ[]
sage: t2 = currRing_wrapper()
sage: t3 = currRing_wrapper()
sage: t == t2
False
sage: t2 == t3
True
sage: t2 != t3
False
sage: t2 == None
False
"""
if not (op == Py_EQ or op == Py_NE):
return NotImplemented
if type(other) is not ring_wrapper_Py:
return op != Py_EQ
r = <ring_wrapper_Py>other
return (self._ring == r._ring) == (op == Py_EQ)
EXAMPLES::
sage: from sage.libs.singular.ring import total_ring_reference_count
sage: R.<x,y,z> = QQ[]
sage: n = total_ring_reference_count()
sage: t = x+y^2-z
sage: total_ring_reference_count() - n
1
sage: del t
sage: total_ring_reference_count() - n
0
cdef wrap_ring(ring* R):
"""
Wrap a C ring pointer into a Python object.
return ring_reference_count
INPUT:
- ``R`` -- a singular ring (a C datastructure).
OUTPUT:
A Python object :class:`ring_wrapper_Py` wrapping the C pointer.
"""
cdef ring_wrapper_Py W = ring_wrapper_Py()
W._ring = R
return W
cdef ring *singular_ring_reference(ring *existing_ring) except NULL:
cdef ring *singular_ring_reference(ring *existing_ring, int *refcount) except NULL:
"""
Refcount the ring ``existing_ring``.
INPUT:
- ``existing_ring`` -- a Singular ring.
- ``refcount`` -- pointer to an int, for reference count
OUTPUT:
......@@ -551,40 +436,42 @@ cdef ring *singular_ring_reference(ring *existing_ring) except NULL:
sage: _ = gc.collect()
sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
sage: from sage.libs.singular.groebner_strategy import GroebnerStrategy
sage: from sage.libs.singular.ring import ring_refcount_dict
sage: n = len(ring_refcount_dict)
sage: prev_rings = set(ring_refcount_dict)
sage: P = MPolynomialRing_libsingular(GF(541), 2, ('x', 'y'), TermOrder('degrevlex', 2))
sage: ring_ptr = set(ring_refcount_dict).difference(prev_rings).pop()
sage: ring_ptr # random output
The ring pointer 0x7f78a646b8d0
sage: ring_refcount_dict[ring_ptr]
sage: from sage.libs.singular.ring import total_ring_reference_count
sage: n = total_ring_reference_count()
sage: P = PolynomialRing(GF(541), names=('x', 'y'), order = TermOrder('degrevlex', 2))
sage: total_ring_reference_count() - n
3
sage: strat = GroebnerStrategy(Ideal([P.gen(0) + P.gen(1)]))
sage: ring_refcount_dict[ring_ptr]
sage: total_ring_reference_count() - n
6
sage: del strat
sage: _ = gc.collect()
sage: ring_refcount_dict[ring_ptr]
sage: total_ring_reference_count() - n
4
By :trac:`13447`, there is no longer a strong cache for multivariate
polynomial rings. Thus, we obtain::
sage: del P
sage: _ = gc.collect()
sage: ring_ptr in ring_refcount_dict
True
sage: total_ring_reference_count() - n
0
"""
if existing_ring is NULL:
raise ValueError('singular_ring_reference(ring*) called with NULL pointer.')
if existing_ring is NULL or refcount is NULL:
raise ValueError('singular_ring_reference(ring*, int*) called with NULL pointer.')
cdef object r = wrap_ring(existing_ring)
ring_refcount_dict[r] += 1
refcount[0] += 1
global ring_reference_count
ring_reference_count += 1
existing_ring.ref = refcount[0] # Some singular functions mess with .ref, so, we force to keep it in sync
return existing_ring
#############################################################################
cdef void singular_ring_delete(ring *doomed):
cdef int singular_ring_delete(ring *doomed, int *refcount) except 1:
"""
Carefully deallocate the ring, without changing "currRing" (since
this method can be called at unpredictable times due to garbage
......@@ -613,18 +500,16 @@ cdef void singular_ring_delete(ring *doomed):
if doomed is NULL:
# When this is called with a NULL pointer, we do nothing.
# This is analogous to the libc function free().
return
if not ring_refcount_dict: # arbitrary finalization order when we shut Sage down
return
return 0
cdef ring_wrapper_Py r = wrap_ring(doomed)
ring_refcount_dict[r] -= 1
if ring_refcount_dict[r] > 0:
return
del ring_refcount_dict[r]
refcount[0] -= 1
global ring_reference_count
ring_reference_count -= 1
doomed.ref = refcount[0] # Some singular functions mess with .ref, so, we force to keep it in sync
if doomed.ref:
return 0
sig_free(refcount)
global currRing
cdef ring *oldRing = currRing
if currRing == doomed:
......@@ -634,7 +519,7 @@ cdef void singular_ring_delete(ring *doomed):
rChangeCurrRing(doomed)
rDelete(doomed)
rChangeCurrRing(oldRing)
return 0
#############################################################################
# helpers for debugging
......@@ -689,16 +574,3 @@ cpdef print_currRing():
"""
cdef size_t addr = <size_t>currRing
print("DEBUG: currRing == " + str(hex(addr)))
def currRing_wrapper():
"""
Returns a wrapper for the current ring, for use in debugging ring_refcount_dict.
EXAMPLES::
sage: from sage.libs.singular.ring import currRing_wrapper
sage: currRing_wrapper()
The ring pointer ...
"""
return wrap_ring(currRing)
......@@ -8,6 +8,7 @@ cdef class MPolynomialRing_libsingular
cdef class MPolynomial_libsingular(MPolynomial):
cdef poly *_poly
cdef ring *_parent_ring
cdef int *_parent_ring_ref
cpdef _add_(self, other)
cpdef _mul_(self, other)
cpdef _floordiv_(self, right)
......@@ -24,6 +25,7 @@ cdef class MPolynomialRing_libsingular(MPolynomialRing_base):
cdef object __minpoly
cdef poly *_one_element_poly
cdef ring *_ring
cdef int *_ring_ref
# new polynomials
cdef MPolynomial_libsingular new_MP(MPolynomialRing_libsingular parent, poly *p)
......@@ -172,7 +172,7 @@ AUTHORS:
# * p_Normalize apparently needs currRing
from cpython.object cimport Py_NE
from cysignals.memory cimport sig_malloc, sig_free
from cysignals.memory cimport sig_malloc, sig_calloc, sig_free
from cysignals.signals cimport sig_on, sig_off
from sage.cpython.string cimport char_to_str, str_to_bytes
......@@ -256,7 +256,6 @@ from sage.misc.sage_eval import sage_eval
cimport cypari2.gen
from . import polynomial_element
permstore=[]
cdef class MPolynomialRing_libsingular(MPolynomialRing_base):
def __cinit__(self):
......@@ -381,15 +380,15 @@ cdef class MPolynomialRing_libsingular(MPolynomialRing_base):
NotImplementedError: polynomials in -1 variables are not supported in Singular
"""
self.__ngens = n
self._ring = singular_ring_new(base_ring, n, names, order)
if self._ring_ref is NULL:
self._ring_ref = <int*>sig_calloc(1, sizeof(int))
self._ring = singular_ring_reference( singular_ring_new(base_ring, n, names, order), self._ring_ref )
self._zero_element = new_MP(self, NULL)
cdef MPolynomial_libsingular one = new_MP(self, p_ISet(1, self._ring))
self._one_element = one
self._one_element_poly = one._poly
MPolynomialRing_base.__init__(self, base_ring, n, names, order)
self._has_singular = True
#permanently store a reference to this ring until deallocation works reliably
permstore.append(self)
def __dealloc__(self):
r"""
......@@ -415,8 +414,8 @@ cdef class MPolynomialRing_libsingular(MPolynomialRing_base):
sage: del R3
sage: _ = gc.collect()
"""
if self._ring != NULL: # the constructor did not raise an exception
singular_ring_delete(self._ring)
singular_ring_delete(self._ring, self._ring_ref)
self._ring = NULL
def __copy__(self):
"""
......@@ -428,23 +427,27 @@ cdef class MPolynomialRing_libsingular(MPolynomialRing_base):
sage: import gc
sage: from sage.rings.polynomial.multi_polynomial_libsingular import MPolynomialRing_libsingular
sage: from sage.libs.singular.ring import ring_refcount_dict
sage: from sage.libs.singular.ring import total_ring_reference_count
sage: gc.collect() # random output
sage: n = len(ring_refcount_dict)
sage: n = total_ring_reference_count()
sage: R = MPolynomialRing_libsingular(GF(547), 2, ('x', 'y'), TermOrder('degrevlex', 2))
sage: len(ring_refcount_dict) == n + 1
We have references to the underlying libsingular ring from `R`, from its two
generators and from the zero and one elements of `R`. Therefore::
sage: total_ring_reference_count() == n + 4
True
By :trac:`13447`, multivariate polynomial rings are no longer strongly
cached. Therefore we have::
sage: Q = copy(R) # indirect doctest
sage: p = R.gen(0) ^2+R.gen(1)^2
sage: p = R.gen(0)^2+R.gen(1)^2
sage: q = copy(p)
sage: del R
sage: del Q
sage: del p
sage: del q
sage: del R, Q, p, q
sage: gc.collect() # random output
sage: len(ring_refcount_dict) == n
False
sage: total_ring_reference_count() == n
True
"""
return self
......@@ -1952,18 +1955,27 @@ cdef class MPolynomial_libsingular(MPolynomial):
0
"""
self._poly = NULL
# During cyclic garbage collection, it can happen that the parent
# is deallocated before the element. But in order to deallocate the element,
# we need a pointer to a valid libsingular ring. Therefore, the element
# should not just store a pointer to the libsingular ring, but we increase
# the ring's reference count.
# _parent_ring and _parent_ring_ref are the same pointers as _parent._ring
# and _parent_ring_ref; this is needed since during deallocation of the
# element the parent may already be gone.
self._parent = parent
self._parent_ring = singular_ring_reference(parent._ring)
self._parent_ring_ref = (<MPolynomialRing_libsingular>parent)._ring_ref
self._parent_ring = singular_ring_reference(parent._ring, self._parent_ring_ref)
def __dealloc__(self):
# WARNING: the Cython class self._parent is now no longer accessible!
if self._poly==NULL:
# e.g. MPolynomialRing_libsingular._zero_element
singular_ring_delete(self._parent_ring)
singular_ring_delete(self._parent_ring, self._parent_ring_ref)
return
assert self._parent_ring != NULL # the constructor has no way to raise an exception
p_Delete(&self._poly, self._parent_ring)
singular_ring_delete(self._parent_ring)
singular_ring_delete(self._parent_ring, self._parent_ring_ref)
def __copy__(self):
"""
......@@ -5590,7 +5602,16 @@ cdef inline MPolynomial_libsingular new_MP(MPolynomialRing_libsingular parent, p
"""
cdef MPolynomial_libsingular p = MPolynomial_libsingular.__new__(MPolynomial_libsingular)
p._parent = parent
p._parent_ring = singular_ring_reference(parent._ring)
# During cyclic garbage collection, it can happen that the parent
# is deallocated before the element. But in order to deallocate the element,
# we need a pointer to a valid libsingular ring. Therefore, the element
# should not just store a pointer to the libsingular ring, but we increase
# the ring's reference count.
# _parent_ring and _parent_ring_ref are the same pointers as _parent._ring
# and _parent_ring_ref; this is needed since during deallocation of the
# element the parent may already be gone.
p._parent_ring_ref = parent._ring_ref
p._parent_ring = singular_ring_reference(parent._ring, p._parent_ring_ref)
p._poly = juice
p_Normalize(p._poly, p._parent_ring)
return p
......
......@@ -21,6 +21,7 @@ cdef class NCPolynomialRing_plural(Ring):
cdef public object _magma_gens, _magma_cache
cdef ring *_ring
cdef int *_ring_ref
# cdef NCPolynomial_plural _one_element
# cdef NCPolynomial_plural _zero_element
......@@ -32,6 +33,8 @@ cdef class ExteriorAlgebra_plural(NCPolynomialRing_plural):
cdef class NCPolynomial_plural(RingElement):
cdef poly *_poly
cdef ring *_parent_ring
cdef int *_parent_ring_ref
cpdef _add_(self, other)
cpdef _mul_(self, other)
cpdef _repr_short_(self)
......
......@@ -111,7 +111,7 @@ from sage.cpython.string cimport char_to_str
# singular rings
from sage.libs.singular.ring cimport singular_ring_new, singular_ring_delete, wrap_ring, singular_ring_reference
from sage.libs.singular.ring cimport singular_ring_delete, singular_ring_reference
from sage.libs.singular.singular cimport si2sa, sa2si, overflow_check
......@@ -333,7 +333,8 @@ cdef class NCPolynomialRing_plural(Ring):
cdef RingWrap rw = ncalgebra(self._c, self._d, ring = P)
# rw._output()
self._ring = singular_ring_reference(rw._ring)
self._ring_ref = rw._ring_ref
self._ring = singular_ring_reference(rw._ring, self._ring_ref)
self._ring.ShortOut = 0
self.__ngens = n
......@@ -412,7 +413,7 @@ cdef class NCPolynomialRing_plural(Ring):
sage: del R3
sage: _ = gc.collect()
"""
singular_ring_delete(self._ring)
singular_ring_delete(self._ring, self._ring_ref)
def _element_constructor_(self, element):
"""
......@@ -564,8 +565,6 @@ cdef class NCPolynomialRing_plural(Ring):
" as noncommutative polynomial") ### ??????
return new_NCP(self,_p)
cpdef _coerce_map_from_(self, S):
"""
The only things that coerce into this ring are:
......@@ -1427,12 +1426,24 @@ cdef class NCPolynomial_plural(RingElement):
"""
self._poly = NULL
self._parent = parent
# During cyclic garbage collection, it can happen that the parent
# is deallocated before the element. But in order to deallocate the element,
# we need a pointer to a valid libsingular ring. Therefore, the element
# should not just store a pointer to the libsingular ring, but we increase
# the ring's reference count.
# _parent_ring and _parent_ring_ref are the same pointers as _parent._ring
# and _parent_ring_ref; this is needed since during deallocation of the
# element the parent may already be gone.
self._parent_ring_ref = parent._ring_ref
self._parent_ring = singular_ring_reference(parent._ring, self._parent_ring_ref)
def __dealloc__(self):
# TODO: Warn otherwise!
# for some mysterious reason, various things may be NULL in some cases
if self._parent is not None and (<NCPolynomialRing_plural>self._parent)._ring != NULL and self._poly != NULL:
p_Delete(&self._poly, (<NCPolynomialRing_plural>self._parent)._ring)
# for some mysterious reason, various things may be NULL
# or already dealeted in some cases
if self._parent_ring != NULL and self._poly != NULL:
p_Delete(&self._poly, self._parent_ring)
singular_ring_delete(self._parent_ring, self._parent_ring_ref)
def __reduce__(self):
"""
......@@ -2820,6 +2831,16 @@ cdef inline NCPolynomial_plural new_NCP(NCPolynomialRing_plural parent,
"""
cdef NCPolynomial_plural p = NCPolynomial_plural.__new__(NCPolynomial_plural)
p._parent = parent
# During cyclic garbage collection, it can happen that the parent
# is deallocated before the element. But in order to deallocate the element,
# we need a pointer to a valid libsingular ring. Therefore, the element
# should not just store a pointer to the libsingular ring, but we increase
# the ring's reference count.
# _parent_ring and _parent_ring_ref are the same pointers as _parent._ring
# and _parent_ring_ref; this is needed since during deallocation of the
# element the parent may already be gone.
p._parent_ring_ref = parent._ring_ref
p._parent_ring = singular_ring_reference(parent._ring, p._parent_ring_ref)
p._poly = juice
p_Normalize(p._poly, parent._ring)
return p
......@@ -2856,10 +2877,10 @@ cpdef MPolynomialRing_libsingular new_CRing(RingWrap rw, base_ring):
Check that :trac:`13145` has been resolved::
sage: h = hash(R.gen() + 1) # sets currRing
sage: from sage.libs.singular.ring import ring_refcount_dict, currRing_wrapper
sage: curcnt = ring_refcount_dict[currRing_wrapper()]
sage: from sage.libs.singular.ring import total_ring_reference_count
sage: curcnt = total_ring_reference_count()
sage: newR = new_CRing(W, H.base_ring())
sage: ring_refcount_dict[currRing_wrapper()] - curcnt
sage: total_ring_reference_count() - curcnt
2
Check that :trac:`29311` is fixed::
......@@ -2875,14 +2896,12 @@ cpdef MPolynomialRing_libsingular new_CRing(RingWrap rw, base_ring):
cdef MPolynomialRing_libsingular self = <MPolynomialRing_libsingular>MPolynomialRing_libsingular.__new__(MPolynomialRing_libsingular)
self._ring = rw._ring
self._ring_ref = rw._ring_ref
self._ring = singular_ring_reference(rw._ring, self._ring_ref)
cdef MPolynomial_libsingular one = new_MP(self, p_ISet(1, self._ring))
self._one_element = one
self._one_element_poly = one._poly
wrapped_ring = wrap_ring(self._ring)
sage.libs.singular.ring.ring_refcount_dict[wrapped_ring] += 1
self._ring.ShortOut = 0
self.__ngens = rw.ngens()
......@@ -2949,10 +2968,9 @@ cpdef NCPolynomialRing_plural new_NRing(RingWrap rw, base_ring):
assert( not rw.is_commutative() )
cdef NCPolynomialRing_plural self = <NCPolynomialRing_plural>NCPolynomialRing_plural.__new__(NCPolynomialRing_plural)
self._ring = rw._ring
wrapped_ring = wrap_ring(self._ring)
sage.libs.singular.ring.ring_refcount_dict[wrapped_ring] += 1
self._ring_ref = rw._ring_ref
self._ring = singular_ring_reference(rw._ring, self._ring_ref)
self._ring.ShortOut = 0
......
......@@ -550,6 +550,29 @@ def PolynomialRing(base_ring, *args, **kwds):
Traceback (most recent call last):
...
TypeError: unable to convert 'x' to an integer
By :trac:`13447`, polynomial rings can be garbage collected::
sage: from sage.libs.singular.ring import total_ring_reference_count
sage: n = total_ring_reference_count()
sage: P.<x,y,z> = GF(19)[]
sage: del P,x,y,z
sage: import gc
sage: _ = gc.collect()
sage: n == total_ring_reference_count()
True
The following still leaks, likely because of strong refs in the coercion system::
sage: from sage.libs.singular.ring import total_ring_reference_count
sage: n = total_ring_reference_count()
sage: P.<x,y,z> = GF(19)[]
sage: p = 2*x^3+x*y*z-z^2*x^2
sage: del P,x,y,z,p
sage: _ = gc.collect()
sage: n == total_ring_reference_count()
False
"""
if not ring.is_Ring(base_ring):
raise TypeError("base_ring {!r} must be a ring".format(base_ring))
......