#13447: Make libsingular multivariate polynomial rings collectable
----------------------------------------------------------------+-----------
Reporter: nbruin |
Owner:
Type: defect |
Status: needs_review
Priority: major |
Milestone: sage-5.4
Component: memleak |
Resolution:
Keywords: | Work
issues:
Report Upstream: None of the above - read trac for reasoning. |
Reviewers: Simon King
Authors: Nils Bruin, Simon King | Merged
in:
Dependencies: #11521 |
Stopgaps:
----------------------------------------------------------------+-----------
Changes (by SimonKing):
* status: needs_info => needs_review
* work_issues: Input from a libsingular expert =>
Comment:
Hans Schönemann gave me a couple of answers - many thanks! I hope I am
translating and summarising correctly:
`r->ref` counts the number of ''interpreter variables'' referencing a ring
''minus one''. Hence:
{{{
ring r=.....; // ref is 0
def rr=r; // ref is 1
kill r; // ref is 0
kill rr; // ref is 0 when calling rKill -> delete it.
}}}
There are locally generated rings (in std, for example), unbeknownst to
the interpreter - they have ref zero.
Singular uses `r->ref` only in `rKill` - it makes me wonder why we use
`rDelete` and not `rKill`, by the way. In fact, `rKill` deletes local
data, but `rDelete` doesn't.
Generally, one should have `currRingHdl.ring == currRing`, or
`currRing==NULL` and `currRingHdl==NULL`. I just notice that he wrote
`currRingHdl.ring`, not `currRingHdl.data.uring` - is that a difference?
Concerning debugging: If one builds Singular so that `OM_NDEBUG` is
''not'' defined, then a debug version of omalloc is used. In that way, we
would have more easily detected the original problem with `omStrDup`.
'''Conclusions'''
I am not sure if you agree with my conclusions, but here we go:
Since `r->ref` does not play a rôle in libsingular, we are free to use
`r->ref` to count the number of pointers to r (not "number of pointers
minus one"). However, when calling singular interpreter functions, we must
make sure that `r->ref>0`. With our patch, we already do so - hence,
that's fine.
Since we apply `singular_ring_delete` to non-commutative (quotient) rings,
and we do not call `rKill` but only `rDelete`, we currently have a memory
leak for non-commutative rings. `rKill` will first test that `r->ref==0`,
then kills local data, then kills the ring, and sets `currRing` and
friends to NULL if the to-be-deleted ring is `currRing`. '''Hence, we
should use `rKill` in `singular_ring_delete`,''' but probably without the
`rChangeCurrRing` dance.
If `currRing==NULL`, we should ''not'' create `currRingHdl`. If
`currRing!=NULL` then we should let uring point to it. the latter is
already done in my patch, '''the former should be done.'''
So far I forgot to ask Hans about `enterid("my_awesome_sage_ring", 0,
RING_CMD, &IDROOT, 1)`.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/13447#comment:57>
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.
