Read tokens correctly from GDB's output on Windows (cs01#55)

Original patch by Leonardo Pereira Santos (see
cs01#55); updated by Marco Barisione.

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
 

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.

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