kootenpv/yagmail

Automatic cleanup should not be encouraged or relied upon

tilgovi opened this issue · 2 comments

tilgovi commented

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 commented

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 commented

👍