classification
Title: subprocess wait() hangs when stdin is closed
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Ben.Wolfson, haypo, idank, kiilerix, neologix, pitrou, python-dev, rosslagerwall
Priority: normal Keywords: needs review, patch

Created on 2011-08-19 16:31 by idank, last changed 2011-08-25 20:17 by neologix. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess_cloexec.diff neologix, 2011-08-19 22:34 review
subprocess_cloexec-1.diff neologix, 2011-08-24 20:38 review
Messages (11)
msg142475 - (view) Author: Idan Kamara (idank) Date: 2011-08-19 16:31
The following program hangs on Debian, Python 2.6.6:

import subprocess

proc1 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)
proc2 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)

proc1.stdin.close()
proc1.wait()

Changing the last two lines to:

proc2.stdin.close()
proc2.wait()

Doesn't hang. The guys at #python-dev confirmed the same happens on 2.7 but not on 3.x.
msg142483 - (view) Author: Ben Wolfson (Ben.Wolfson) Date: 2011-08-19 18:14
"The guys at #python-dev confirmed the same happens on 2.7 but not on 3.x."

Really? This is on gentoo, not debian, admittedly:

coelacanth ~ 11:12:36 $ python3
Python 3.1.3 (r313:86834, May  1 2011, 09:41:48) 
[GCC 4.4.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> proc1 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)
>>> proc2 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)
>>> proc2.stdin.close()
>>> proc2.wait()
0
>>> 



coelacanth ~ 11:12:13 $ python3.1
Python 3.1.3 (r313:86834, May  1 2011, 09:41:48) 
[GCC 4.4.4] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> proc1 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)
>>> proc2 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)
>>> proc1.stdin.close()
>>> proc1.wait()

[hangs]
msg142485 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-08-19 18:15
Hello Idan,

> The following program hangs on Debian

Debian is a good choice :-)

Concerning your example, it hangs is because the file descriptor used
to communicate with proc1 is inherited when proc2 is created (FDs are
inherited upon fork by default): thus, when you call
proc1.stdin.close(), `cat` doesn't receive EOF on read(0), and remains
stuck.
If you close proc2's stdin and wait it first, then you don't have this problem:
- because it's been created after proc1, its stdin FD has not been inherited
- when you call proc2.stdin.close(), `cat` receives EOF from read(0), and exits

There are two reasons while it doesn't hang on Python 3.x:
1) it uses close_fds=True by default
2) it sets the pipe FDs CLOEXEC, so that they're closed when cat is `executed`

Try with
"""
proc1 = subprocess.Popen(['cat'], stdin=subprocess.PIPE)
proc2 = subprocess.Popen(['cat'], stdin=subprocess.PIPE, close_fds=True)

proc1.stdin.close()
proc1.wait()
"""

And you'll be fine.

We can't make the change 1) in 2.7, because that's a behavior change.
We could however set the FDs used to communicate with the children CLOEXEC.
I'll work on a patch for 2.7 (but it won't be thread-safe, because
pipe2 is not exposed and there's no _posixsubprocess.c like in 3.x).
msg142501 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-08-19 22:34
Here's a patch + test for 2.7.

> Really? This is on gentoo, not debian, admittedly:

That's because the change of close_fds to True by default and the CLOEXEC flag were added in 3.2.
Since 3.1 is in security-fix mode only, this patch is only relevant to 2.7.
msg142559 - (view) Author: Idan Kamara (idank) Date: 2011-08-20 20:31
Thanks for getting on top of this so quickly Charles. Setting close_fds=True worked like a charm.
msg142914 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-24 20:19
You might want to add a comment in the patch that the cloexec flag is removed from the child's pipes by calling dup2() before exec() :) I was a bit baffled at first when reading the patch.
msg142917 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-08-24 20:38
With comment.
I'll add a similar comment to default.
msg142937 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-08-24 22:37
subprocess_cloexec-1.diff: I'm too tired too review the test. The subprocess.py part looks correct, except the pipe2 name. Python 3 uses "_create_pipe = _posixsubprocess.cloexec_pipe". Pick one of those.
msg142965 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-08-25 12:24
Patch looks good after Victor's comment.
msg142987 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-25 19:19
New changeset 498b03a55297 by Charles-François Natali in branch '2.7':
Issue #12786: Set communication pipes used by subprocess.Popen CLOEXEC to avoid
http://hg.python.org/cpython/rev/498b03a55297
msg142990 - (view) Author: Charles-François Natali (neologix) * (Python committer) Date: 2011-08-25 20:17
Patch committed.
Idan, thanks for the report.
History
Date User Action Args
2011-08-25 20:17:25neologixsetstatus: open -> closed
resolution: fixed
messages: + msg142990

stage: patch review -> resolved
2011-08-25 19:19:45python-devsetnosy: + python-dev
messages: + msg142987
2011-08-25 12:24:20rosslagerwallsetmessages: + msg142965
2011-08-24 22:37:28hayposetmessages: + msg142937
2011-08-24 20:38:35neologixsetfiles: + subprocess_cloexec-1.diff

messages: + msg142917
2011-08-24 20:19:54pitrousetmessages: + msg142914
2011-08-24 19:14:09neologixsetnosy: + pitrou, rosslagerwall
2011-08-20 20:31:46idanksetmessages: + msg142559
2011-08-19 22:34:56neologixsetfiles: + subprocess_cloexec.diff

versions: - Python 2.6
keywords: + patch, needs review
nosy: + haypo

messages: + msg142501
stage: patch review
2011-08-19 21:05:56kiilerixsetnosy: + kiilerix
2011-08-19 18:15:07neologixsetmessages: + msg142485
2011-08-19 18:14:14Ben.Wolfsonsetnosy: + Ben.Wolfson
messages: + msg142483
2011-08-19 16:41:52pitrousetnosy: + neologix
type: resource usage -> behavior
2011-08-19 16:31:01idankcreate