[issue11767] Maildir iterator leaks file descriptors by default

Serhiy Storchaka added the comment: Python 2.7 is no longer supported. -- nosy: +serhiy.storchaka resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker

[issue11767] Maildir iterator leaks file descriptors by default

Changes by Ezio Melotti ezio.melo...@gmail.com: -- stage: - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11767 ___ ___

[issue11767] Maildir iterator leaks file descriptors by default

Roundup Robot devnull@devnull added the comment: New changeset fea1920ae75f by R David Murray in branch '3.2': #11767: use context manager to close file in __getitem__ to prevent FD leak http://hg.python.org/cpython/rev/fea1920ae75f New changeset 1d7a91358517 by R David Murray in branch

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: The patch doesn't work on 2.7. The failures are related to #11700, although that would seem to indicate that something *is* trying to close the file. I have no idea what. -- dependencies: +mailbox.py proxy updates

[issue11767] Maildir iterator leaks file descriptors by default

Filip Gruszczyński grusz...@gmail.com added the comment: Should I try to do something about this right now or should I wait until #1170 is finishedclosed and only then try to fix this issue too? -- ___ Python tracker rep...@bugs.python.org

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: Well, if you want to you could investigate further, and try the patch from #11700 and see what is left to do here after it has been applied. I'll try to get 11700 in soon, though. -- ___

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: Well, it turns out I was totally wrong about the 11700 dependency. I misread the errors that were produced by the test suite. Even after fixing 11700 they are still there: the tests are reading from the closed files. So something

[issue11767] Maildir iterator leaks file descriptors by default

Changes by Terry J. Reedy tjre...@udel.edu: -- versions: -Python 2.5, Python 2.6 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11767 ___ ___

[issue11767] Maildir iterator leaks file descriptors by default

Filip Gruszczyński grusz...@gmail.com added the comment: Here is a new patch, that uses with in __getitem__. I wonder, if we shouldn't check for an AttributeError in case object returned by get_file doesn't have __exit__ method, but does have close and use close instead of with. But it's for

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: Ah, that's exactly why I suggested using the 'closing' context manager from contextlib. That context manager returns the object passed to it, and then when its __exit__ method is called, calls the close method of that object that was

[issue11767] Maildir iterator leaks file descriptors by default

Filip Gruszczyński grusz...@gmail.com added the comment: I am sorry. This is the first time I see contextlib and haven't understood, that I should use a function from it. Here is a version with mock object having close method and __getitem__ using contextlib.closing. -- Added file:

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: I shouldn't have assumed that you knew about contextlib, and should have mentioned it by name the first time. The patch looks good to me. Not sure it is necessary to loop through ten fake mailboxes, but it doesn't hurt, either. (Other

[issue11767] Maildir iterator leaks file descriptors by default

Filip Gruszczyński grusz...@gmail.com added the comment: Shouldn't this be your responsibility to close this descriptor in the object used as self._factory? When self._factory is called it should read from the file like object and then simply close it, just as get_message (when self._factory

[issue11767] Maildir iterator leaks file descriptors by default

Brendan Dolan-Gavitt brenda...@gatech.edu added the comment: The _factory object didn't open the file, so why should it close it? It's also worth noting that things one would naturally consider using as factories (like email.message_from_file) don't close the fd when they're done. I will

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: It is not clear from the docs what the expected behavior of factory is. It is certainly the case that the custom classes used by mailbox by default do not close the file they are passed. So either those classes should close the input

[issue11767] Maildir iterator leaks file descriptors by default

Changes by R. David Murray rdmur...@bitdance.com: Removed file: http://bugs.python.org/file21554/11767.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11767 ___

[issue11767] Maildir iterator leaks file descriptors by default

R. David Murray rdmur...@bitdance.com added the comment: get_file's promise is that what is returned is a file like object, so it not having a close() method would be an error. So I don't think you need the try/except. What I would suggest is to use the 'closing' context manager around the

[issue11767] Maildir iterator leaks file descriptors by default

Changes by Antoine Pitrou pit...@free.fr: -- nosy: +r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue11767 ___ ___ Python-bugs-list

[issue11767] Maildir iterator leaks file descriptors by default

New submission from Brendan Dolan-Gavitt brenda...@gatech.edu: The default constructor for Maildir is rfc822.Message. This means that when iterating over a Maildir mailbox instantiated with default settings, the Mailbox class will return self._factory(self.get_file(key)), leaking the file