| msg64533 - (view) |
Author: Fergus Henderson (fergushenderson) |
Date: 2008-03-26 01:15 |
| 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. |
|
|
| msg64534 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2008-03-26 01:22 |
| Hi Fergus, I would suggest using "if not data" to check for EOF |
|
|
| msg64535 - (view) |
Author: Fergus Henderson (fergushenderson) |
Date: 2008-03-26 01:46 |
| 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. |
|
|
| msg87914 - (view) |
Author: Daniel Diniz (ajaksu2) *  |
Date: 2009-05-16 19:38 |
| 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). |
|
|
| msg88683 - (view) |
Author: Fergus Henderson (fergushenderson) |
Date: 2009-06-01 22:51 |
| 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". |
|
|
| msg115120 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-08-27 19:35 |
| Woops, didn't mean to delete that file. Reattaching. |
|
|
| msg115121 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-08-27 19:38 |
| That didn't work so well :( |
|
|
| msg115123 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-08-27 19:48 |
| OK, file restored. Design bugs are usually fixed by "feature requests" :) See issue 967171 for the feature request. |
|
|
| msg153466 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-02-16 08:40 |
| 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 |
|
|
| msg153467 - (view) |
Author: Gregory P. Smith (gregory.p.smith) * |
Date: 2012-02-16 08:43 |
| I'm keeping this open to address the added behavior for spawn in 3.3. |
|
|
| msg171593 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2012-09-29 19:41 |
| 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 |
|
|
| msg171594 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2012-09-29 20:03 |
| 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 |
|
|