From 69ff94a47b8b137e967a6d9ed63880be7a0d0b47 Mon Sep 17 00:00:00 2001 From: Risan Date: Wed, 9 Jul 2025 14:23:36 +0900 Subject: [PATCH] Synchronize cypari2 stack and Python lifecycle --- cypari2/gen.pxd | 26 +++++++++++++++++++++++++- cypari2/gen.pyx | 24 ++++++++++++++++-------- cypari2/pari_instance.pyx | 2 +- cypari2/stack.pyx | 20 +++++++++++++------- 4 files changed, 55 insertions(+), 17 deletions(-) diff --git a/cypari2/gen.pxd b/cypari2/gen.pxd index 9579dee..0cb7775 100644 --- a/cypari2/gen.pxd +++ b/cypari2/gen.pxd @@ -1,5 +1,6 @@ cimport cython from cpython.object cimport PyObject +from cpython.weakref cimport PyWeakref_NewRef, PyWeakref_GetObject from .types cimport GEN, pari_sp @@ -28,14 +29,37 @@ cdef class Gen(Gen_base): cdef inline pari_sp sp(self) noexcept: return self.address + # Enable weak references in Gen. + cdef object __weakref__ + # The Gen objects on the PARI stack form a linked list, from the # bottom to the top of the stack. This makes sense since we can only # deallocate a Gen which is on the bottom of the PARI stack. If this # is the last object on the stack, then next = top_of_stack # (a singleton object). # + # The connection between the list elements are implemented using the + # _next attribute. In order to not increase reference counts, the + # _next attribute is implemented as a weak reference. # In the clone and constant cases, this is None. - cdef Gen next + # + # Do not set or access _next directly. Please use the get_next() and + # set_next() methods. + cdef object _next + + cdef inline object get_next(self): + if self._next is None: + return None + cdef PyObject* result_ptr = PyWeakref_GetObject(self._next) + if result_ptr == NULL or result_ptr is None: + return None + return result_ptr + + cdef inline void set_next(self, Gen value): + if value is None: + self._next = None + else: + self._next = PyWeakref_NewRef(value, None) # A cache for __getitem__. Initially, this is None but it will be # turned into a dict when needed. diff --git a/cypari2/gen.pyx b/cypari2/gen.pyx index db23af2..6d575d3 100644 --- a/cypari2/gen.pyx +++ b/cypari2/gen.pyx @@ -154,13 +154,21 @@ cdef class Gen(Gen_base): def __init__(self): raise RuntimeError("PARI objects cannot be instantiated directly; use pari(x) to convert x to PARI") + def __del__(self): + if self.get_next() is not None: + # Python can deallocate Gen at any time even when it is not + # at the stack bottom. When self is not at the stack + # bottom, we need to move everything below it to heap + # before deallocation. + move_gens_to_heap(self.sp()) + def __dealloc__(self): - if self.next is not None: + if self.get_next() is not None: # stack remove_from_pari_stack(self) elif self.address is not NULL: # clone - gunclone_deep(self.address) + gunclone(self.address) cdef Gen new_ref(self, GEN g): """ @@ -191,7 +199,7 @@ cdef class Gen(Gen_base): >>> pari("[[1, 2], 3]")[0][1] # indirect doctest 2 """ - if self.next is not None: + if self.get_next() is not None: raise TypeError("cannot create reference to PARI stack (call fixGEN() first)") if is_on_stack(g): raise ValueError("new_ref() called with GEN which does not belong to parent") @@ -205,8 +213,8 @@ cdef class Gen(Gen_base): Return the PARI ``GEN`` corresponding to ``self`` which is guaranteed not to change. """ - if self.next is not None: - move_gens_to_heap(self.sp()) + if self.get_next() is not None: + move_gens_to_heap((self.get_next()).sp()) return self.g cdef GEN ref_target(self) except NULL: @@ -1532,7 +1540,7 @@ cdef class Gen(Gen_base): raise IndexError("column j(=%s) must be between 0 and %s" % (j, self.ncols()-1)) self.cache((i, j), x) - xt = x.ref_target() + xt = x.fixGEN() set_gcoeff(self.g, i+1, j+1, xt) return @@ -1556,7 +1564,7 @@ cdef class Gen(Gen_base): raise IndexError("index (%s) must be between 0 and %s" % (i, glength(self.g)-1)) self.cache(i, x) - xt = x.ref_target() + xt = x.fixGEN() if typ(self.g) == t_LIST: listput(self.g, xt, i+1) else: @@ -4631,7 +4639,7 @@ cdef class Gen(Gen_base): cdef int Gen_clear(self) except -1: """ Implementation of tp_clear() for Gen. We need to override Cython's - default since we do not want self.next to be cleared: it is crucial + default since we do not want self._next to be cleared: it is crucial that the next Gen stays alive until remove_from_pari_stack(self) is called by __dealloc__. """ diff --git a/cypari2/pari_instance.pyx b/cypari2/pari_instance.pyx index a0e89df..99d4d9a 100644 --- a/cypari2/pari_instance.pyx +++ b/cypari2/pari_instance.pyx @@ -1269,7 +1269,7 @@ cdef class Pari(Pari_auto): for j in range(n): sig_check() x = objtogen(entries[k]) - set_gcoeff(A.g, i+1, j+1, x.ref_target()) + set_gcoeff(A.g, i+1, j+1, x.fixGEN()) A.cache((i, j), x) k += 1 return A diff --git a/cypari2/stack.pyx b/cypari2/stack.pyx index b4d51fe..1115e47 100644 --- a/cypari2/stack.pyx +++ b/cypari2/stack.pyx @@ -62,11 +62,12 @@ cdef void remove_from_pari_stack(Gen self) noexcept: print(f"Expected: 0x{self.sp():x}") print(f"Actual: 0x{avma:x}") else: - warn(f"cypari2 leaked {self.sp() - avma} bytes on the PARI stack", - RuntimeWarning, stacklevel=2) - n = self.next + # This could only happen when a PARI computation is interrupted, in which + # case the avma needs to be reset. + reset_avma() + n = self.get_next() stackbottom = n - self.next = None + self.set_next(None) reset_avma() @@ -81,7 +82,7 @@ cdef inline Gen Gen_stack_new(GEN x): # the PARI stack. n = stackbottom z = Gen_new(x, avma) - z.next = n + z.set_next(n) stackbottom = z sz = z.sp() sn = n.sp() @@ -116,12 +117,17 @@ cdef int move_gens_to_heap(pari_sp lim) except -1: Move some/all Gens from the PARI stack to the heap. If lim == -1, move everything. Otherwise, keep moving as long as - avma <= lim. + avma < lim. """ - while avma <= lim and stackbottom is not top_of_stack: + while avma < lim and stackbottom is not top_of_stack: current = stackbottom sig_on() current.g = gclone(current.g) + # If current is a container type (e.g., a matrix or a vector), + # its entries will be cloned during this move. Hence, the + # itemcache will contain obsolete objects that needs to be + # cleared. + current.itemcache = None sig_block() remove_from_pari_stack(current) sig_unblock()