diff --git a/Doc/library/pty.rst b/Doc/library/pty.rst index 0ab766065d6e81..2e665322771881 100644 --- a/Doc/library/pty.rst +++ b/Doc/library/pty.rst @@ -2,8 +2,8 @@ ======================================== .. module:: pty - :platform: Linux - :synopsis: Pseudo-Terminal Handling for Linux. + :platform: Unix + :synopsis: Pseudo-Terminal Handling for Unix. .. moduleauthor:: Steen Lumholt .. sectionauthor:: Moshe Zadka @@ -16,9 +16,9 @@ The :mod:`pty` module defines operations for handling the pseudo-terminal concept: starting another process and being able to write to and read from its controlling terminal programmatically. -Because pseudo-terminal handling is highly platform dependent, there is code to -do it only for Linux. (The Linux code is supposed to work on other platforms, -but hasn't been tested yet.) +Pseudo-terminal handling is highly platform dependent. This code is mainly +tested on Linux, FreeBSD, and OS X (it is supposed to work on other POSIX +platforms). The :mod:`pty` module defines the following functions: @@ -41,9 +41,13 @@ The :mod:`pty` module defines the following functions: .. function:: spawn(argv[, master_read[, stdin_read]]) - Spawn a process, and connect its controlling terminal with the current - process's standard io. This is often used to baffle programs which insist on - reading from the controlling terminal. + Spawn a child process, and connect its controlling terminal with the + current process's standard io. This is often used to baffle programs which + insist on reading from the controlling terminal. + + A loop copies STDIN of the current process to the child and data received + from the child to STDOUT of the current process. It is not signaled to the + child if STDIN of the current process closes down. The functions *master_read* and *stdin_read* should be functions which read from a file descriptor. The defaults try to read 1024 bytes each time they are diff --git a/Lib/pty.py b/Lib/pty.py index e841f12f3edb9b..ae122b81ea0aa7 100644 --- a/Lib/pty.py +++ b/Lib/pty.py @@ -1,13 +1,14 @@ """Pseudo terminal utilities.""" # Bugs: No signal handling. Doesn't set slave termios and window size. -# Only tested on Linux. +# Only tested on Linux, FreeBSD, and OS X. # See: W. Richard Stevens. 1992. Advanced Programming in the # UNIX Environment. Chapter 19. # Author: Steen Lumholt -- with additions by Guido. from select import select import os +import sys import tty __all__ = ["openpty","fork","spawn"] @@ -133,11 +134,16 @@ def _copy(master_fd, master_read=_read, stdin_read=_read): standard input -> pty master (stdin_read)""" fds = [master_fd, STDIN_FILENO] while True: + # The expected path to leave this infinite loop is that the + # child exits and its slave_fd is destroyed. In this case, + # master_fd will become ready in select() and reading from + # master_fd either raises an OSError (Input/output error) on + # Linux or returns EOF on BSD. rfds, wfds, xfds = select(fds, [], []) if master_fd in rfds: data = master_read(master_fd) if not data: # Reached EOF. - fds.remove(master_fd) + return else: os.write(STDOUT_FILENO, data) if STDIN_FILENO in rfds: @@ -153,7 +159,15 @@ def spawn(argv, master_read=_read, stdin_read=_read): argv = (argv,) pid, master_fd = fork() if pid == CHILD: - os.execlp(argv[0], *argv) + try: + os.execlp(argv[0], *argv) + except: + # If we wanted to be really clever, we would use + # the same method as subprocess() to pass the error + # back to the parent. For now just dump stack trace. + sys.excepthook(*sys.exc_info()) + finally: + os._exit(1) try: mode = tty.tcgetattr(STDIN_FILENO) tty.setraw(STDIN_FILENO) @@ -163,6 +177,10 @@ def spawn(argv, master_read=_read, stdin_read=_read): try: _copy(master_fd, master_read, stdin_read) except OSError: + # Some OSes never return an EOF on pty, just raise + # an error instead. + pass + finally: if restore: tty.tcsetattr(STDIN_FILENO, tty.TCSAFLUSH, mode) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 3b448569a2ffcb..64f593c521ec3c 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -211,8 +211,15 @@ def test_fork(self): # pty.fork() passed. +class _MockSelectEternalWait(Exception): + """Used both as exception and placeholder value. Models that no + more select activity is expected and that a test can be + terminated.""" + pass + class SmallPtyTests(unittest.TestCase): - """These tests don't spawn children or hang.""" + """Whitebox mocking tests which don't spawn children or hang. Test + the _copy loop to transfer data between parent and child.""" def setUp(self): self.orig_stdin_fileno = pty.STDIN_FILENO @@ -223,6 +230,10 @@ def setUp(self): self.select_rfds_lengths = [] self.select_rfds_results = [] + # monkey-patch and replace with mock + pty.select = self._mock_select + self._mock_stdin_stdout() + def tearDown(self): pty.STDIN_FILENO = self.orig_stdin_fileno pty.STDOUT_FILENO = self.orig_stdout_fileno @@ -243,67 +254,154 @@ def _pipe(self): self.fds.extend(pipe_fds) return pipe_fds - def _socketpair(self): - socketpair = socket.socketpair() - self.files.extend(socketpair) - return socketpair + def _mock_stdin_stdout(self): + """Mock STDIN and STDOUT with two fresh pipes. Replaces + pty.STDIN_FILENO/pty.STDOUT_FILENO by one end of the pipe. + Makes the other end of the pipe available via self.""" + self.read_from_stdout_fd, pty.STDOUT_FILENO = self._pipe() + pty.STDIN_FILENO, self.write_to_stdin_fd = self._pipe() def _mock_select(self, rfds, wfds, xfds): + """Simulates the behavior of select.select. Only implemented + for reader waiting list (first parameter).""" + assert wfds == [] and xfds == [] # This will raise IndexError when no more expected calls exist. self.assertEqual(self.select_rfds_lengths.pop(0), len(rfds)) - return self.select_rfds_results.pop(0), [], [] + if len(rfds) == 0: + # called with three empty lists as file descriptors to wait + # on. Behavior of real select is platform-dependent and + # likely infinite blocking on Linux. + self.fail("mock select on no waitables") + rfds_result = self.select_rfds_results.pop(0) + + if rfds_result is _MockSelectEternalWait: + raise _MockSelectEternalWait + return rfds_result, [], [] + + def test__mock_stdin_stdout(self): + """Test that _mock_stdin_stdout was called during setUp.""" + self.assertGreater(pty.STDIN_FILENO, 2, "stdin replaced by our mock") + self.assertGreater(pty.STDOUT_FILENO, 2, "stdout replaced by our mock") + + def test__mock_select(self): + # Test the select proxy of this test class. Meta testing. + self.select_rfds_lengths.append(0) + with self.assertRaises(AssertionError): + self._mock_select([], [], []) + + # Prepare two select calls. Second one will block forever. + self.select_rfds_lengths.append(3) + self.select_rfds_results.append("foo") + self.select_rfds_lengths.append(3) + self.select_rfds_results.append(_MockSelectEternalWait) + + # Call one + self.assertEqual(self._mock_select([1, 2, 3], [], []), + ("foo", [], [])) + + # Call two + with self.assertRaises(_MockSelectEternalWait): + self._mock_select([1, 2, 3], [], []) + + # lists are cleaned + self.assertEqual(self.select_rfds_lengths, []) + self.assertEqual(self.select_rfds_results, []) + + def _socketpair(self): + socketpair = socket.socketpair() + self.files.extend(socketpair) + return socketpair def test__copy_to_each(self): - """Test the normal data case on both master_fd and stdin.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd - socketpair = self._socketpair() - masters = [s.fileno() for s in socketpair] + # Test the normal data case on both master_fd and stdin. + masters = [s.fileno() for s in self._socketpair()] # Feed data. Smaller than PIPEBUF. These writes will not block. os.write(masters[1], b'from master') - os.write(write_to_stdin_fd, b'from stdin') + os.write(self.write_to_stdin_fd, b'from stdin') - # Expect two select calls, the last one will cause IndexError - pty.select = self._mock_select + # Expect two select calls, the last one will simulate eternal waiting self.select_rfds_lengths.append(2) - self.select_rfds_results.append([mock_stdin_fd, masters[0]]) + self.select_rfds_results.append([pty.STDIN_FILENO, masters[0]]) self.select_rfds_lengths.append(2) + self.select_rfds_results.append(_MockSelectEternalWait) - with self.assertRaises(IndexError): + with self.assertRaises(_MockSelectEternalWait): pty._copy(masters[0]) # Test that the right data went to the right places. - rfds = select.select([read_from_stdout_fd, masters[1]], [], [], 0)[0] - self.assertEqual([read_from_stdout_fd, masters[1]], rfds) - self.assertEqual(os.read(read_from_stdout_fd, 20), b'from master') + rfds = select.select([self.read_from_stdout_fd, masters[1]], [], [], 0)[0] + self.assertEqual([self.read_from_stdout_fd, masters[1]], rfds) + self.assertEqual(os.read(self.read_from_stdout_fd, 20), b'from master') self.assertEqual(os.read(masters[1], 20), b'from stdin') - def test__copy_eof_on_all(self): - """Test the empty read EOF case on both master_fd and stdin.""" - read_from_stdout_fd, mock_stdout_fd = self._pipe() - pty.STDOUT_FILENO = mock_stdout_fd - mock_stdin_fd, write_to_stdin_fd = self._pipe() - pty.STDIN_FILENO = mock_stdin_fd + def _copy_eof_close_slave_helper(self, close_stdin): + """Helper to test the empty read EOF case on master_fd and/or + stdin.""" socketpair = self._socketpair() masters = [s.fileno() for s in socketpair] + # This side of the channel would usually be the slave_fd of the + # child. We simulate that the child has exited and its side of + # the channel is destroyed. socketpair[1].close() - os.close(write_to_stdin_fd) + self.files.remove(socketpair[1]) - # Expect two select calls, the last one will cause IndexError - pty.select = self._mock_select + # Optionally close fd or fill with dummy data in order to + # prevent blocking on one read call + if close_stdin: + os.close(self.write_to_stdin_fd) + self.fds.remove(self.write_to_stdin_fd) + else: + os.write(self.write_to_stdin_fd, b'from stdin') + + # Expect exactly one select() call. This call returns master_fd + # and STDIN. Since the slave side of masters is closed, we + # expect the _copy loop to exit immediately. self.select_rfds_lengths.append(2) - self.select_rfds_results.append([mock_stdin_fd, masters[0]]) - # We expect that both fds were removed from the fds list as they - # both encountered an EOF before the second select call. - self.select_rfds_lengths.append(0) + self.select_rfds_results.append([pty.STDIN_FILENO, masters[0]]) + + # Run the _copy test, which returns nothing and cleanly exits + self.assertIsNone(pty._copy(masters[0])) + + # We expect that everything is consumed + self.assertEqual(self.select_rfds_results, []) + self.assertEqual(self.select_rfds_lengths, []) + + def test__copy_eof_on_all(self): + # Test the empty read EOF case on both master_fd and stdin. + self._copy_eof_close_slave_helper(close_stdin=True) + + def test__copy_eof_on_master(self): + # Test the empty read EOF case on only master_fd. + self._copy_eof_close_slave_helper(close_stdin=False) + + def test__copy_eof_on_stdin(self): + # Test the empty read EOF case on stdin. + masters = [s.fileno() for s in self._socketpair()] + + # Fill with dummy data + os.write(masters[1], b'from master') - with self.assertRaises(IndexError): + os.close(self.write_to_stdin_fd) + self.fds.remove(self.write_to_stdin_fd) + + # Expect two select() calls. The first call returns master_fd + # and STDIN. + self.select_rfds_lengths.append(2) + self.select_rfds_results.append([pty.STDIN_FILENO, masters[0]]) + # The second call causes _MockSelectEternalWait. We expect that + # STDIN is removed from the waiters as it reached EOF. + self.select_rfds_lengths.append(1) + self.select_rfds_results.append(_MockSelectEternalWait) + + with self.assertRaises(_MockSelectEternalWait): pty._copy(masters[0]) + # We expect that everything is consumed + self.assertEqual(self.select_rfds_results, []) + self.assertEqual(self.select_rfds_lengths, []) + def tearDownModule(): reap_children() diff --git a/Misc/ACKS b/Misc/ACKS index 54d8d62b633f70..8fc9eafdf861e7 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1787,3 +1787,5 @@ Jelle Zijlstra Gennadiy Zlobin Doug Zongker Peter Åstrand +Chris Torek +Cornelius Diekmann diff --git a/Misc/NEWS.d/next/Library/2017-10-29.bpo-26228.piIl5E.rst b/Misc/NEWS.d/next/Library/2017-10-29.bpo-26228.piIl5E.rst new file mode 100644 index 00000000000000..b998bdf7c72ded --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-29.bpo-26228.piIl5E.rst @@ -0,0 +1 @@ +pty.spawn no longer hangs on FreeBSD, OS X, and Solaris.