Use __del__ in Python to clean up Rust objects - #61
Open
scooter-dangle wants to merge 1 commit into
Open
Conversation
`__del__` more closely maps to what we want than context managers. For
instance, with the current code's use of context management, the only way to
make a collection of `ZipCodeDatabase`s would be to explicitly nest
`with ... as` statements. As well, a context manager would complicate
persisting a Rust object that is expensive to initialize but cheap to use,
whereas `__del__` supports this while still freeing the resources when the
reference count drops to zero.
There are two drawbacks to using `__del__` over context management:
1. Memory cleanup in the presence of reference cycles in Python is less
predictable.
* This is already an issue in Python that developers will need to be aware
of. The fact that some of the objects involved will be cleaned up by
calling a Rust function doesn't exacerbate that problem.
2. The Rust cleanup function might not run immediately upon the variable going
out of scope
* Since the cleanup function is to avoid a memory leak, I don't see how this
is any different than any other allocated object in Python. Additionally,
we know in the Rust world to not rely on `drop` being called to guarantee
properties like safety, so I think `__del__` maps better to the concept of
the `drop` function it will be calling.
Aside from `__del__` reducing the restrictions on using Rust objects within
Python, it's also conceptually simpler. And to an audience that's already
learning Rust, it should be easier to teach in conjunction with teaching
`Drop`.
Fixes shepmaster#60
scooter-dangle
force-pushed
the
cleanup-rust-objects-in-python-via-__del__
branch
from
February 15, 2018 14:27
de7972f to
c94fe45
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
__del__more closely maps to what we want than context managers. Forinstance, with the current code's use of context management, the only way to
make a collection of
ZipCodeDatabases would be to explicitly nestwith ... asstatements. As well, a context manager would complicatepersisting a Rust object that is expensive to initialize but cheap to use,
whereas
__del__supports this while still freeing the resources when thereference count drops to zero.
There are two drawbacks to using
__del__over context management:predictable.
of. The fact that some of the objects involved will be cleaned up by
calling a Rust function doesn't exacerbate that problem.
out of scope
is any different than any other allocated object in Python. Additionally,
we know in the Rust world to not rely on
dropbeing called to guaranteeproperties like safety, so I think
__del__maps better to the concept ofthe
dropfunction it will be calling.Aside from
__del__reducing the restrictions on using Rust objects withinPython, it's also conceptually simpler. And to an audience that's already
learning Rust, it should be easier to teach in conjunction with teaching
Drop.Fixes #60