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

@@ -10,6 +10,7 @@ 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

@@ -8,6 +8,8 @@
 import select
 import time
 from pprint import pformat
+from queue import Empty, Queue
+from threading import Thread
 from typing import Any, Dict, List, Optional, Tuple, Union
 
 from pygdbmi import gdbmiparser
@@ -19,11 +21,7 @@
 )
 
 
-if USING_WINDOWS:
-    import msvcrt
-    from ctypes import POINTER, WinError, byref, windll, wintypes  # type: ignore
-    from ctypes.wintypes import BOOL, DWORD, HANDLE
-else:
+if not USING_WINDOWS:
     import fcntl
 
 
@@ -67,9 +65,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
@@ -109,22 +124,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
@@ -137,11 +153,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:
@@ -324,28 +341,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

@@ -276,7 +276,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)
@@ -294,13 +296,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()