[issue25995] os.walk() consumes a lot of file descriptors

Martin Panter added the comment: The change seems to be causing some Windows buildbot failures : == ERROR:

[issue25995] os.walk() consumes a lot of file descriptors

Roundup Robot added the comment: New changeset e951d76f1945 by Serhiy Storchaka in branch '3.5': Issue #25995: os.walk() no longer uses FDs proportional to the tree depth. https://hg.python.org/cpython/rev/e951d76f1945 New changeset 6197a09a56b1 by Serhiy Storchaka in branch 'default': Issue

[issue25995] os.walk() consumes a lot of file descriptors

Serhiy Storchaka added the comment: walk_consume_fds_opt1.patch is applied in 3.5 (it is more reliable with the absence of close()) and walk_consume_fds_opt2.patch is applied in 3.6. -- resolution: -> fixed stage: patch review -> resolved status: open -> closed

[issue25995] os.walk() consumes a lot of file descriptors

Changes by Serhiy Storchaka : -- assignee: -> serhiy.storchaka ___ Python tracker ___

[issue25995] os.walk() consumes a lot of file descriptors

Serhiy Storchaka added the comment: Yes, OSError, MemoryError and KeyboardInterrupt can be raised during iterating scandir object or calling is_dir() or is_symlink() methods of the result. They stop iterating and left file descriptor open. If close file descriptor on error during iteration

[issue25995] os.walk() consumes a lot of file descriptors

Serhiy Storchaka added the comment: > But using FDs proportional to the tree depth seems reasonable to me. This was the main reason for rejecting issue15200. May be we can allow this for globbing, but only explicitly enabled with special parameter or separate functions. The benefit for

[issue25995] os.walk() consumes a lot of file descriptors

Guido van Rossum added the comment: I like them both, if I had to choose I'd pick patch 2. But yes, we need to add a close() method to the scandir iterator object. In the meantime, I am still worried about what would happen if somehow the loop got interrupted and the frame got kept alive and

[issue25995] os.walk() consumes a lot of file descriptors

Serhiy Storchaka added the comment: Both patches are basically equivalent. The first one collects all scandir() results in a list, the second one collects only directory names in a list. The purpose of using os.scandir() in os.walk() was a speed up (issue23605), and both patches preserve it.

[issue25995] os.walk() consumes a lot of file descriptors

Guido van Rossum added the comment: I am all for preventing the leaks. But using FDs proportional to the tree depth seems reasonable to me. (If you are worried about some kind of DoS attack on the algorithm by someone who can build a tree with depth 1000, well, if they can do that they can also

[issue25995] os.walk() consumes a lot of file descriptors

Guido van Rossum added the comment: I don't like the first solution (it just collects all scandir() results in a list, which defeats one of the purposes of using scandir()), but I don't understand the second solution. Ideally the number of FDs used should be limited by the depth of the tree

[issue25995] os.walk() consumes a lot of file descriptors

Serhiy Storchaka added the comment: Here are two alternative solutions for this issue. -- keywords: +patch stage: -> patch review Added file: http://bugs.python.org/file41482/walk_consume_fds_opt1.patch ___ Python tracker

[issue25995] os.walk() consumes a lot of file descriptors

Changes by Serhiy Storchaka : Added file: http://bugs.python.org/file41483/walk_consume_fds_opt2.patch ___ Python tracker ___