add .unwrap_and_destroy() method to remove reference to value/error when unwrapping outcome#49
add .unwrap_and_destroy() method to remove reference to value/error when unwrapping outcome#49graingert wants to merge 7 commits intopython-trio:mainfrom
Conversation
e978344 to
b5f58f2
Compare
CoolCat467
left a comment
There was a problem hiding this comment.
As far as I can tell this looks like a good approach.
|
I like the idea of at some point making I'm not sure it helps to provide a (or does this footgun not really matter in most cases? then maybe breaking everything isn't a good idea... sorry, I don't really know the context for this change) |
|
@A5rocks here's the benefit of unwrap_and_destroy: https://github.com/python-trio/outcome/pull/49/changes#diff-57edcfc849c5b8867dc8c235f66164cfa33cab5f855acce0d9defa4f23bce499R115 |
when I make the change in trio to use the new method I'll change the dep on outcome to |
|
Hmm, I admit I still don't really get it. Like, here's the way I see it (which I guess must contain something wrong)... my core assumption is that this is a footgun we'd like to remove:
Now there's a reason to do 1: it would allow us to change Trio without breaking past versions -- until we do the major version bump. However, given we do the major version anyways, anyone on an existing version of Trio is technically using a version of It may be valuable to have the extra few versions on Trio with a |
|
Personally I think this is a work-around for a bug in CPython (and pypy) in genobject.c's send/throw method and I'm leaving test_unwrap_leaves_a_refcycle as a canary test to see if this is ever fixed in python implementations So I don't think it's so bad to have two different unwrap methods |
Fixes #47
currently unwraping exceptions leaves a refcycle that keeps the entire traceback and any locals alive. This PR resolves that by clearing the error field when the Error is unwrap_and_destroy-ed.
For the previous behaviour of Value (eg in trio) if you receive a large bytes object in a Value that bytes object stays alive long after it's needed. This PR resolves that by clearing the value field when the Value is unwrap_and_destroy-ed.
this is an alternative approach to #45 that is fully backwards compatible and will only need a minor version bump, eg
outcome==1.4.0