classification
Title: os.scandir(str) reference leak (test_os refleak)
Type: resource usage Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: python-dev, serhiy.storchaka, vstinner, xiang.zhang
Priority: normal Keywords: patch

Created on 2017-02-09 12:09 by vstinner, last changed 2017-02-09 21:16 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
scandir-refleak-3.6.patch serhiy.storchaka, 2017-02-09 13:30 review
scandir-refleak-3.7.patch serhiy.storchaka, 2017-02-09 13:48 review
Messages (10)
msg287409 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-02-09 12:09
The following code leaks one reference on Python 3.6:

    def test_attributes(self):
        d = dict((entry.name, entry) for entry in os.scandir('.'))

Or try the command:

   ./python -m test -R 3:3 test_os -v -m test_attributes
msg287415 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-09 13:30
The owning of references in the path_t structure was changed in issue29034, but some code still directly manipulates reference counters of path_t fields. Proposed patch should fix the leak.
msg287416 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-09 13:48
3.7 doesn't leak, but contains outdated comment and code.
msg287420 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-09 14:52
3.6 LGTM. 3.7 is technically right to me. But it looks to me AC shouldn't call path_cleanup in .c.h. It always do nothing.
msg287422 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-02-09 15:02
If PyObject_New(ScandirIterator, &ScandirIteratorType) fails the path should be cleaned up by Argument Clinic.
msg287423 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-02-09 15:03
scandir-refleak-3.6.patch LGTM. The following removed Py_CLEAR() is just redundant, so it's ok to remove it.

-    Py_CLEAR(iterator->path.object);
msg287424 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-09 15:04
> If PyObject_New(ScandirIterator, &ScandirIteratorType) fails the path should be cleaned up by Argument Clinic.

Ohh yes. My stupid. Then both LGTM.
msg287447 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-02-09 18:07
New changeset 4e3a16bdadae by Serhiy Storchaka in branch '3.6':
Issue #29513: Fixed a reference leak in os.scandir() added in issue #29034.
https://hg.python.org/cpython/rev/4e3a16bdadae

New changeset a3f8c5d172b4 by Serhiy Storchaka in branch 'default':
Issue #29513: Fix outdated comment and remove redundand code is os.scandir().
https://hg.python.org/cpython/rev/a3f8c5d172b4
msg287451 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-02-09 19:00
New changeset be85fd4ef41979dbe68262938da12328fb6cfb8c by Serhiy Storchaka in branch '3.6':
Issue #29513: Fixed a reference leak in os.scandir() added in issue #29034.
https://github.com/python/cpython/commit/be85fd4ef41979dbe68262938da12328fb6cfb8c
msg287456 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-02-09 21:16
Thanks for the quick fix Serhiy! I was working on patch and I like to check
for refleaks. Sadly, it wasn't noticed before. At least my test was pass :-)
History
Date User Action Args
2017-02-09 21:16:00vstinnersetmessages: + msg287456
2017-02-09 19:00:22python-devsetmessages: + msg287451
2017-02-09 18:08:13serhiy.storchakasetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2017-02-09 18:07:38python-devsetnosy: + python-dev
messages: + msg287447
2017-02-09 15:04:56xiang.zhangsetmessages: + msg287424
stage: patch review -> commit review
2017-02-09 15:03:09vstinnersetmessages: + msg287423
2017-02-09 15:02:43serhiy.storchakasetassignee: serhiy.storchaka
messages: + msg287422
2017-02-09 14:52:02xiang.zhangsetmessages: + msg287420
2017-02-09 13:48:44serhiy.storchakasetfiles: + scandir-refleak-3.7.patch

messages: + msg287416
versions: + Python 3.7
2017-02-09 13:30:42serhiy.storchakasetfiles: + scandir-refleak-3.6.patch

nosy: + xiang.zhang
messages: + msg287415

keywords: + patch
stage: patch review
2017-02-09 12:09:14vstinnercreate