msg259088 - (view) |
Author: Chris Torek (chris.torek) * |
Date: 2016-01-28 04:04 |
The pty.spawn() code assumes that when the process on the slave side of the pty quits, the master side starts raising OSError when read-from or written-to. That used to be true in FBSD, but then someone fixed (?) it, and now the master side simply returns EOF when read-from. Furthermore, writes to the master simply disappear into the aether (this may be an OS bug, but even if the writes raised OSError, you would still have to type something in on stdin to get it copied over to get the error raised to get out of the loop). The fix here makes an assumption that is true when using the built-in read calls: EOF on the master indicates that the slave is no longer reachable in any way and the whole thing should finish up immediately. It might perhaps need a bit of documentation should someone want to substitute in their own read function (see enhancement request in ). I also fixed (sort of) , but only barely minimally, and put in a comment that it should really use the same mechanism as subprocess (but I think that should be a separate patch). |
|
|
msg259093 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2016-01-28 06:08 |
I agree with all the changes you made. I made one review comment. It would be nice to add a test case to expose the problem. Correct me if I am wrong, but it doesn’t look like pty.spawn() is tested at all. FWIW on Linux, reading from the master end seems to raise EIO if the slave has been closed. And writing to the master when the slave is closed seems to fill up a buffer and eventually blocks. Ideally I think the best solution for handing exec() failure (Issue 17824) would be to eliminate fork-exec with posix_spawn(); see Issue 20104. But as you say, that’s a separate problem. |
|
|
msg284493 - (view) |
Author: Cornelius Diekmann (Cornelius Diekmann) * |
Date: 2017-01-02 18:39 |
I just tested pty.spawn() on OS X 10.6.8 and FreeBSD 11 and it also hangs. It works on Linux. Your patch solves the problem. I proposed a test suite for pty.spawn() in . The test suite currently only exposes the problem, as suggested by Martin. |
|
|
msg284552 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2017-01-03 12:26 |
> That used to be true in FBSD, but then someone fixed (?) it, and now the master side simply returns EOF when read-from. Is it a behaviour change in a new version of Python? Or a change in FreeBSD? I don't understand. |
|
|
msg284590 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2017-01-03 19:45 |
Behaviour change in Free BSD as I understand. Nothing changed in Python, but perhaps older versions of Free BSD behaved like Linux and raised EIO (or another errno; it is not clear). |
|
|
msg292382 - (view) |
Author: John Beck (jbeck) |
Date: 2017-04-26 20:23 |
Solaris has this problem also, and Chris' patch fixes it for us. |
|
|
msg306934 - (view) |
Author: Martin Panter (martin.panter) *  |
Date: 2017-11-25 02:13 |
If it helps, here is a basic test case I wrote for “pty.spawn”. I hope that it exposes the problem on Free BSD, but I have only tested it on Linux. parent = r'''\ import pty, sys pty.spawn((sys.executable, "-c", sys.argv[1])) ''' child = r'''\ import sys # Read input first of all to minimize output buffering input = sys.stdin.readline() print("isatty: {}, {}, {}".format( sys.stdin.isatty(), sys.stdout.isatty(), sys.stderr.isatty())) print("input: " + repr(input)) sys.stdout.write("stdout data\n") sys.stderr.write("stderr data\n") ''' args = (sys.executable, "-c", parent, child) parent = subprocess.Popen(args, stdin=subprocess.PIPE, stdout=subprocess.PIPE) try: parent.stdin.write(b"stdin data\n") parent.stdin.flush() # Leave input open. When the child closes the slave terminal on # Free BSD, “spawn” used to keep waiting for input (BPO 26228). output = parent.stdout.read() finally: parent.stdout.close() parent.stdin.close() parent.wait() self.assertEqual(0, parent.returncode, repr(output)) self.assertIn(b"isatty: True, True, True", output) self.assertIn(br"input: 'stdin data\n'", output) self.assertIn(b"stdout data", output) self.assertIn(b"stderr data", output) |
|
|
msg336601 - (view) |
Author: Jarry Shaw (jarryshaw) * |
Date: 2019-02-26 03:21 |
Chris' patch works on macOS 10.14 (Mojave); but after it terminates, tty attributes are not restored (new-lines are missing). btw, I've implemented a workaround library with solution through either `ps` command or `psutil` package, see `ptyng` on PyPI. |
|
|
msg336637 - (view) |
Author: Zefira Shannon (RadicalZephyr) * |
Date: 2019-02-26 09:50 |
I took a shot at fixing this in a smaller more targeted patch. I think this should still solve the major issue of pty.spawn hanging on platforms that don't raise an error. In addition, pty.spawn should now _ALWAYS_ return the terminal to the previous settings. |
|
|
msg336689 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2019-02-26 16:15 |
Please have a look at this pty.spawn() documentation change: https://github.com/python/cpython/pull/11980 |
|
|
msg336708 - (view) |
Author: Zefira Shannon (RadicalZephyr) * |
Date: 2019-02-26 19:54 |
I'm aware of it. I actually wrote that patch as well. :D |
|
|
msg375084 - (view) |
Author: Soumendra Ganguly (soumendra) * |
Date: 2020-08-09 22:13 |
Hi! Can anyone please take a look at https://bugs.python.org/issue41494 [ https://github.com/python/cpython/pull/21752 ]? I think these are related. I wrote a new function called wspawn, which is like spawn+the following differences. 1. It sets window size at the beginning. 2. It registers a SIGWINCH handler. 3. It does not depend on OSError to return. It does a clean return instead. |
|
|
msg399420 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2021-08-11 22:22 |
New changeset 81ab8db235580317edcb0e559cd4c983f70883f5 by Zephyr Shannon in branch 'main': bpo-26228: Fix pty EOF handling (GH-12049) https://github.com/python/cpython/commit/81ab8db235580317edcb0e559cd4c983f70883f5 |
|
|
msg399445 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2021-08-12 12:36 |
New changeset 5d444434ad4e1943a88c9d3aadd300fd0f05dab7 by Miss Islington (bot) in branch '3.10': bpo-26228: Fix pty EOF handling (GH-12049) (GH-27732) https://github.com/python/cpython/commit/5d444434ad4e1943a88c9d3aadd300fd0f05dab7 |
|
|
msg399446 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2021-08-12 12:37 |
This is now merged. Thanks for all input, everyone! ✨ 🍰 ✨ |
|
|
msg399521 - (view) |
Author: Łukasz Langa (lukasz.langa) *  |
Date: 2021-08-13 10:57 |
New changeset dd8eb303b90d63e1f56684bedadca6674bb74a29 by Łukasz Langa in branch 'main': bpo-26228: [doc] Adapt PTY documentation updates from GH-4167 (GH-27754) https://github.com/python/cpython/commit/dd8eb303b90d63e1f56684bedadca6674bb74a29 |
|
|
msg399526 - (view) |
Author: miss-islington (miss-islington) |
Date: 2021-08-13 11:21 |
New changeset d4128485d6c2cbfebe756f3eeec2c60137b63bba by Miss Islington (bot) in branch '3.10': bpo-26228: [doc] Adapt PTY documentation updates from GH-4167 (GH-27754) https://github.com/python/cpython/commit/d4128485d6c2cbfebe756f3eeec2c60137b63bba |
|
|