Use weakref to break circular dependency between wrapper and wrapped objects #217
Conversation
|
Note: this should get a minor version bump to prevent breaking code that don't keep references to wrapped classes |
| asyncgen = gen2_s2(gen_s, 3) | ||
| lst = [z async for z in asyncgen] | ||
| assert lst == [0, 1, 2] | ||
| s2._close_loop() |
There was a problem hiding this comment.
With this PR, should s2.__del__() and s2._close_loop be called automatically or is it still explicitly required at the end of this test?
There was a problem hiding this comment.
It's good to put it at the end of the test to get repeatability, but yes it would get called by the garbage collector "in a timely manner" after the test completes after this patch :D
Does that mean we'll have to go to 0.11.0 when we merge it? |
Yup. Also doesn't feel super urgent to release this right now, as it has a higher risk than immediate benefit |
Fix for #214
This adds weakref on the unsynchronized -> synchronized type references, such that if you no longer keep another reference to a wrapper type or method, the unwrapped/implementation version will not prevent it from getting cleaned up.
Another option to break the circular references that I considered was to weakref the
Synchronizerobject that's referenced by wrappers. At first I thought it was only used by the type stub generation (through SYNCHRONIZER_ATTR), but I soon realized that the synchronizer is also referenced by a bunch of closures in wrappers (which makes sense), and weakrefing the synchronizer would break potential "anonymous" wraps using short lived synchronizers.This is a fairly scary change so I'll probably hold off on merging this and let it simmer for a while