#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:
--------------------------------------------------+-------------------------
Changes (by SimonKing):
* cc: AlexanderDreyer, PolyBoRi (added)
Comment:
Replying to [comment:244 nbruin]:
> Replying to [comment:242 SimonKing]:
> Then `GroebnerStrategy` ''never'' deletes `_strat` and
`ReductionStrategy` ''always'' does that.
Again, the problem is that `GroebnerStrategy._strat` is not the thing that
is lend to `ReductionStrategy._strat`. The reference that is lend is
`GroebnerStrategy._strat.generators`. But since `GroebnerStrategy._strat`
is more than that, `GroebnerStrategy.__dealloc__` ''must'' take care of
it. "Take care of that" means: Call a C++ destructor on
`GroebnerStrategy._strat`. Hence, changing it would mean to change the
polybori spkg. I am ccing the authors, perhaps they consider such change
for the next version.
> 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.
Probably you are right. But the practical question is whether this should
better be on a "proper" `PolyBoRi` ticket. If you see an easy way to fix
the problem properly, then it may be done here. But my impression is that
a proper solution needs a non-trivial amount of changes to
sage/rings/polynomial/pbori.pyx; I'd prefer to have a non-trivial change
on a new ticket.
> 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.
Well, they are different things.
> '''EDIT:''' Plus the complication about
`GroebnerStrategy._strat.generators` and `ReductionStrategy._strat`. I
suspect this whole bit can be cleaned up without resorting to `borrowed`
flags but this is not the place.
Well, but how, without opening the polybori spkg?
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/12313#comment:246>
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.
