From 0609120434b6be5a223a8b816ee7ddca7498315e Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 7 Sep 2025 19:48:07 +0800 Subject: [PATCH 1/8] fix: pyrepl can messup and poll is not thread safe Signed-off-by: yihong0618 --- Lib/_pyrepl/unix_console.py | 44 +++++++++++++++---- Lib/test/test_pyrepl/test_unix_console.py | 15 +++++++ ...-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst | 1 + 3 files changed, 52 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index a7e49923191c07..9340e74373d974 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -28,6 +28,7 @@ import signal import struct import termios +import threading import time import types import platform @@ -157,6 +158,8 @@ def __init__( self.pollob = poll() self.pollob.register(self.input_fd, select.POLLIN) + self._poll_lock = threading.RLock() + self._polling_thread = None self.terminfo = terminfo.TermInfo(term or None) self.term = term @@ -325,8 +328,11 @@ def prepare(self): """ Prepare the console for input/output operations. """ - self.__svtermstate = tcgetattr(self.input_fd) - raw = self.__svtermstate.copy() + # gh-130168: prevents signal handlers from overwriting the original state + if not hasattr(self, '_UnixConsole__svtermstate'): + self.__svtermstate = tcgetattr(self.input_fd) + + raw = tcgetattr(self.input_fd).copy() raw.iflag &= ~(termios.INPCK | termios.ISTRIP | termios.IXON) raw.oflag &= ~(termios.OPOST) raw.cflag &= ~(termios.CSIZE | termios.PARENB) @@ -368,7 +374,11 @@ def restore(self): self.__disable_bracketed_paste() self.__maybe_write_code(self._rmkx) self.flushoutput() - tcsetattr(self.input_fd, termios.TCSADRAIN, self.__svtermstate) + + if hasattr(self, '_UnixConsole__svtermstate'): + tcsetattr(self.input_fd, termios.TCSADRAIN, self.__svtermstate) + # Clear the saved state so prepare() can save a fresh one next time + del self.__svtermstate if platform.system() == "Darwin" and os.getenv("TERM_PROGRAM") == "Apple_Terminal": os.write(self.output_fd, b"\033[?7h") @@ -417,10 +427,25 @@ def wait(self, timeout: float | None = None) -> bool: """ Wait for events on the console. """ - return ( - not self.event_queue.empty() - or bool(self.pollob.poll(timeout)) - ) + if not self.event_queue.empty(): + return True + + current_thread = threading.current_thread() + + if self._polling_thread is current_thread: + # This is a re-entrant call from the same thread + # like old repl runtime error + raise RuntimeError("can't re-enter readline") + + if not self._poll_lock.acquire(blocking=False): + return False + + try: + self._polling_thread = current_thread + return bool(self.pollob.poll(timeout)) + finally: + self._polling_thread = None + self._poll_lock.release() def set_cursor_vis(self, visible): """ @@ -786,7 +811,10 @@ def __tputs(self, fmt, prog=delayprog): # using .get() means that things will blow up # only if the bps is actually needed (which I'm # betting is pretty unlkely) - bps = ratedict.get(self.__svtermstate.ospeed) + if hasattr(self, '_UnixConsole__svtermstate'): + bps = ratedict.get(self.__svtermstate.ospeed) + else: + bps = None while True: m = prog.search(fmt) if not m: diff --git a/Lib/test/test_pyrepl/test_unix_console.py b/Lib/test/test_pyrepl/test_unix_console.py index ab1236768cfb3e..77172718b92244 100644 --- a/Lib/test/test_pyrepl/test_unix_console.py +++ b/Lib/test/test_pyrepl/test_unix_console.py @@ -1,6 +1,7 @@ import itertools import os import sys +import threading import unittest from functools import partial from test.support import os_helper, force_not_colorized_test_class @@ -303,3 +304,17 @@ def test_getheightwidth_with_invalid_environ(self, _os_write): self.assertIsInstance(console.getheightwidth(), tuple) os.environ = [] self.assertIsInstance(console.getheightwidth(), tuple) + + def test_wait_reentry_protection(self, _os_write): + # gh-130168: Test signal handler re-entry protection + console = UnixConsole(term="xterm") + console.prepare() + + console._polling_thread = threading.current_thread() + + with self.assertRaises(RuntimeError) as cm: + console.wait(timeout=0) + self.assertEqual(str(cm.exception), "can't re-enter readline") + + console._polling_thread = None + console.restore() diff --git a/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst b/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst new file mode 100644 index 00000000000000..cadd00a12be6d7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst @@ -0,0 +1 @@ +Fix: pyrepl messed up terminal if a signal handler expects stdin. From ffc77f2b9750d6d29f72874b30348a9a94f6a9dd Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 7 Sep 2025 19:54:50 +0800 Subject: [PATCH 2/8] fix: mypy type check Signed-off-by: yihong0618 --- Lib/_pyrepl/unix_console.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 9340e74373d974..2f30479fc5b215 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -159,7 +159,7 @@ def __init__( self.pollob = poll() self.pollob.register(self.input_fd, select.POLLIN) self._poll_lock = threading.RLock() - self._polling_thread = None + self._polling_thread: threading.Thread | None = None self.terminfo = terminfo.TermInfo(term or None) self.term = term From 8fa9c19ac1f5ab81c0502586c8b0b24465e70b84 Mon Sep 17 00:00:00 2001 From: yihong Date: Sun, 7 Sep 2025 20:12:12 +0800 Subject: [PATCH 3/8] fix: apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> --- Lib/_pyrepl/unix_console.py | 19 ++++++------------- Lib/test/test_pyrepl/test_unix_console.py | 9 +++------ ...-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst | 3 ++- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 2f30479fc5b215..34ab8e81b33680 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -329,10 +329,9 @@ def prepare(self): Prepare the console for input/output operations. """ # gh-130168: prevents signal handlers from overwriting the original state - if not hasattr(self, '_UnixConsole__svtermstate'): + if self.__svtermstate is None: self.__svtermstate = tcgetattr(self.input_fd) - - raw = tcgetattr(self.input_fd).copy() + raw = self.__svtermstate.copy() raw.iflag &= ~(termios.INPCK | termios.ISTRIP | termios.IXON) raw.oflag &= ~(termios.OPOST) raw.cflag &= ~(termios.CSIZE | termios.PARENB) @@ -374,11 +373,10 @@ def restore(self): self.__disable_bracketed_paste() self.__maybe_write_code(self._rmkx) self.flushoutput() - - if hasattr(self, '_UnixConsole__svtermstate'): + if self.__svtermstate is not None: tcsetattr(self.input_fd, termios.TCSADRAIN, self.__svtermstate) - # Clear the saved state so prepare() can save a fresh one next time - del self.__svtermstate + # Reset the state for the next prepare() call. + self.__svtermstate = None if platform.system() == "Darwin" and os.getenv("TERM_PROGRAM") == "Apple_Terminal": os.write(self.output_fd, b"\033[?7h") @@ -429,17 +427,12 @@ def wait(self, timeout: float | None = None) -> bool: """ if not self.event_queue.empty(): return True - current_thread = threading.current_thread() - if self._polling_thread is current_thread: - # This is a re-entrant call from the same thread - # like old repl runtime error + # Forbid re-entrant calls and use the old REPL error message. raise RuntimeError("can't re-enter readline") - if not self._poll_lock.acquire(blocking=False): return False - try: self._polling_thread = current_thread return bool(self.pollob.poll(timeout)) diff --git a/Lib/test/test_pyrepl/test_unix_console.py b/Lib/test/test_pyrepl/test_unix_console.py index 77172718b92244..6d4e8593bc1952 100644 --- a/Lib/test/test_pyrepl/test_unix_console.py +++ b/Lib/test/test_pyrepl/test_unix_console.py @@ -309,12 +309,9 @@ def test_wait_reentry_protection(self, _os_write): # gh-130168: Test signal handler re-entry protection console = UnixConsole(term="xterm") console.prepare() + self.addCleanup(console.restore) + self.addCleanup(setattr, console, '_polling_thread', None) console._polling_thread = threading.current_thread() - - with self.assertRaises(RuntimeError) as cm: + with self.assertRaisesRegex(RuntimeError, "can't re-enter readline"): console.wait(timeout=0) - self.assertEqual(str(cm.exception), "can't re-enter readline") - - console._polling_thread = None - console.restore() diff --git a/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst b/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst index cadd00a12be6d7..e217f223e5825e 100644 --- a/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst +++ b/Misc/NEWS.d/next/Library/2025-09-07-19-47-35.gh-issue-130168.qvtlOZ.rst @@ -1 +1,2 @@ -Fix: pyrepl messed up terminal if a signal handler expects stdin. +Make sure that the new REPL does not get scrambled when a signal handler +uses interactive input. From 78d741629bee518b350b2e009c13f2c59b2402cc Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 7 Sep 2025 20:16:35 +0800 Subject: [PATCH 4/8] fix: __svtermstate default None Signed-off-by: yihong0618 --- Lib/_pyrepl/unix_console.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 34ab8e81b33680..f069f6b6993c6b 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -160,6 +160,7 @@ def __init__( self.pollob.register(self.input_fd, select.POLLIN) self._poll_lock = threading.RLock() self._polling_thread: threading.Thread | None = None + self.__svtermstate = None self.terminfo = terminfo.TermInfo(term or None) self.term = term @@ -804,7 +805,7 @@ def __tputs(self, fmt, prog=delayprog): # using .get() means that things will blow up # only if the bps is actually needed (which I'm # betting is pretty unlkely) - if hasattr(self, '_UnixConsole__svtermstate'): + if self.__svtermstate is not None: bps = ratedict.get(self.__svtermstate.ospeed) else: bps = None From ca27ae4d0977020354a6c0d125046f6988bbf70c Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 7 Sep 2025 20:33:40 +0800 Subject: [PATCH 5/8] fix: drop cleanup part for new test Signed-off-by: yihong0618 --- Lib/test/test_pyrepl/test_unix_console.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_pyrepl/test_unix_console.py b/Lib/test/test_pyrepl/test_unix_console.py index 6d4e8593bc1952..05ee9efc4b0ab4 100644 --- a/Lib/test/test_pyrepl/test_unix_console.py +++ b/Lib/test/test_pyrepl/test_unix_console.py @@ -309,8 +309,6 @@ def test_wait_reentry_protection(self, _os_write): # gh-130168: Test signal handler re-entry protection console = UnixConsole(term="xterm") console.prepare() - self.addCleanup(console.restore) - self.addCleanup(setattr, console, '_polling_thread', None) console._polling_thread = threading.current_thread() with self.assertRaisesRegex(RuntimeError, "can't re-enter readline"): From b194f8fae6b22f2adda6526ad7da93ad6c01cadd Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Sun, 7 Sep 2025 20:57:59 +0800 Subject: [PATCH 6/8] fix: forget to add clean up back Signed-off-by: yihong0618 --- Lib/test/test_pyrepl/test_unix_console.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Lib/test/test_pyrepl/test_unix_console.py b/Lib/test/test_pyrepl/test_unix_console.py index 05ee9efc4b0ab4..7f8782eda3618c 100644 --- a/Lib/test/test_pyrepl/test_unix_console.py +++ b/Lib/test/test_pyrepl/test_unix_console.py @@ -313,3 +313,6 @@ def test_wait_reentry_protection(self, _os_write): console._polling_thread = threading.current_thread() with self.assertRaisesRegex(RuntimeError, "can't re-enter readline"): console.wait(timeout=0) + + console._polling_thread = None + console.restore() From 5d83648eed320a6e7dcfaac2ee806afc0f4f0d3f Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Wed, 17 Sep 2025 11:23:21 +0800 Subject: [PATCH 7/8] fix: tests failed by wrong conflict fix Signed-off-by: yihong0618 --- Lib/_pyrepl/unix_console.py | 7 ++++--- Lib/test/test_pyrepl/test_unix_console.py | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 92ba77b71ac604..aab55eec17dc93 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -386,9 +386,10 @@ def restore(self): self.__maybe_write_code(self._rmkx) self.flushoutput() try: - tcsetattr(self.input_fd, termios.TCSADRAIN, self.__svtermstate) - # Reset the state for the next prepare() call. - self.__svtermstate = None + if self.__svtermstate is not None: + tcsetattr(self.input_fd, termios.TCSADRAIN, self.__svtermstate) + # Reset the state for the next prepare() call. + self.__svtermstate = None except termios.error as e: if e.args[0] != errno.EIO: raise diff --git a/Lib/test/test_pyrepl/test_unix_console.py b/Lib/test/test_pyrepl/test_unix_console.py index d1867189a16f65..90bd1ffe6bec2a 100644 --- a/Lib/test/test_pyrepl/test_unix_console.py +++ b/Lib/test/test_pyrepl/test_unix_console.py @@ -320,6 +320,7 @@ def test_wait_reentry_protection(self, _os_write): console._polling_thread = None console.restore() + @unittest.skipUnless(sys.platform == "darwin", "requires macOS") def test_restore_with_invalid_environ_on_macos(self, _os_write): # gh-128636 for macOS From 17877b634ed7c25626fcc16d501d129ef2fb5b2c Mon Sep 17 00:00:00 2001 From: yihong0618 Date: Thu, 25 Sep 2025 21:20:02 +0800 Subject: [PATCH 8/8] fix: merge regress Signed-off-by: yihong0618 --- Lib/_pyrepl/unix_console.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Lib/_pyrepl/unix_console.py b/Lib/_pyrepl/unix_console.py index 9a2feea9c863da..46c3f0ec0d674c 100644 --- a/Lib/_pyrepl/unix_console.py +++ b/Lib/_pyrepl/unix_console.py @@ -350,8 +350,7 @@ def prepare(self): self.__svtermstate = tcgetattr(self.input_fd) self.__buffer = [] - self.__svtermstate = tcgetattr(self.input_fd) - raw = self.__svtermstate.copy() + raw = tcgetattr(self.input_fd).copy() raw.iflag &= ~(termios.INPCK | termios.ISTRIP | termios.IXON) raw.oflag &= ~(termios.OPOST) raw.cflag &= ~(termios.CSIZE | termios.PARENB)