Fixes issue # 54: IoManager._get_responses_windows mangles token when reading from stdout

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**

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.

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()