[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Changes by Jon Dufresne : -- pull_requests: +2015 ___ Python tracker ___ ___

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Jon Dufresne added the comment: I decided to try a new direction. Instead of modifying _TemporaryFileCloser to handle urllib, I've changed urllib classes to not inherit from _TemporaryFileCloser. The urllib classes are not temporary files as built by tempfile, so I believe this makes more

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Jon Dufresne added the comment: I've taken a new approach to resolve the urllib issues. I believe HTTPError _should not_ warn when __del__ is called as HTTPError wraps an existing resource instead of generating its own. IIUC, in this case, I believe it falls to the responsibility of code

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Changes by STINNER Victor : -- nosy: +haypo ___ Python tracker ___ ___

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Jon Dufresne added the comment: Just for some context, the e.close() is handling this bit of code: https://github.com/python/cpython/blob/d8132c4da7c46587221c5a244224b770d03860b6/Lib/urllib/request.py#L739-L754 When there is no error, http_error_302() will close the passed fp, on error, it

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

R. David Murray added the comment: Hmm. That e.close() looks like there may be a deeper problem here. I don't have time to investigate myself, unfortunately. -- ___ Python tracker

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Jon Dufresne added the comment: Thanks for the review. I have updated the patch. Now all warnings during tests handled. Please let me know if there are any other concerns with the changes. -- Added file: http://bugs.python.org/file45789/namedtemporaryfile-resourcewarning-2.patch

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

SilentGhost added the comment: The ResourceWarning that you've introduced need to be "fixed" in a few place in test_urllib2 -- stage: -> needs patch versions: +Python 3.7 ___ Python tracker

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

Changes by Jon Dufresne : -- keywords: +patch Added file: http://bugs.python.org/file45783/namedtemporaryfile-resourcewarning.patch ___ Python tracker

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

R. David Murray added the comment: __del__ is a destructor. If the file is open when it is called, it should emit a ResourceWarning. -- ___ Python tracker

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

SilentGhost added the comment: > The point of ResourceWarning it tell you when you are depending on > destructors for cleanup But there is an explicit call in __del__ to .close(), it's looks like intended behaviour and is ultimately the difference that Jon was seeing between

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

R. David Murray added the comment: The point of ResourceWarning it tell you when you are depending on destructors for cleanup, so this does look like a bug. -- nosy: +r.david.murray ___ Python tracker

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

SilentGhost added the comment: Looking at the code it seems NamedTemporaryFile is closed on destruction of the object, so there is no need for ResourceWarning, because the file descriptor is not leaking. You also don't need to involve unittest here. Just running python with -Wall would

[issue28867] NamedTemporaryFile does not generate a ResourceWarning for unclosed files (unlike TemporaryFile)

New submission from Jon Dufresne: When using unittest, I'll frequently enable -Wall to help catch code smells and potential bugs. One feature of this, I'm told when files aren't explicitly closed with an error like: "ResourceWarning: unclosed file <...>". I've noticed this warning is