classification
Title: PyRun_SimpleFileExFlags() can leak a FILE pointer
Type: resource usage Stage: resolved
Components: Versions: Python 3.4, Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: georg.brandl Nosy List: brett.cannon, christian.heimes, georg.brandl, haypo, jcea, python-dev
Priority: release blocker Keywords: 3.3regression, patch

Created on 2012-09-09 22:50 by christian.heimes, last changed 2012-09-24 05:46 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
closeown.patch christian.heimes, 2012-09-09 22:50 review
Messages (12)
msg170142 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-09 22:50
In Python/pythonrun.c PyRun_SimpleFileExFlags() may leak a FILE pointer if closeit is False and maybe_pyc_file returns true. This happens when the filename ends with ".pyc" or ".pyc". In this case the file is opened a second time with fopen() a few lines lower and assigned to the same variable "fp". However "fp" is never closed.

The bug was found by Coverity as CID 719689.

Georg: I've set the priority to release blocker as a resource leak of a file pointer and therefore rare file descriptor is IMHO a severe issue.
msg170149 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2012-09-10 02:24
LGTM.

Instead of using a flag variable, could you use something like "if (fp!=NULL) fclose(fp);"?
msg170169 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-10 09:31
No, that's not possible. 'fp' is part of the argument to PyRun_SimpleFileExFlags() and never NULL. We need some way to distinguish a externally provided fp from a self-opened fp and fclose(fp) in the latter case.
msg170208 - (view) Author: Jesús Cea Avión (jcea) * (Python committer) Date: 2012-09-10 17:58
Look good to me, then. Are you committing the patch yourself?.

You can patch 2.7, 3.2 and default, and bug Georg for 3.3.0 inclusion (patches to default will be in 3.3.1), unless Georg is notified and chooses to cherry pick it.
msg170296 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-11 12:12
New changeset 4754c4a710e6 by Christian Heimes in branch 'default':
Issue #15895: Fix FILE pointer leak in PyRun_SimpleFileExFlags() when filename points to a pyc/pyo file and closeit is false.
http://hg.python.org/cpython/rev/4754c4a710e6
msg170298 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2012-09-11 12:15
The bug was a 3.3 regression, possibly introduced with the merge of importlib. I forgot to tick the 3,3regression keyword.

I've applied the fix. I'm leaving this ticket open and assigned to Georg for 3.3.0.
msg170306 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-11 13:47
New changeset c3539eb02470 by Christian Heimes in branch 'default':
Issue #15895: my analysis was slightly off. The FILE pointer is only leaked when set_main_loader() fails for a pyc file with closeit=0. In the success case run_pyc_file() does its own cleanup of the fp. I've changed the code to use another FILE ptr for pyc files and moved the fclose() to PyRun_SimpleFileExFlags() to make it more obvious what's happening.
http://hg.python.org/cpython/rev/c3539eb02470
msg170331 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-11 17:28
New changeset 6fea947edead by Christian Heimes in branch 'default':
Updates NEWS for issue #15895
http://hg.python.org/cpython/rev/6fea947edead
msg170932 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-09-21 21:52
This issue is not a regression, I think that the fix can wait for Python 3.3.1.
msg170933 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2012-09-21 21:56
> This issue is not a regression

My bad, Crys explained me on IRC that the leak does not exist in Python 3.2, it was introduced by the importlib. So it is a regression.
msg170958 - (view) Author: Georg Brandl (georg.brandl) * (Python committer) Date: 2012-09-22 06:53
Now transplanted to release clone.
msg171094 - (view) Author: Roundup Robot (python-dev) Date: 2012-09-24 05:46
New changeset fe4e6e0d6b0f by Christian Heimes in branch 'default':
Issue #15895: Fix FILE pointer leak in PyRun_SimpleFileExFlags() when filename points to a pyc/pyo file and closeit is false.
http://hg.python.org/cpython/rev/fe4e6e0d6b0f

New changeset 9251fb0440a0 by Christian Heimes in branch 'default':
Issue #15895: my analysis was slightly off. The FILE pointer is only leaked when set_main_loader() fails for a pyc file with closeit=0. In the success case run_pyc_file() does its own cleanup of the fp. I've changed the code to use another FILE ptr for pyc files and moved the fclose() to PyRun_SimpleFileExFlags() to make it more obvious what's happening.
http://hg.python.org/cpython/rev/9251fb0440a0

New changeset 61b1c25d55f2 by Christian Heimes in branch 'default':
Updates NEWS for issue #15895
http://hg.python.org/cpython/rev/61b1c25d55f2
History
Date User Action Args
2012-09-24 05:46:45python-devsetmessages: + msg171094
2012-09-22 06:53:35georg.brandlsetstatus: open -> closed

messages: + msg170958
2012-09-21 21:56:10hayposetmessages: + msg170933
2012-09-21 21:52:45hayposetnosy: + haypo
messages: + msg170932
2012-09-11 17:28:52python-devsetmessages: + msg170331
2012-09-11 13:47:38python-devsetmessages: + msg170306
2012-09-11 12:15:10christian.heimessetversions: + Python 3.3, Python 3.4
messages: + msg170298

keywords: + 3.3regression
resolution: fixed
stage: patch review -> resolved
2012-09-11 12:12:32python-devsetnosy: + python-dev
messages: + msg170296
2012-09-10 17:58:01jceasetmessages: + msg170208
2012-09-10 09:31:34christian.heimessetmessages: + msg170169
2012-09-10 02:24:49jceasetnosy: + jcea
messages: + msg170149
2012-09-09 22:50:50christian.heimescreate