classification
Title: subprocess(stdout=..., stderr=sys.stdout) breaks stderr for child
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.2, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: rosslagerwall Nosy List: Evgeny.Tarasov, alexey-smirnov, chn, haypo, mmarkk, neologix, pitrou, python-dev, r.david.murray, rosslagerwall
Priority: normal Keywords: patch

Created on 2011-07-22 09:46 by chn, last changed 2011-07-27 19:42 by rosslagerwall. This issue is now closed.

Files
File name Uploaded Description Edit
i12607.patch rosslagerwall, 2011-07-22 11:26 review
i12607_v2.patch rosslagerwall, 2011-07-25 06:25 review
Messages (12)
msg140864 - (view) Author: Christian Häggström (chn) Date: 2011-07-22 09:46
I hit a variant of issue #12251, namely when you redirect both stdout and stderr of a child process and one of them uses a low fd.
Testcase:

import subprocess, sys
subprocess.call(["ls", "asda"], stderr = sys.stdout, stdout = open("/dev/null", "w"))

strace output:

open("/dev/null", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
[child process]
dup2(3, 1)   = 1  // overwrites the stdout filedescriptor..
dup2(1, 2)   = 2  // that was supposed to be duped here.
close(3)    = 0   // okay

The testcase is supposed to print "ls: file not found" on stdout, but is silent.
msg140866 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-22 10:11
> stdout = open("/dev/null", "w"), stderr = sys.stdout

You ask to write all outputs to /dev/null. Why do you expect anything on stdout?
msg140867 - (view) Author: Christian Häggström (chn) Date: 2011-07-22 10:18
I expect that 'ls' print the error message on its stderr, which would be redirected to stdout of the test Python program.

If I had been using stderr = subprocess.STDOUT, I can agree with you that both output streams would go to /dev/null.
msg140868 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-22 11:22
> stdout = open("/dev/null", "w"), stderr = sys.stdout

Oh, I read subprocess.STDOUT instead of sys.stdout.
msg140869 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-07-22 11:26
It is indeed a problem.

It seems like the problem comes about due to the "swapping" of fds.
i.e. using stdout as stderr. The reverse appears to work due to the order in which the dup() calls are performed.

Attached is a patch which fixes the issue.
msg140879 - (view) Author: Christian Häggström (chn) Date: 2011-07-22 13:57
Thanks for the patch, I haven't tried it (I'm still on Python 2.7) but it looks very special-cased to my case. I can think about exotic cases like

stdin = sys.stderr, stdout = sys.stdin, stderr = sys.stdout

It can happen in reality if a daemon have closed stdio (see bug 10806) and reusing fd's 0,1,2 for other purposes.
msg141072 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-07-25 06:25
Attached is a patch which tests all combinations and includes a testcase.
msg141080 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-25 09:51
save_fds should be moved outside the try block:

+            try:
+                # save a copy of the standard file descriptors
+                saved_fds = [os.dup(fd) for fd in range(3)]
+                ...
+            finally:
+                for std, saved in enumerate(saved_fds):
+                    os.dup2(saved, std)
+                    os.close(saved)


"temp_fds = [fd for fd, fname in temps]" should also be moved outside its try block.

Why not using .communicate() to write into stdin and read stdout/stderr?
msg141135 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-07-26 04:44
communicate() requires setting stdin, stdout and stderr to subprocess.PIPE which defeats the purpose of the test: setting them to 0, 1 and 2 (in various orders) so that they need to be swapped...
msg141189 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2011-07-26 22:05
i12607_v2.patch: the test fails if Python is compiled with pydebug. Add "err = support.strip_python_stderr(err)" before self.assertEqual(err, b"err") in test_subprocess.py to fix the failure.
msg141206 - (view) Author: Roundup Robot (python-dev) Date: 2011-07-27 05:47
New changeset 1140b32747c9 by Ross Lagerwall in branch '3.2':
Issue #12607: In subprocess, fix issue where if stdin, stdout or stderr is
http://hg.python.org/cpython/rev/1140b32747c9

New changeset 943d323cb4b8 by Ross Lagerwall in branch 'default':
Issue #12607: Merge with 3.2.
http://hg.python.org/cpython/rev/943d323cb4b8
msg141244 - (view) Author: Roundup Robot (python-dev) Date: 2011-07-27 16:55
New changeset 6119440bae30 by Ross Lagerwall in branch '2.7':
Issue #12607: In subprocess, fix issue where if stdin, stdout or stderr is
http://hg.python.org/cpython/rev/6119440bae30
History
Date User Action Args
2011-07-27 19:42:51rosslagerwallsetstatus: open -> closed
type: behavior
stage: resolved
resolution: fixed
assignee: rosslagerwall
2011-07-27 16:55:57python-devsetmessages: + msg141244
2011-07-27 05:47:16python-devsetnosy: + python-dev
messages: + msg141206
2011-07-26 22:05:41hayposetmessages: + msg141189
versions: - Python 2.6, Python 3.1, Python 3.4
2011-07-26 04:44:19rosslagerwallsetmessages: + msg141135
2011-07-25 09:51:07hayposetmessages: + msg141080
2011-07-25 06:25:10rosslagerwallsetfiles: + i12607_v2.patch
nosy: + pitrou
messages: + msg141072

2011-07-22 13:57:33chnsetmessages: + msg140879
2011-07-22 11:26:16rosslagerwallsetfiles: + i12607.patch

nosy: + rosslagerwall
messages: + msg140869

keywords: + patch
2011-07-22 11:22:00hayposetmessages: + msg140868
2011-07-22 10:18:34chnsetmessages: + msg140867
2011-07-22 10:11:22hayposetmessages: + msg140866
2011-07-22 09:46:24chncreate