The attached patch fixes some bugs in the pty.py module:

  - spawn() would not wait for the invoked process to finish.
    Also, it did not return a meaningful value, so there was no way
    to tell if the invoked process failed.    After this patch,
    spawn() now waits for the invoked process, and returns the value
    returned from os.waitpid().

  - There was a bug in the _copy() loop that caused it to spin
    using 100% CPU rather than blocking after EOF was reached on
    one of the two file descriptors that it was waiting for.

Hi Fergus,

I would suggest using "if not data" to check for EOF

On Tue, Mar 25, 2008 at 9:22 PM, Guilherme Polo <report@bugs.python.org> wrote:
>
>  I would suggest using "if not data" to check for EOF

Good idea.

Fergus,
Can you provide a test for the _copy loop bug? IIUC, the spawn change is
an RFE and shouldn't land on the maintenance branches (or 3.1).

The spawn change (the last hunk of the original patch) is a bug fix, not
an RFE.
It has two parts that fix two bugs:
  (1) a coding bug: spawn() would not wait for the invoked process to
finish.  This is fixed by the line that adds the call to os.waitpid().
  (2) a design bug: because previously spawn() didn't return a value,
there is no way to tell if the invoked process failed.  This is fixed by
the "return status" line.

Now I guess you can argue that (2) is an RFE.
But (1) is just a bug fix, not an RFE, IMHO.

Those are both separate from the other bug fixed in the patch:
  (3) Another coding bug: the bug in the _copy() loop that caused
    it to spin using 100% CPU rather than blocking

It's a little tricky to write a test of the _copy() loop bug, for
several reasons.
(a) There currently isn't any test for pty.spawn, apparently since
"Cannot do extensive 'do or fail' testing because pty code is not too
portable."
(b) Also, for this bug the symptom is just that the code spins (using
100% CPU, if available) rather than blocking.  It's difficult to detect
that situation using portable code.

I can maybe figure out how deal with (a), but I'm not sure how to
address (b), especially since I don't know the intended portability goals.

I will split the patch up into two patches, one of which addresses
(1)+(2), and the other of which addresses (3).

I have addressed Guilherme Polo's suggestion about using "if not data".

Woops, didn't mean to delete that file.  Reattaching.

That didn't work so well :(

OK, file restored.

Design bugs are usually fixed by "feature requests" :)

See issue 967171 for the feature request.

New changeset 994659efa292 by Gregory P. Smith in branch '3.2':
Issue #2489: Fix bug in _copy loop that could consume 100% cpu on EOF.
http://hg.python.org/cpython/rev/994659efa292

New changeset c7338f62f956 by Gregory P. Smith in branch 'default':
Issue #2489: Fix bug in _copy loop that could consume 100% cpu on EOF.
http://hg.python.org/cpython/rev/c7338f62f956

New changeset f889458c65cc by Gregory P. Smith in branch '2.7':
Issue #2489: Fix bug in _copy loop that could consume 100% cpu on EOF.
http://hg.python.org/cpython/rev/f889458c65cc

I'm keeping this open to address the added behavior for spawn in 3.3.

New changeset ec2921d4de37 by Gregory P. Smith in branch 'default':
pty.spawn() now returns the child process status as returned by os.waitpid().
http://hg.python.org/cpython/rev/ec2921d4de37

The test breaks on OpenIndiana (and possibly elsewhere):
http://buildbot.python.org/all/builders/x86%20OpenIndiana%203.x/builds/4640

======================================================================
FAIL: test_spawn_returns_status (test.test_pty.PtyTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/test/test_pty.py", line 200, in test_spawn_returns_status
    status = pty.spawn([sys.executable, '-c', 'import sys; sys.exit(0)'])
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/pty.py", line 175, in spawn
    _copy(master_fd, master_read, stdin_read)
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/pty.py", line 147, in _copy
    rfds, wfds, xfds = select(fds, [], [])
  File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/test/test_pty.py", line 65, in handle_sig
    self.fail("isatty hung")
AssertionError: isatty hung