From 3d3facdc5f2d5d2a3cbabf083ee88fc6bf6bff27 Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Sat, 13 Aug 2022 10:10:45 +0100 Subject: [PATCH] Read tokens correctly from GDB's output on Windows (#55) Original patch by Leonardo Pereira Santos (see https://github.com/cs01/pygdbmi/pull/55); updated by Marco Barisione. --- CHANGELOG.md | 7 ++++ pygdbmi/IoManager.py | 82 ++++++++++++++++++++----------------------- tests/test_pygdbmi.py | 15 ++++---- 3 files changed, 52 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f27361f..35422aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,15 @@ ## dev +**Breaking changes** + +- Removed `pygdbmi.IoManager.make_non_blocking` which was never meant to be public API + +Other changes + - Fixed a bug where notifications without a payload were not recognized as such - Invalid octal sequences produced by GDB are left unchanged instead of causing a `UnicodeDecodeError` (#64) +- Fix IoManager not to mangle tokens when reading from stdout on Windows (#55) Internal changes diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index ab226be..5c238e6 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -17,18 +17,14 @@ GdbTimeoutError, ) -if USING_WINDOWS: - import msvcrt - from ctypes import windll, byref, wintypes, WinError, POINTER # type: ignore - from ctypes.wintypes import HANDLE, DWORD, BOOL -else: +from threading import Thread +from queue import Queue, Empty + +if not USING_WINDOWS: import fcntl -__all__ = [ - "IoManager", - "make_non_blocking", -] +__all__ = ["IoManager"] logger = logging.getLogger(__name__) @@ -68,9 +64,26 @@ def __init__( self._allow_overwrite_timeout_times = ( self.time_to_check_for_additional_output_sec > 0 ) - make_non_blocking(self.stdout) - if self.stderr: - make_non_blocking(self.stderr) + + if USING_WINDOWS: + self.queue_stdout = Queue() # type: Queue + self.thread_stdout = Thread( + target=_enqueue_output, args=(self.stdout, self.queue_stdout) + ) + self.thread_stdout.daemon = True # thread dies with the program + self.thread_stdout.start() + + if self.stderr: + self.queue_stderr = Queue() # type: Queue + self.thread_stderr = Thread( + target=_enqueue_output, args=(self.stderr, self.queue_stderr) + ) + self.thread_stderr.daemon = True # thread dies with the program + self.thread_stderr.start() + else: + fcntl.fcntl(self.stdout, fcntl.F_SETFL, os.O_NONBLOCK) + if self.stderr: + fcntl.fcntl(self.stderr, fcntl.F_SETFL, os.O_NONBLOCK) def get_gdb_response( self, timeout_sec: float = DEFAULT_GDB_TIMEOUT_SEC, raise_error_on_timeout=True @@ -110,22 +123,23 @@ def get_gdb_response( def _get_responses_windows(self, timeout_sec): """Get responses on windows. Assume no support for select and use a while loop.""" + assert USING_WINDOWS + timeout_time_sec = time.time() + timeout_sec responses = [] while True: responses_list = [] + try: - self.stdout.flush() - raw_output = self.stdout.readline().replace(b"\r", b"\n") + raw_output = self.queue_stdout.get_nowait() responses_list = self._get_responses_list(raw_output, "stdout") - except IOError: + except Empty: pass try: - self.stderr.flush() - raw_output = self.stderr.readline().replace(b"\r", b"\n") + raw_output = self.queue_stderr.get_nowait() responses_list += self._get_responses_list(raw_output, "stderr") - except IOError: + except Empty: pass responses += responses_list @@ -138,11 +152,12 @@ def _get_responses_windows(self, timeout_sec): ) elif time.time() > timeout_time_sec: break - return responses def _get_responses_unix(self, timeout_sec): """Get responses on unix-like system. Use select to wait for output.""" + assert not USING_WINDOWS + timeout_time_sec = time.time() + timeout_sec responses = [] while True: @@ -325,28 +340,7 @@ def _buffer_incomplete_responses( return (raw_output, buf) -def make_non_blocking(file_obj: io.IOBase): - """make file object non-blocking - Windows doesn't have the fcntl module, but someone on - stack overflow supplied this code as an answer, and it works - http://stackoverflow.com/a/34504971/2893090""" - - if USING_WINDOWS: - LPDWORD = POINTER(DWORD) - PIPE_NOWAIT = wintypes.DWORD(0x00000001) - - SetNamedPipeHandleState = windll.kernel32.SetNamedPipeHandleState - SetNamedPipeHandleState.argtypes = [HANDLE, LPDWORD, LPDWORD, LPDWORD] - SetNamedPipeHandleState.restype = BOOL - - h = msvcrt.get_osfhandle(file_obj.fileno()) # type: ignore - - res = windll.kernel32.SetNamedPipeHandleState(h, byref(PIPE_NOWAIT), None, None) - if res == 0: - raise ValueError(WinError()) - - else: - # Set the file status flag (F_SETFL) on the pipes to be non-blocking - # so we can attempt to read from a pipe with no new data without locking - # the program up - fcntl.fcntl(file_obj, fcntl.F_SETFL, os.O_NONBLOCK) +def _enqueue_output(out, queue): + for line in iter(out.readline, b""): + queue.put(line.replace(b"\r", b"\n")) + # Not necessary to close, it will be done in the main process. diff --git a/tests/test_pygdbmi.py b/tests/test_pygdbmi.py index bf30b42..8c33e2e 100755 --- a/tests/test_pygdbmi.py +++ b/tests/test_pygdbmi.py @@ -275,7 +275,9 @@ def test_controller(self): assert response["stream"] == "stdout" assert response["token"] is None - responses = gdbmi.write(["-file-list-exec-source-files", "-break-insert main"]) + responses = gdbmi.write( + ["-file-list-exec-source-files", "-break-insert main"], timeout_sec=3 + ) assert len(responses) != 0 responses = gdbmi.write(["-exec-run", "-exec-continue"], timeout_sec=3) @@ -293,13 +295,10 @@ def test_controller(self): assert responses is None assert gdbmi.gdb_process is None - # Test NoGdbProcessError exception - got_no_process_exception = False - try: - responses = gdbmi.write("-file-exec-and-symbols %s" % c_hello_world_binary) - except IOError: - got_no_process_exception = True - assert got_no_process_exception is True + # Test ValueError exception + self.assertRaises( + ValueError, gdbmi.write, "-file-exec-and-symbols %s" % c_hello_world_binary + ) # Respawn and test signal handling gdbmi.spawn_new_gdb_subprocess()