classification
Title: Fix memory cycles in Weak*Dictionary
Type: Stage:
Components: Library (Lib) Versions:
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: fdrake Nosy List: barry, ddaa10, fdrake
Priority: high Keywords: patch

Created on 2001-04-21 04:19 by ddaa10, last changed 2001-09-28 19:02 by fdrake. This issue is now closed.

Files
File name Uploaded Description Edit
weakref-patch ddaa10, 2001-04-21 04:19 patch-weakref.py
Messages (3)
msg36428 - (view) Author: Deleted User ddaa10 (ddaa10) Date: 2001-04-21 04:19
There is a subtle bug in WeakValueDictionary and
WeakKeyDictonary that cause them to create reference
cycles.

This does not break anything by itself, but:
1/ it's bad to rely on the garbage collector;
2/ this could lead to uncollectable cycles if a
subclass of Weak*Dictionary defines __del__.

The trick is that deletions of a key/value of a item
trigger a remove function. That remove function is
strong referenced inside the weakref object. That
function internally stores a strong reference to the
self.data (dictionary) object. self.data obviously
strong references the weakref object.

The minimal overhead solution would be to use a __del__
method that would empty the self.data dictionary (thus
breaking the references loops at Weak*Dictionary
destruction). This solution is not acceptable since a
key (for WeakValueDictionnary) or a value (for
WeakKeyDictionary) can be a composite object. Such
composite object can contain a reference which
ultimately loops back to the Weak*Dictionary. Ok that's
an user ref cycle, but the library __del__ method makes
it uncollectable.

The solution I propose is to change the remove function
to contain, not a strong reference to the dictionary,
but a weak reference to the Weak*Dictionary object. If
the weakref has expired when the remove function is
called; we just do nothing.

This can lead to a silent change in behavior if the
user has broken the encapsulation of the
Weak*Dictionary and stored an external reference to the
data dictionary. I think the best possible solution
would be to give the choice between the two remove
functions at runtime through a module variable.

The attached patch makes a "weakref-patched.py" file
from the "weakref.py" file of the 2.1 distribution.
That file implements my proposed solution. It has been
regression tested.

It would make sense to add test units for memory cycles
int test_weakref.py, but I'm too newbie (and do not
have enough time) to do it myself.

Keep on the good work.
msg36429 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2001-08-21 04:16
Logged In: YES 
user_id=12800

Bumping the priority to 6 since It Would Be Good if Fred
could look at this before 2.2a2, but not critical.
msg36430 - (view) Author: Fred L. Drake, Jr. (fdrake) (Python committer) Date: 2001-09-28 19:02
Logged In: YES 
user_id=3066

Accepted with minor modifications as Lib/weakref.py revision
1.13.
(Sorry for the delay!)
History
Date User Action Args
2001-04-21 04:19:19ddaa10create