Changes between Initial Version and Version 1 of Ticket #715, comment 298


Ignore:
Timestamp:
09/10/12 20:55:39 (7 years ago)
Author:
nbruin
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #715, comment 298

    initial v1  
    2525In the mean time, we can "fix" the segfault on bsd by allocating a little extra space for variable names. At least 9 bytes seems to do the trick. By now it's pretty clear that the real error is probably a refcounting error in sage libsingular rings, which didn't become apparent until these things actually do get deallocated.
    2626
    27 If we insist that libsingular behaves as specified, then part of their specification is likely that they should not be deallocated, so then we should put in a strong reffing cache on these things (easy to do). Then one can make another ticket "make libsingular rings deallocatable".
     27If we insist that libsingular rings behave as specified, then part of their specification is likely that they should not be deallocated. Since Volker has already put in manual refcounting, we can simply get the result by
     28
     29'''sage/libs/singular/ring.pyx''':
     30{{{#!diff
     31     wrapped_ring = wrap_ring(_ring)
     32     if wrapped_ring in ring_refcount_dict:
     33         raise ValueError('newly created ring already in dictionary??')
     34-    ring_refcount_dict[wrapped_ring] = 1
     35+    ring_refcount_dict[wrapped_ring] = 2
     36     return _ring
     37}}}
     38Then one can make another ticket "make libsingular rings deallocatable". Given that these rings get tied into the coercion framework anyway, I think you'd be hard-pressed to find a memory regression wrt. pre-#715 sage (perhaps one would have to increase a refcount on an object one level higher, since the `ring_wrapper_Py` objects don't actually live with the _ring. They're only to do an equality test. So with this fix, I think rings would leak in the sense that the `UniqueRepresentation` type that wraps them would die without the ring dying.)
    2839
    2940I think exposing the rest of sage to mortal parents is too important to delay on a hard-to-track-down memory issue for deallocation in libsingular.