Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(2058)

#25994: File descriptor leaks in os.scandir()

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 8 months ago by storchaka+cpython
Modified:
1 year, 8 months ago
Reviewers:
victor.stinner, vadmium+py, jimjjewett
CC:
gvanrossum, haypo, benhoyt, devnull_psf.upfronthosting.co.za, Martin Panter, storchaka, josh.rosenberg
Visibility:
Public.

Patch Set 1 #

Total comments: 11

Patch Set 2 #

Total comments: 9

Patch Set 3 #

Total comments: 11

Patch Set 4 #

Total comments: 9
Unified diffs Side-by-side diffs Delta from patch set Stats Patch
Doc/library/os.rst View 1 2 3 2 chunks +24 lines, -3 lines 5 comments Download
Doc/whatsnew/3.6.rst View 1 2 3 1 chunk +11 lines, -0 lines 2 comments Download
Lib/os.py View 1 2 3 2 chunks +54 lines, -38 lines 0 comments Download
Lib/test/test_os.py View 1 2 3 1 chunk +66 lines, -0 lines 2 comments Download
Misc/NEWS View 1 2 3 1 chunk +3 lines, -0 lines 0 comments Download
Modules/posixmodule.c View 1 2 3 6 chunks +66 lines, -7 lines 0 comments Download

Messages

Total messages: 13
haypo
Can you please add a test calling close() and calling next() to check that the ...
1 year, 8 months ago #1
Martin Panter
https://bugs.python.org/review/25994/diff/16528/Doc/library/os.rst File Doc/library/os.rst (right): https://bugs.python.org/review/25994/diff/16528/Doc/library/os.rst#newcode1585 Doc/library/os.rst:1585: The :class:`scandir` function returns directory entries along with Maybe ...
1 year, 8 months ago #2
storchaka_gmail.com
Thank you for your review Victor and Martin. http://bugs.python.org/review/25994/diff/16528/Doc/library/os.rst File Doc/library/os.rst (right): http://bugs.python.org/review/25994/diff/16528/Doc/library/os.rst#newcode1585 Doc/library/os.rst:1585: The ...
1 year, 8 months ago #3
haypo
The overall changes now looks good to me with the new ResourceWarning. Just a few ...
1 year, 8 months ago #4
storchaka_gmail.com
http://bugs.python.org/review/25994/diff/16538/Lib/os.py File Lib/os.py (right): http://bugs.python.org/review/25994/diff/16538/Lib/os.py#newcode452 Lib/os.py:452: return self On 2016/02/09 16:50:33, haypo wrote: > add ...
1 year, 8 months ago #5
Martin Panter
https://bugs.python.org/review/25994/diff/16542/Doc/library/os.rst File Doc/library/os.rst (right): https://bugs.python.org/review/25994/diff/16542/Doc/library/os.rst#newcode1935 Doc/library/os.rst:1935: exhausted or explicitly closed a :exc:`ResourceWarning` will be emitted ...
1 year, 8 months ago #6
haypo
http://bugs.python.org/review/25994/diff/16542/Doc/library/os.rst File Doc/library/os.rst (right): http://bugs.python.org/review/25994/diff/16542/Doc/library/os.rst#newcode1935 Doc/library/os.rst:1935: exhausted or explicitly closed a :exc:`ResourceWarning` will be emitted ...
1 year, 8 months ago #7
storchaka_gmail.com
https://bugs.python.org/review/25994/diff/16542/Doc/library/os.rst File Doc/library/os.rst (right): https://bugs.python.org/review/25994/diff/16542/Doc/library/os.rst#newcode1935 Doc/library/os.rst:1935: exhausted or explicitly closed a :exc:`ResourceWarning` will be emitted ...
1 year, 8 months ago #8
haypo
http://bugs.python.org/review/25994/diff/16550/Lib/test/test_os.py File Lib/test/test_os.py (right): http://bugs.python.org/review/25994/diff/16550/Lib/test/test_os.py#newcode3097 Lib/test/test_os.py:3097: self.assertEqual(w, []) It looks like the code it duplicated ...
1 year, 8 months ago #9
storchaka_gmail.com
http://bugs.python.org/review/25994/diff/16550/Lib/test/test_os.py File Lib/test/test_os.py (right): http://bugs.python.org/review/25994/diff/16550/Lib/test/test_os.py#newcode3097 Lib/test/test_os.py:3097: self.assertEqual(w, []) On 2016/02/10 10:25:12, haypo wrote: > It ...
1 year, 8 months ago #10
Jim.J.Jewett
just English usage nits http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst File Doc/library/os.rst (right): http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst#newcode1902 Doc/library/os.rst:1902: collected, or when an error ...
1 year, 8 months ago #11
Martin Panter
http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst File Doc/library/os.rst (right): http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst#newcode1934 Doc/library/os.rst:1934: :func:`~scandir.close()` method. If a :func:`scandir` iterator is not On ...
1 year, 8 months ago #12
storchaka_gmail.com
1 year, 8 months ago #13
Thanks Jim for your help.

http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst
File Doc/library/os.rst (right):

http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst#newcode1902
Doc/library/os.rst:1902: collected, or when an error happened during iterating. 
However it
On 2016/02/10 23:20:41, Jim.J.Jewett wrote:
> happened -> happens

Done.

http://bugs.python.org/review/25994/diff/16550/Doc/library/os.rst#newcode1934
Doc/library/os.rst:1934: :func:`~scandir.close()` method.  If a :func:`scandir`
iterator is not
On 2016/02/10 23:20:41, Jim.J.Jewett wrote:
> "is not ... nor"  ->
> "is neither ... nor"

Done.

http://bugs.python.org/review/25994/diff/16550/Doc/whatsnew/3.6.rst
File Doc/whatsnew/3.6.rst (right):

http://bugs.python.org/review/25994/diff/16550/Doc/whatsnew/3.6.rst#newcode113
Doc/whatsnew/3.6.rst:113: iterator is not exhausted nor explicitly closed a
:exc:`ResourceWarning`
On 2016/02/10 23:20:41, Jim.J.Jewett wrote:
> "not ... nor" -> "neither ... nor"

Done.
Sign in to reply to this message.

RSS Feeds Recent Issues | This issue
This is Rietveld 894c83f36cb7