diff --git a/CHANGELOG.md b/CHANGELOG.md index 78847f1..1675b44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # pygdbmi release history +## 0.10.0.1 + +Refactored IOManager non-blocking reading for Windows. Fixes issue #54 : "`IoManager._get_responses_windows` mangles token when reading from stdout" + ## 0.10.0.0 **Breaking Changes** diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 60205b3..38173ab 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -17,11 +17,10 @@ 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 logger = logging.getLogger(__name__) @@ -61,9 +60,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 @@ -107,18 +123,17 @@ def _get_responses_windows(self, 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 @@ -131,7 +146,6 @@ def _get_responses_windows(self, timeout_sec): ) elif time.time() > timeout_time_sec: break - return responses def _get_responses_unix(self, timeout_sec): @@ -265,9 +279,7 @@ def write( for fileno in outputready: if fileno == self.stdin_fileno: # ready to write - self.stdin.write( # type: ignore - mi_cmd_to_write_nl.encode() - ) + self.stdin.write(mi_cmd_to_write_nl.encode()) # type: ignore # must flush, otherwise gdb won't realize there is data # to evaluate, and we won't get a response self.stdin.flush() # type: ignore @@ -321,28 +333,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()) - - 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 f83fd50..e6f5680 100755 --- a/tests/test_pygdbmi.py +++ b/tests/test_pygdbmi.py @@ -188,7 +188,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) @@ -206,13 +208,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()