classification
Title: pty.spawn hangs on FreeBSD 9.3, 10.x
Type: behavior Stage: patch review
Components: Library (Lib) Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Cornelius Diekmann, RadicalZephyr, chris.torek, jarryshaw, jbeck, martin.panter, vstinner
Priority: normal Keywords: patch

Created on 2016-01-28 04:04 by chris.torek, last changed 2019-02-26 19:54 by RadicalZephyr.

Files
File name Uploaded Description Edit
pty.patch chris.torek, 2016-01-28 04:04 minor restructuring of pty.py, plus a change to its test modules review
Pull Requests
URL Status Linked Edit
PR 2932 closed python-dev, 2017-07-28 15:44
PR 4167 open Cornelius Diekmann, 2017-10-29 17:45
PR 12049 open RadicalZephyr, 2019-02-26 09:29
Messages (11)
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 issue22865).

I also fixed (sort of) issue17824, 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) * (Python committer) 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 issue29070. The test suite currently only exposes the problem, as suggested by Martin.
msg284552 - (view) Author: STINNER Victor (vstinner) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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: Geoff 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) * (Python committer) 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: Geoff Shannon (RadicalZephyr) * Date: 2019-02-26 19:54
I'm aware of it. I actually wrote that patch as well. :D
History
Date User Action Args
2019-02-26 19:54:42RadicalZephyrsetmessages: + msg336708
2019-02-26 16:15:30vstinnersetmessages: + msg336689
2019-02-26 09:50:18RadicalZephyrsetnosy: + RadicalZephyr
messages: + msg336637
2019-02-26 09:29:00RadicalZephyrsetpull_requests: + pull_request12074
2019-02-26 03:21:12jarryshawsetnosy: + jarryshaw
messages: + msg336601
2019-02-21 22:21:58martin.panterlinkissue34785 superseder
2017-11-25 02:13:30martin.pantersetmessages: + msg306934
2017-10-29 17:45:08Cornelius Diekmannsetpull_requests: + pull_request4136
2017-07-28 15:44:14python-devsetpull_requests: + pull_request2984
2017-04-26 20:23:38jbecksetnosy: + jbeck
messages: + msg292382
2017-01-03 19:45:46martin.pantersetmessages: + msg284590
2017-01-03 12:26:59vstinnersetmessages: + msg284552
2017-01-03 12:25:11vstinnersetnosy: + vstinner
2017-01-02 18:39:21Cornelius Diekmannsetnosy: + Cornelius Diekmann
messages: + msg284493
2016-01-28 06:08:25martin.pantersettype: behavior
components: + Library (Lib)
versions: + Python 2.7, Python 3.5, Python 3.6
nosy: + martin.panter

messages: + msg259093
stage: patch review
2016-01-28 04:04:21chris.torekcreate