#14711: Weak references in the coercion graph
-------------------------------------+-------------------------------------
Reporter: jpflori | Owner: davidloeffler
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-5.13
Component: number fields | Resolution:
Keywords: memleak, number | Merged in:
field, QuadraticField | Reviewers:
Authors: Simon King | Work issues:
Report Upstream: N/A | Commit:
Branch: | 364b9856b28d7060e3ea9825144de66c8f11ca2a
u/SimonKing/ticket/14711 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by nbruin):
Replying to [comment:126 SimonKing]:
> First of all, this leak is not introduced by my branch. Hence, it would
probably be better to attempt a fix on a different ticket, as the changes
introduced in my branch already are big enough.
Agreed.
> Typically, `C._coerce_map_from_(A)` just returns True, None or False,
and not
> a map. If it returns true, then a ''direct'' conversion `chi'` from A to
C is
> stored as coercion.
Hm, I agree that the references are in that case out of reach of what
we're considering here, but all this is saying is "it is okay to use
conversion as a coercion from A to C". This conversion still has to be
programmed/stored/discovered somewhere, so I'd expect that some conversion
cache might be liable to hold a strong reference.
> The punchline is: '''If `chi` is discovered by backtracking then `psi`
is
> stored in `C._coerce_from_list`.''' Without `psi` being on this list,
> backtracking won't find `chi`.
>
> Hence, there has `C.register_coercion(psi)` been done. With the current
code, it
> means that C will keep B alive.
Right, I was already expecting something along these lines when you
explained the function of `coerce_from_list`: The backbone of the coercion
framework presently requires lifetime specifications to be explicit and it
seems this is not just a by-product of the implementation, it seems to be
part of the spec. That's fine by itself. Whether having such implications
is sufficient for sage in the future remains to be seen, but changing that
is a redesign problem that would need to be carefully considered (just as
it might be desirable to allow multiple embeddings to be registered)
As a consequence, in the present model,
`register_coercion(...,strong=false)` would ''not'' be advisable.
> Case (3) is a leak, but this case can easily be
> avoided by returning a "simple" map that is mathematically equivalent to
the
> composite map.
This would be a necessary step to avoid the leak, and all the coercion
system can do, but if programmed in a generic way, the references causing
the leak would likely still be present internally.
--
Ticket URL: <http://trac.sagemath.org/ticket/14711#comment:131>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
