[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Changes by Jesús Cea Avión : -- nosy: +jcea ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Marking as closed/fixed. -- assignee: -> larry resolution: -> fixed stage: patch review -> committed/rejected status: open -> closed ___ Python tracker __

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Roundup Robot added the comment: New changeset bf68711bc939 by Larry Hastings in branch '3.3': Issue #17899: Fix rare file descriptor leak in os.listdir(). http://hg.python.org/cpython/rev/bf68711bc939 -- ___ Python tracker

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Roundup Robot added the comment: New changeset e51cbc45f4ca by Larry Hastings in branch 'default': Issue #17899: Fix rare file descriptor leak in os.listdir(). http://hg.python.org/cpython/rev/e51cbc45f4ca -- nosy: +python-dev ___ Python tracker

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Christian Heimes added the comment: ping... -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.pyth

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Okay, this bug has dragged on long enough. Serhiy already said they looked good to him, and I am now declaring that that's good enough for me. I'll check in my patches Saturday-ish unless someone pipes up. -- ___ P

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: I swear I've seen a compiler where you couldn't assign to errno. But now I'm pretty sure it was MSVC, and possibly a version back in the 90s. So I'm happy to live in a world where errno is an lvalue. -- ___ Python

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Christian Heimes added the comment: That's a preliminary report I made a couple of days ago. I posted it on G+ for some friends, it got re-posted by the official Python account and gone viral on Twitter. The interview is tonight. Coverity is probably going to create their own report after the

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Looks like it's too late for Christian, he's already generated the report with Coverity: https://docs.google.com/file/d/0B5wQCOK_TiRiMWVqQ0xPaDEzbkU/edit But I'm still interested in putting this to bed. -- ___ Pyth

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: And here's an updated patch for 3.3; the only change from the previous 3.3 patch is that I call path_error before calling close. -- Added file: http://bugs.python.org/file31021/larry.3.3.listdir.fd.leakage.bug.2.patch _

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: If someone will give me an LGTM I'll check these in tonight, honest, cross my heart. -- ___ Python tracker ___

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Attached is a new patch for trunk, removing the #undef HAVE_FDOPENDIR debug scaffolding, and rearranging the lines of error handling so close doesn't clear the errno before we use it. By cracky, while most days I do enjoy the exacting pedantry of the Python c

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: I've been staring at the code. I just realized: we really should call path_error() as soon as possible once we detect the error--as Christian's patch points out, close() will clear the error. So instead of playing footsie with assigning (further) to errno, I

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Christian Heimes added the comment: May I suggest a simpler patch that closes the fd sooner? -- Added file: http://bugs.python.org/file31017/listdir_fd.patch ___ Python tracker _

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Serhiy Storchaka added the comment: Larry, you forgot to remove "#undef HAVE_FDOPENDIR" at the start of the file. Beside this line the patch LGTM. I withdraw my objections in msg190887 because `close(fd)` can fail and change errno. -- ___ Python tra

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Christian Heimes added the comment: Preferable both but Coverity Scan uses only trunk. We could commit the patch to trunk first and test if Coverity still detects the leak. -- ___ Python tracker __

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Patch for 3.3. or trunk? -- ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: h

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Christian Heimes added the comment: I'd like to have a patch by tonight. This issue is one of two remaining bugs that are considered "high impact" by Coverity Scan. The other one looks like a false positive. I have an interview with Coverity tomorrow about the development style and quality of

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Serhiy Storchaka added the comment: rewinddir() is called only when dirp != NULL && fd > -1. fdopendir() is called when fd != -1. close() is called when dirp == NULL && fd != -1. Therefore rewinddir() and fdopendir() with close() can't be called in the same time. And you can move block if

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Here's a patch for 3.3. There's been enough churn around listdir in trunk that I was gonna have to write the patches separately anyway. -- Added file: http://bugs.python.org/file30521/larry.3.3.listdir.fd.leakage.bug.1.patch _

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Second rev incorporating a suggestion from the ever-present Serhiy. Also, for what it's worth, I walked through this with the debugger when using os.listdir(0) and it worked fine. -- Added file: http://bugs.python.org/file30520/larry.listdir.fd.leaka

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Changes by Serhiy Storchaka : -- nosy: +serhiy.storchaka ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe: http:/

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Evgeny Kapun added the comment: Simple test: while True: try: listdir(0) except NotADirectoryError: pass -- ___ Python tracker ___ __

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Evgeny Kapun added the comment: To make fdopendir fail, just pass any valid FD which points to a non-directory, such as a file or a pipe. -- ___ Python tracker ___ _

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Larry Hastings added the comment: Patch attached. I don't know how to make fdopendir fail, so I had to do it by inspection. While I was in there, I ifdef'd out the variable that should only be used if fdopendir is available. -- keywords: +patch stage: needs patch -> patch review Adde

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

Changes by Antoine Pitrou : -- nosy: +larry stage: -> needs patch ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscrib

[issue17899] os.listdir() leaks FDs if invoked on FD pointing to a non-directory

New submission from Evgeny Kapun: When called with a file descriptor as an argument, os.listdir() duplicates it to pass to fdopendir(3). If this call fails, the new file descriptor is not closed, which leads to file descriptor leak. -- components: Library (Lib) messages: 188322 nosy: a