classification
Title: Subprocess error if fds 0,1,2 are closed
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: georg.brandl, giampaolo.rodola, gregory.p.smith, pitrou, rosslagerwall, vstinner
Priority: normal Keywords: patch

Created on 2011-01-02 18:08 by rosslagerwall, last changed 2011-01-03 18:47 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess.patch rosslagerwall, 2011-01-02 18:08 Patch + test for issue
subprocess_v2.patch rosslagerwall, 2011-01-02 19:55 Updated patch for debug mode
sp.patch pitrou, 2011-01-02 20:46
sp2.patch pitrou, 2011-01-02 21:19
sp3.patch pitrou, 2011-01-03 15:07
Messages (9)
msg125067 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-02 18:08
There is an issue where if a python program closes all the std. file descriptors (e.g. a daemon) and then uses the subprocess module, the file descriptors may not be set up properly in the subprocess. This may actually be a fairly common use case in a daemon program that needs to run a subprocess and set up pipes to it.

Here is an example:

import os, subprocess, sys

x=os.open('/dev/null', os.O_RDWR)
os.close(0)
os.close(1)
os.close(2)

res = subprocess.Popen([sys.executable, "-c",
                      'import sys;'
                      'sys.stdout.write("apple");'
                      'sys.stdout.flush();'
                      'sys.stderr.write("orange")'],
           stdin=x,
           stdout=subprocess.PIPE,
           stderr=subprocess.PIPE).communicate()
with open('/tmp/out', 'w') as f:
    f.write(repr(res) + '\n')
    f.write(repr((b'apple', b'orange')) + '\n')

The expected output in /tmp/out is:
('apple', 'orange')
('apple', 'orange')

but we get:
(b'', b"Fatal Python error: Py_Initialize: can't initialize sys standard streams\nOSError: [Errno 9] Bad file descriptor\n")
(b'apple', b'orange')

The problem comes about where the calls are made (this applies to the python & c versions):
 os.dup2(p2cread, 0)
 os.dup2(c2pwrite, 1)
 os.dup2(errwrite, 2)

if c2pwrite or p2cread or errwrite is the same as what it's being dupped() to (eg if c2pwrite == 1) then the dup2 call does nothing. But, if we're using pipes, the close-on-exec flags are set initially and the dup2() call would normally remove the flag but it doesn't.

Attached is a patch which basically uses fcntl if necessary to remove the close-on-exec flag, and tests.
msg125069 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-02 18:27
> Attached is a patch which basically uses fcntl if necessary to remove
> the close-on-exec flag, and tests.

Python adds extra output at the end of stderr when compiled in debug
mode. Therefore you first have to strip that output, like this:

            out, err = subprocess.Popen(...).communicate()
            err = support.strip_python_stderr(err)
            self.assertEqual((out, err), (b'apple', b'orange'))

Otherwise, looks good, thank you.
msg125083 - (view) Author: Ross Lagerwall (rosslagerwall) (Python committer) Date: 2011-01-02 19:55
Updated patch for debug mode. Does this also need to be applied for 3.1?
msg125090 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-02 20:15
> Updated patch for debug mode. Does this also need to be applied for 3.1?

Yes, but we can port it ourselves (unless you're really motivated)
msg125094 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-02 20:46
Patch works fine, thank you. Here is an attempt at a slightly more readable code by refactoring.
msg125101 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-02 21:19
This further patch also addresses issue9905 (incorporating Ross' tests).
msg125107 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2011-01-02 21:32
See also #6610 which has a patch with another test.
msg125179 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 15:07
This new patch makes tests more comprehensive (closes all combinations of the three standard fds).
msg125211 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-03 18:47
Committed in r87695 (3.2), r87696 (3.1) and r87697 (2.7).
History
Date User Action Args
2011-06-03 11:55:07neologixlinkissue12251 superseder
2011-01-03 18:47:38pitroulinkissue9905 superseder
2011-01-03 18:47:19pitrousetstatus: open -> closed
versions: + Python 3.1, Python 2.7
nosy: georg.brandl, gregory.p.smith, pitrou, vstinner, giampaolo.rodola, rosslagerwall
messages: + msg125211

resolution: fixed
stage: resolved
2011-01-03 18:31:05georg.brandllinkissue6610 superseder
2011-01-03 15:07:38pitrousetfiles: + sp3.patch
nosy: georg.brandl, gregory.p.smith, pitrou, vstinner, giampaolo.rodola, rosslagerwall
messages: + msg125179
2011-01-02 21:32:05vstinnersetnosy: + vstinner
messages: + msg125107
2011-01-02 21:19:37pitrousetfiles: + sp2.patch
nosy: georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125101
2011-01-02 20:46:17pitrousetfiles: + sp.patch
nosy: georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125094
2011-01-02 20:15:17pitrousetnosy: georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125090
2011-01-02 19:55:31rosslagerwallsetfiles: + subprocess_v2.patch
nosy: georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125083
2011-01-02 18:27:12pitrousetnosy: georg.brandl, gregory.p.smith, pitrou, giampaolo.rodola, rosslagerwall
messages: + msg125069
2011-01-02 18:08:35rosslagerwallcreate