[New-bugs-announce] [issue16140] subprocess.Popen the os.close calls in _execute_child can raise an EBADF exception

[Gregory P. Smith]

New submission from Gregory P. Smith:

Ben Leslie writes this on python-dev:

Hi all,

I have a Python program where I have many threads each calling Popen, and I was hitting some trouble.

I've been seeing this on 3.2.3, however I believe the same issue is still potentially a problem on head.

The error manifests itself when a call to os.close(errpipe_read) fails with EBADF (http://hg.python.org/releasing/3.2.3/file/86d1421a552c/Lib/subprocess.py#l1314)

I believe the root cause of this problem is due to a double close() on a different file descriptor (which is then reused as errpipe_read).

The file descriptors: p2cwrite, c2pread and errread are all closed at the end of the _execute_child method:

http://hg.python.org/releasing/3.2.3/file/86d1421a552c/Lib/subprocess.py#l1351

However, these filedescriptors are wrapped up into file objects during __init__ (see: http://hg.python.org/releasing/3.2.3/file/86d1421a552c/Lib/subprocess.py#l725)

As far as I can tell at the point where the garbage collector kicks in  Popen.{stdin,stdout,stderr} all go out of scope, and will be deallocated, and the underlying filedescriptor closed.

However because the filedescriptor is already closed, and by this time is actually reused, this deallocation closes what ends up being an incorrect file-descriptor.

Since closing a file object where the underlying fd is already closed is silenced (http://hg.python.org/releasing/3.2.3/file/86d1421a552c/Modules/_io/iobase.c#l235) this would not normally be very apparent.

This race between a new filedescriptor being allocated and the garbage collector deallocating the file descriptors certainly hits when using a few threads, but I guess depending on the exact behaviour of the garbage collector it could potentially also occur in a single threaded case as well.

I think a fix would be to remove the explicit close of these file descriptors at the end of _execute_child, and let the garbage collector close them. Of course that may leak file descriptors, if the GC doesn't kick in for a while, so the other option would be to close the file object, rather than just the file descriptor.

Hopefully someone more intimately familiar with the module can point me in the right direction to verify this, and provide a fix.

Thanks,

Benno

----------
assignee: gregory.p.smith
messages: 172053
nosy: gregory.p.smith
priority: high
severity: normal
stage: test needed
status: open
title: subprocess.Popen the os.close calls in _execute_child can raise an EBADF exception
type: behavior
versions: Python 3.2, Python 3.3, Python 3.4

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue16140>
_______________________________________