classification
Title: PyFile_FromString leaks file descriptors in python 2.7
Type: resource usage Stage: resolved
Components: Interpreter Core Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: doko, pitrou, python-dev, vstinner
Priority: high Keywords:

Created on 2012-04-05 09:01 by doko, last changed 2012-04-05 17:25 by vstinner. This issue is now closed.

Messages (8)
msg157555 - (view) Author: Matthias Klose (doko) * (Python committer) Date: 2012-04-05 09:01
[forwarded from http://bugs.debian.org/664529]

seen with 2.7.3 rc2

File descriptors opened by PyFile_FromString don't get closed when the
reference count is decreased.

Here's my test program, pythony.c:

#include <Python.h>

int main()
{
  int i = 0;
  PyObject *obj;

  Py_Initialize();
  while (i++ < 5) {
    obj = PyFile_FromString("hello.py", "r");
    assert(obj);
    Py_DECREF(obj);
  }
  Py_Finalize();
}

hello.py is 'print("hello world")'.

I'm compiling it with both Python 2.6 and 2.7.
$ gcc pythony.c -lpython2.6 -L/usr/lib/python2.6/config -I/usr/include/python2.6/ -o pythony-2.6
$ gcc pythony.c -lpython2.7 -L/usr/lib/python2.7/config -I/usr/include/python2.7/ -o pythony-2.7

$ strace ./pythony-2.6 2>&1 | tail -n 20
ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B38400 opost isig icanon echo ...}) = 0
ioctl(1, SNDCTL_TMR_TIMEBASE or TCGETS, 0x7fffb1d097b0) = -1 EINVAL (Invalid argument)
ioctl(2, SNDCTL_TMR_TIMEBASE or TCGETS, 0x7fffb1d097b0) = -1 EINVAL (Invalid argument)
open("hello.py", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
close(3)                                = 0
open("hello.py", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
close(3)                                = 0
open("hello.py", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
close(3)                                = 0
open("hello.py", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
close(3)                                = 0
open("hello.py", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
close(3)                                = 0
rt_sigaction(SIGINT, {SIG_DFL, [], SA_RESTORER, 0x7f1e1a0224f0}, {0x7f1e1a49a160, [], SA_RESTORER, 0x7f1e1a0224f0}, 8) = 0
exit_group(0)                           = ?


$ strace ./pythony-2.7 2>&1 | tail -n 20
fstat(4, {st_mode=S_IFREG|0644, st_size=1950, ...}) = 0
read(4, "", 4096)                       = 0
close(4)                                = 0
munmap(0x7fa41f10f000, 4096)            = 0
close(3)                                = 0
ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B38400 opost isig icanon echo ...}) = 0
ioctl(1, SNDCTL_TMR_TIMEBASE or TCGETS, 0x7ffff7bd33f0) = -1 EINVAL (Invalid argument)
ioctl(2, SNDCTL_TMR_TIMEBASE or TCGETS, 0x7ffff7bd33f0) = -1 EINVAL (Invalid argument)
open("hello.py", O_RDONLY)              = 3
fstat(3, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
open("hello.py", O_RDONLY)              = 4
fstat(4, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
open("hello.py", O_RDONLY)              = 5
fstat(5, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
open("hello.py", O_RDONLY)              = 6
fstat(6, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
open("hello.py", O_RDONLY)              = 7
fstat(7, {st_mode=S_IFREG|0644, st_size=21, ...}) = 0
rt_sigaction(SIGINT, {SIG_DFL, [], SA_RESTORER, 0x7fa4206e24f0}, {0x7fa420b8dd50, [], SA_RESTORER, 0x7fa4206e24f0}, 8) = 0
exit_group(0)                           = ?


The Python 2.7 version never calls close, not even at Py_Finalize().

On #d-d, jwilk suspected that this change might be the cause:
http://hg.python.org/cpython/rev/0f5b64630fda/#l4.46
msg157559 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-05 10:30
> File descriptors opened by PyFile_FromString don't get closed
> when the reference count is decreased.

Correct, PyFile_FromString() doesn't close the file descriptor, even if you call its close() method. You have to call manually fclose(file->f_fp).

Or you can use PyFile_FromFile:

fp = fopen(name, mode);
if (fp == NULL) ...
obj = PyFile_FromFile(fp, name, mode, fclose);
if (obj == NULL) {
   /* no need to call fclose(fp) here, it's done by PyFile_FromFile() */
   ...
}
...
Py_DECREF(obj);

Would you like to write a patch for the documentation?
msg157560 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-05 10:41
Victor, that sounds like a strange behaviour to me. PyFile_FromString is a public API and maybe it shouldn't have changed between 2.6 and 2.7.
msg157561 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-05 11:20
> Victor, that sounds like a strange behaviour to me.

We may change PyFile_FromString() to call fclose() when the file is
closed explicitly (call its close() method), but it may break backward
compatibility. I prefer to document the behaviour instead.

> PyFile_FromString is a public API and maybe it shouldn't
> have changed between 2.6 and 2.7.

PyFile_FromString() didn't change in Python 2.7.

I changed PyFile_FromFile() in Python 2.7 to fix the issue #7732.

changeset:   72456:0f5b64630fda
branch:      2.7
parent:      72450:c02e790c4535
user:        Victor Stinner <victor.stinner@haypocalc.com>
date:        Fri Sep 23 19:37:03 2011 +0200
files:       Doc/c-api/file.rst Lib/test/test_import.py Misc/NEWS
Objects/fileobject.c Python/import.c
description:
Issue #7732: Fix a crash on importing a module if a directory has the same name
than a Python module (e.g. "__init__.py"): don't close the file twice.

PyFile_FromFile() does also close the file if PyString_FromString() failed. It
did already close the file on fill_file_fields() error (e.g. if the file is a
directory).
msg157563 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-05 11:27
Victor, what exactly are you talking about? http://hg.python.org/cpython/rev/0f5b64630fda/ *does* change PyFile_FromString.
And Matthias mentioned that the problem didn't occur with 2.6.
msg157574 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2012-04-05 12:13
New changeset 8258e5fa4a19 by Antoine Pitrou in branch '2.7':
Issue #14505: Fix file descriptor leak when deallocating file objects created with PyFile_FromString().
http://hg.python.org/cpython/rev/8258e5fa4a19
msg157575 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2012-04-05 12:16
Matthias, this should be fixed now.
msg157607 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2012-04-05 17:25
> Victor, what exactly are you talking about? http://hg.python.org/cpython/rev/0f5b64630fda/ *does* change PyFile_FromString.

Oh. I don't know (remember) why I did this change!? I suppose that I
changed it to test something and then forget to remove the temporary
hack :-/
History
Date User Action Args
2012-04-05 17:25:05vstinnersetmessages: + msg157607
2012-04-05 12:16:26pitrousetstatus: open -> closed
type: resource usage
messages: + msg157575

resolution: fixed
stage: resolved
2012-04-05 12:13:21python-devsetnosy: + python-dev
messages: + msg157574
2012-04-05 11:27:12pitrousetmessages: + msg157563
2012-04-05 11:20:07vstinnersetmessages: + msg157561
2012-04-05 10:41:49pitrousetmessages: + msg157560
2012-04-05 10:30:12vstinnersetnosy: + pitrou
messages: + msg157559
2012-04-05 09:01:54dokosetnosy: + vstinner
2012-04-05 09:01:22dokocreate