#12313: Fix yet another memory leak caused by caching of coercion data
--------------------------------------------------+-------------------------
Reporter: SimonKing | Owner:
Type: defect | Status:
needs_review
Priority: major | Milestone: sage-5.3
Component: memleak | Resolution:
Keywords: coercion weak dictionary | Work issues:
Report Upstream: N/A | Reviewers: Simon King,
Jean-Pierre Flori, John Perry
Authors: Simon King, Jean-Pierre Flori | Merged in:
Dependencies: #11521, #11599, #12969, #12215 | Stopgaps:
--------------------------------------------------+-------------------------
Comment (by nbruin):
Replying to [comment:242 SimonKing]:
> No, the reduction strategy borrows from the Gröbner strategy.
Yes, you're right. Sorry. But the problem remains of what to trust on
dealloc. The whole borrowing thing complicates that. I think we can
simplify it here.
Since `GroebnerStrategy` ''always'' lends a reference to the
`ReductionStrategy` it creates, why not give it outright? Then
`GroebnerStrategy` ''never'' deletes `_strat` and `ReductionStrategy`
''always'' does that.
The zeroing out of `GroebnerStrategy.reduction_strategy._struct` via that
path makes me a little queasy. Plus, what happens if someone goes digging
in `GroebnerStrategy` and keeps `ReductionStrategy` alive for longer? I
think handing the reference to a suborbinate on "loan" just makes the code
a little harder to reason about. It means you have to enforce that the
subordinate doesn't go and do some work for someone else with the lent
goods. It's so much easier if you don't have to enforce that at all.
Logically, I think `GroebnerStrategy` shouldn't have a `_strat` attribute.
It should always access `GroebnerStrategy.reduction_strategy._strat`. The
fact that `GroebnerStrategy._strat` does exist should be viewed as an
optimization, but not affect program logic.
[Note: this is just squabbling over keeping code more easily maintainable.
I agree JP's patch solves the problem. It's just nice if we can resolve
the problem by reducing code complexity rather than increasing it]
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12313#comment:244>
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.
