Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use __del__ in Python to clean up Rust objects #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

scooter-dangle
Copy link

__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 ZipCodeDatabases 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.
  1. 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 #60

`__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 scooter-dangle force-pushed the cleanup-rust-objects-in-python-via-__del__ branch from de7972f to c94fe45 Compare February 15, 2018 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Returning objects to Python from Rust should use __del__ for cleanup rather than context manager
1 participant