classification
Title: subprocess fails in select when descriptors are large
Type: behavior Stage: commit review
Components: Library (Lib) Versions: Python 3.1
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: gregory.p.smith Nosy List: amaury.forgeotdarc, benjamin.peterson, d_kagedal, fpmc, gregory.p.smith, loewis, mkc, yorick
Priority: release blocker Keywords: patch

Created on 2008-07-17 11:25 by yorick, last changed 2009-08-13 18:52 by benjamin.peterson. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess_with_poll.diff fpmc, 2009-03-31 08:13
subprocess_with_poll3.diff fpmc, 2009-03-31 23:33
subprocess_with_poll3-gps03.diff gregory.p.smith, 2009-07-04 01:51
unnamed fpmc, 2009-07-04 09:37
Messages (16)
msg69880 - (view) Author: Mattias Engdegård (yorick) Date: 2008-07-17 11:25
If the stdin/out file descriptors are too large to be used with
select(), subprocess will fail in .communicate(). Example:

# raise the fd limit to something like 2048 before running this
import subprocess
somefiles = [open("/etc/passwd") for i in xrange(2000)]
print subprocess.Popen(["date"], stdout=subprocess.PIPE).communicate()

The solution would be to use select.poll() in subprocess instead.
msg73585 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2008-09-22 16:34
I don't understand the problem. If you open a lot of files, the open() 
loop will stop an exception (IOError: too many open files), and so 
subprocess is not used.

If I open <limit>-1 files, subprocess._get_handles() will fail on 
os.pipe().

If I open <limit>-4 files, the script doesn't fail.

I'm using Linux. Here is my script:
---
# open <limit>-4 files
import subprocess
files = []
while 1:
    try:
        newfile = open("/etc/passwd")
    except IOError, err:
        print "ERR: %s" % err
        break
    files.append(newfile)

# use -3 to get an error
for file in files[-4:]:
    print "close"
    file.close()

# success
subprocess.Popen(["date"], stdout=subprocess.PIPE).communicate()
---
msg73586 - (view) Author: Mattias Engdegård (yorick) Date: 2008-09-22 16:43
As the comment in the original bug report said, you need to raise the
file descriptor limit to something well above 2000 before running the
test case.

Needless to say, you may need to do that part as root in case that would
exceed your hard RLIMIT_NOFILE limit.
msg83469 - (view) Author: Mike Coleman (mkc) Date: 2009-03-11 15:42
I also ran into this bug.  

In my case I'm able to work around this by "reserving" some fds at the
start of my program (open 20 reserved fds, open 2000 real fds, close 20
reserved fds), but this is both a kludge and not a general solution.

Probably virtually all uses of 'select' (both in Python and C) should be
replaced under Linux with poll or epoll.

(Also, until/unless this is fixed, it'd be nice to mention it in the
'communicate' doc.)
msg84761 - (view) Author: Frank Chu (fpmc) Date: 2009-03-31 08:13
Hi,

This is a patch that uses poll() in subprocess.communicate() if it is
available.  This is my first patch and may contain style errors.  I try
to conform to PEP 8 as close as I can.

Besides the discussion here, I would like to add this is desired because
select() has its own limit on the file descriptor size, in FD_SETSIZE. 
Unix has a different limit on the largest file descriptor, in 'ulimit
-n'.  When the select() limit is smaller than the ulimit limit, it is
not hard to get a file descriptor that is beyond FD_SETSIZE.  In that
case select() simply does not work.  Since there is no precise way to
force file descriptor to be small, the only workaround seems to be hack
like what hte first poster yorick suggests.

This is tested via 'python regrtest.py -w test_subprocess' under a
system that supports poll().  I don't have a system that does *not*
support poll, but I've done some manual edits to force that code path
and test it as well.  The test passes, which I think should be good
enough as it has several tests on communicate().

This patch is against 2.6.
msg84794 - (view) Author: Mattias Engdegård (yorick) Date: 2009-03-31 15:00
The patch looks all right in general. I would use something like

if "poll" in dir(select)

instead of catching AttributeError which risks hiding bugs in 
_communicate_with_poll().

PEP8 probably wants spaces around the bitwise-or operator.

Some systems cannot use TTYs in poll(2) but this should not be a 
problem here - there is no point in using .communicate() with stdin/
stdout/stderr set to anything but PIPE, right?
msg84807 - (view) Author: Mike Coleman (mkc) Date: 2009-03-31 15:48
My original report didn't mention it specifically, but I believe I was
hitting the FD_SETSIZE limit that Frank mentioned.  (Thanks for working
on this!)
msg84948 - (view) Author: Frank Chu (fpmc) Date: 2009-03-31 23:33
Updated with new patch.  I moved the try: except logic in the module
initialization step so I get a global has_poll, similar to the global
mswindows already there.

I now use a try/except/else to be more robust (not needed now since it's
only "has_poll = True", but I think it's better code.
msg89739 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2009-06-26 19:42
The patch looks good to me, but I don't use Linux regularly. Another review 
is needed!
msg90094 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-07-04 01:51
Your patch looks pretty good.  I cleaned up a few things in it and added
a unittest (the existing test suite is run with both select and poll).

Committed to trunk in r73825.

I am leaving the issue open until this is ported and merged into py3k as
well as backported to release26-maint and release31-maint.
msg90099 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-07-04 02:47
Merged to py3k in r73833.
msg90110 - (view) Author: Frank Chu (fpmc) Date: 2009-07-04 09:37
Thanks!  Good to hear it's checked in finally :-).

Frank

On Fri, Jul 3, 2009 at 7:47 PM, Gregory P. Smith <report@bugs.python.org>wrote:

>
> Gregory P. Smith <greg@krypto.org> added the comment:
>
> Merged to py3k in r73833.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue3392>
> _______________________________________
>
msg91524 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2009-08-13 17:42
Unfortunately, this change has broken windows support of the subprocess
module, which now reports, on "import subprocess".

>>> import subprocess
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\loewis\31\python\lib\subprocess.py", line 390, in <module>
    _PIPE_BUF = getattr(select, 'PIPE_BUF', 512)
NameError: name 'select' is not defined

As a consequence, even building Python on Windows now fails, so setting
the priority to "release blocker". One solution would be to move the
"import select" to being top-level.
msg91525 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-08-13 17:58
r73916 appears to fix this?
msg91529 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2009-08-13 18:34
merged into release31-maint in r74425.

reopen the issue if this doesn't fix the problem.
msg91531 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-08-13 18:52
I will retag 3.1.1rc then...
History
Date User Action Args
2009-08-13 18:52:23benjamin.petersonsetmessages: + msg91531
2009-08-13 18:34:08gregory.p.smithsetstatus: open -> closed

messages: + msg91529
versions: - Python 2.6
2009-08-13 17:58:52gregory.p.smithsetmessages: + msg91525
2009-08-13 17:42:20loewissetpriority: normal -> release blocker
nosy: + loewis, benjamin.peterson
messages: + msg91524

2009-07-04 09:37:00fpmcsetfiles: + unnamed

messages: + msg90110
2009-07-04 02:47:22gregory.p.smithsetmessages: + msg90099
2009-07-04 01:51:56gregory.p.smithsetfiles: + subprocess_with_poll3-gps03.diff
versions: + Python 3.1, - Python 3.0
messages: + msg90094

keywords: - needs review
resolution: accepted
stage: patch review -> commit review
2009-06-26 19:42:36amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg89739

keywords: + needs review
stage: patch review
2009-03-31 23:33:35fpmcsetfiles: + subprocess_with_poll3.diff

messages: + msg84948
2009-03-31 15:48:08mkcsetmessages: + msg84807
2009-03-31 15:00:47yoricksetmessages: + msg84794
2009-03-31 08:13:24fpmcsetfiles: + subprocess_with_poll.diff

nosy: + fpmc
messages: + msg84761

keywords: + patch
2009-03-24 23:09:56vstinnersetnosy: - vstinner
2009-03-11 15:42:03mkcsetnosy: + mkc
messages: + msg83469
2008-09-22 16:43:49yoricksetmessages: + msg73586
2008-09-22 16:34:04vstinnersetnosy: + vstinner
messages: + msg73585
2008-09-22 00:54:07gregory.p.smithsetassignee: gregory.p.smith
nosy: + gregory.p.smith
2008-08-15 10:14:25pitrousetpriority: normal
type: behavior
versions: + Python 2.6, Python 3.0, - Python 2.5
2008-08-01 08:11:21d_kagedalsetnosy: + d_kagedal
2008-07-17 11:25:39yorickcreate