-
-
Notifications
You must be signed in to change notification settings - Fork 31.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pty.spawn hangs on FreeBSD 9.3, 10.x, 12.1 #70416
Comments
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. |
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 |
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: |
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.
|
This is now merged. Thanks for all input, everyone! ✨ 🍰 ✨ |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: