#13447: Make libsingular multivariate polynomial rings collectable
----------------------------------------------------------------+-----------
Reporter: nbruin |
Owner:
Type: defect |
Status: needs_work
Priority: major |
Milestone: sage-5.4
Component: memleak |
Resolution:
Keywords: | Work
issues: Understand why sometimes `new_RingWrap` needs an incref and sometimes
not
Report Upstream: None of the above - read trac for reasoning. |
Reviewers: Simon King
Authors: Nils Bruin, Simon King | Merged
in:
Dependencies: #11521 |
Stopgaps:
----------------------------------------------------------------+-----------
Comment (by nbruin):
Replying to [comment:70 SimonKing]:
> I think one potential problem with our current use of `currRingHdl` in
combination with `rKill` is that we would ''redefine''
`my_awesome_sage_ring`, hence, we would see the following message on
stderr:
> {{{
> // ** redefining my_awesome_sage_ring **
> }}}
I suspect the records do get cleared, because otherwise Singular would
leave a dangling pointer around and one would have what is essentially a
singular ring variable with NULL pointer to a ring or a pointer to
unallocated memory .
If those variables do get properly removed, by the time we have to
redefine the thing there should be no trace of the original and hence it
would not be a redefinition. If the skeleton somehow does remain (with a
nulled out `data.uring` field then, I suppose?), we could keep it around:
{{{
saved_skeleton=enterid("my_awesome_sage_ring", 0, RING_CMD, &IDROOT,
0)
....
if currRingHdl == NULL:
currRingHdl = saved_skeleton
currRingHdl.data.uring = singular_ring_reference(currRing)
}}}
Incidentally, doesn't `currRingHdl` count as a ring reference? The code in
`SingularFunction` that started this whole thing seems to indicate this.
In that case the only `singular_ring_delete(R)` that can trigger nulling
of `currRingHdl` and `currRing` is when the explicit intention is
`singular_ring_delete(currRingHdl.data.uring)`. When decreffing any other
pointer that just happens to be pointing to the same ring structure, there
would still be a refcount left, so no actual deletion would take place.
Note that after `singular_ring_delete(currRingHdl.data.uring)` we should
absolutely NULL `currRingHdl.data.uring` (probably safer/better to do
`currRingHdl=NULL`) even if deallocation didn't occur, because we should
remove the reference. Therefore, I find it surprising that `rKill` does do
a `currRingHdl=NULL` conditionally internally. With correct usage, `rKill`
should never execute that code usefully, because it needs to happen
unconditionally outside of `rKill` if it ever happens. Is it there as a
safety net against sloppy programming?
Note that doing `singular_ring_delete(currRing)` is nearly always wrong,
because `currRing` is a borrowed reference as far as I understand.
Are we the only ones ever setting `currRingHdl` or can one write/call
singular functions that internally will change `currRingHdl`?
If the former holds, the code becomes a little easier to reason about and
then we can make our own provisions and ensure they are followed properly
(such as saving a skeleton).
In the latter case we really have to play by Singular's rules, because
Singular might be doing the deallocating outside of our control.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13447#comment:72>
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.
