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 bpo-22865).

I also fixed (sort of) bpo-17824, 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).

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 (bpo-17824) would be to eliminate fork-exec with posix_spawn(); see bpo-20104. But as you say, that’s a separate problem.

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 bpo-29070. The test suite currently only exposes the problem, as suggested by Martin.

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.

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).

Solaris has this problem also, and Chris' patch fixes it for us.

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)

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.

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.

Please have a look at this pty.spawn() documentation change:
#11980

I'm aware of it. I actually wrote that patch as well. :D

Hi! Can anyone please take a look at

https://bugs.python.org/issue41494 [ https://github.com//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.

New changeset 81ab8db by Zephyr Shannon in branch 'main':
bpo-26228: Fix pty EOF handling (GH-12049)
81ab8db

New changeset 5d44443 by Miss Islington (bot) in branch '3.10':
bpo-26228: Fix pty EOF handling (GH-12049) (GH-27732)
5d44443

This is now merged. Thanks for all input, everyone! ✨ 🍰 ✨

New changeset dd8eb30 by Łukasz Langa in branch 'main':
bpo-26228: [doc] Adapt PTY documentation updates from #48417 (GH-27754)
dd8eb30

New changeset d412848 by Miss Islington (bot) in branch '3.10':
bpo-26228: [doc] Adapt PTY documentation updates from #48417 (GH-27754)
d412848