#14711: Weak references in the coercion graph
-------------------------------------+-------------------------------------
Reporter: jpflori | Owner: davidloeffler
Type: defect | Status: needs_review
Priority: critical | Milestone: sage-6.1
Component: number fields | Resolution:
Keywords: memleak, number | Merged in:
field, QuadraticField | Reviewers:
Authors: Simon King | Work issues:
Report Upstream: N/A | Commit:
Branch: | 37bf59eac24eda1ece89aff7dde4a1db5d0cbb5c
u/SimonKing/ticket/14711 | Stopgaps:
Dependencies: |
-------------------------------------+-------------------------------------
Comment (by jpflori):
I've had a look at the discussion here and the actual changes and am quite
happy with them, at least with my current knowledge of the coercion
framework.
I'll push in a few minutes some reviewer changes mostly for formatting
issues, a dirty utf8 รถ character and a failing doctest.
Apart from this, there is one piece of code I'm not completely sure to
get:
{{{
mor = self.discover_convert_map_from(S)
- self._convert_from_list.append(mor)
+ # Before trac #14711, the morphism has been
+ # put both into _convert_from_list and into
+ # _convert_from_hash. But there is no reason
+ # to have a double book-keeping, specifically
+ # if one of them is by strong references!
self._convert_from_hash.set(S, mor)
+ # Moreover, again by #14711, the morphism should
+ # only keep weak references to domain and codomain,
+ # to allow them being garbage collected.
+ if mor is not None:
+ mor._make_weak_references()
return mor
}}}
First, I feel the inlined comments may deserve to be moved into the
docstring.
Second, is the weakening necessary? Isn't the map already "weak"? And if
not, why weaken it after storing it in the MonoDict? If I get that
correctly, in the end, what is stored is weakened anyway as mor is just a
pointer to some python object, it is just that doing it after storing a
pointer to the object elsewhere feels akward to me.
(The same thing is done at some other places in the same file.)
--
Ticket URL: <http://trac.sagemath.org/ticket/14711#comment:168>
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.
