[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Automatic cleanup should not be encouraged or relied upon #39

Closed
tilgovi opened this issue Apr 30, 2016 · 2 comments
Closed

Automatic cleanup should not be encouraged or relied upon #39

tilgovi opened this issue Apr 30, 2016 · 2 comments

Comments

@tilgovi
Copy link
tilgovi commented Apr 30, 2016

The README says "Note that this connection is reusable, closable and when it leaves scope it will clean up after itself."

I suspect that this is referring to CPython's reference counting garbage collection, but this is an implementation detail of CPython and not a specified behavior of the language.

On PyPy, I suspect following this advice would lead to leaks.

See also: http://doc.pypy.org/en/latest/cpython_differences.html#differences-related-to-garbage-collection-strategies

I would recommend removing this language from the README and encouraging people to always explicitly close connections.
@kootenpv
Copy link
Owner
kootenpv commented Apr 30, 2016 •
edited
Loading

Thanks tilgovi, for pointing it out! I was under the impression that at least in an earlier version it was working. I went on quite a hunt.

Observations:

  1. __exit__ was missing self.close
  2. __del__ was there in an earlier version, but somehow disappeared.
  3. __del__ makes sure that whenever it leaves scope, it indeed cleans up after itself in CPython
  4. In PyPy, __del__ indeed does not trigger on leaving scope!
  5. Using the context manager with does work though

It was a nice case for using the logging that's put in there; it made it super obvious.

I made a new commit (09b2a84), updating the README to watch out with PyPy and use with there. In CPython, with adding back in __del__, I don't believe we have anything to worry about. PyPy should use with to make sure it gets closed well.

Please verify that it works as you expect in yagmail 0.5.140, and feel free to close the issue or update it.

@tilgovi
Copy link
Author
tilgovi commented Apr 30, 2016

👍

@tilgovi tilgovi closed this as completed Apr 30, 2016
@kootenpv kootenpv mentioned this issue Aug 25, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@tilgovi @kootenpv