From 68fe9b1fd72f6dcddd929a360fb5a2177f5a4d0a Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Mon, 17 May 2021 20:31:10 -0400 Subject: [PATCH 01/10] Fixed bug that the token wasn't being correctly read form GDB's output. --- pygdbmi/IoManager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 60205b3..0f333ac 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -131,6 +131,7 @@ def _get_responses_windows(self, timeout_sec): ) elif time.time() > timeout_time_sec: break + time.sleep(0.01) return responses From 80974977bc8ff0aedad1355f623d7a620a835ab8 Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Tue, 18 May 2021 11:47:40 -0400 Subject: [PATCH 02/10] Added internal parameter to parametrize the delay in IoManager._get_responses_windows. --- pygdbmi/IoManager.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 0f333ac..5db3f6c 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -65,6 +65,10 @@ def __init__( if self.stderr: make_non_blocking(self.stderr) + # Sometimes stdout.readline() would mangle the data read from GDB. + # That's fixed if we pause a little. + self._response_windows_delay = 0.02 + def get_gdb_response( self, timeout_sec: float = DEFAULT_GDB_TIMEOUT_SEC, raise_error_on_timeout=True ): @@ -131,7 +135,8 @@ def _get_responses_windows(self, timeout_sec): ) elif time.time() > timeout_time_sec: break - time.sleep(0.01) + + time.sleep(self._response_windows_delay) return responses From 45a6d46f652eb1d392880b350bd7ee01b2f6c7c5 Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Tue, 18 May 2021 17:51:52 -0400 Subject: [PATCH 03/10] Fixed non-blocking read for Windows. --- pygdbmi/IoManager.py | 75 ++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 41 deletions(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 5db3f6c..805bb97 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -17,6 +17,15 @@ GdbTimeoutError, ) +import sys +from subprocess import PIPE, Popen +from threading import Thread + +try: + from queue import Queue, Empty +except ImportError: + from Queue import Queue, Empty # python 2.x + if USING_WINDOWS: import msvcrt from ctypes import windll, byref, wintypes, WinError, POINTER # type: ignore @@ -25,6 +34,7 @@ import fcntl logger = logging.getLogger(__name__) +logger.setLevel(logging.DEBUG) class IoManager: @@ -61,13 +71,22 @@ 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) - # Sometimes stdout.readline() would mangle the data read from GDB. - # That's fixed if we pause a little. - self._response_windows_delay = 0.02 + if USING_WINDOWS: + self.queue_stdout = 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() + 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 @@ -111,18 +130,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 @@ -135,9 +153,6 @@ def _get_responses_windows(self, timeout_sec): ) elif time.time() > timeout_time_sec: break - - time.sleep(self._response_windows_delay) - return responses def _get_responses_unix(self, timeout_sec): @@ -326,29 +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()) - - 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")) + out.close() From 67d0b2011646e4648905252be0bf0ad23e03acda Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 12:25:39 -0400 Subject: [PATCH 04/10] Fixed ValueError exception. --- pygdbmi/IoManager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 805bb97..7c190e8 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -344,4 +344,4 @@ def _buffer_incomplete_responses( def enqueue_output(out, queue): for line in iter(out.readline, b''): queue.put(line.replace(b"\r", b"\n")) - out.close() + # Not necessary to close, it will be done in the main process. From 3a6904df8d2bc41b22790e2d6c8111ba1c027354 Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 12:31:38 -0400 Subject: [PATCH 05/10] Removed line that was istakenly committed. --- pygdbmi/IoManager.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 7c190e8..f8da396 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -34,8 +34,6 @@ import fcntl logger = logging.getLogger(__name__) -logger.setLevel(logging.DEBUG) - class IoManager: def __init__( From 3e064a93e793e856e7dc3b3b5a10ab4d077fdaf1 Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 12:37:08 -0400 Subject: [PATCH 06/10] Updated changelog. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) 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** From db20b4c4874a55287ed4812c81fb6c2a7481b2da Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 13:07:56 -0400 Subject: [PATCH 07/10] Fixed unit tests. --- tests/test_pygdbmi.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/test_pygdbmi.py b/tests/test_pygdbmi.py index f83fd50..f6eae0b 100755 --- a/tests/test_pygdbmi.py +++ b/tests/test_pygdbmi.py @@ -188,7 +188,7 @@ 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 +206,8 @@ 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() From 59dd171b04d78d633d763136ac2a4b8b6a96cc0e Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 14:37:12 -0400 Subject: [PATCH 08/10] Fixed linter issues. --- pygdbmi/IoManager.py | 22 +++++++++++++--------- tests/test_pygdbmi.py | 8 ++++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index f8da396..ab21613 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -19,7 +19,7 @@ import sys from subprocess import PIPE, Popen -from threading import Thread +from threading import Thread try: from queue import Queue, Empty @@ -35,6 +35,7 @@ logger = logging.getLogger(__name__) + class IoManager: def __init__( self, @@ -72,14 +73,18 @@ def __init__( if USING_WINDOWS: self.queue_stdout = 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 = 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() - 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 = 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) @@ -284,9 +289,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 @@ -339,7 +342,8 @@ def _buffer_incomplete_responses( return (raw_output, buf) + def enqueue_output(out, queue): - for line in iter(out.readline, b''): + 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 f6eae0b..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"], timeout_sec=3) + 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) @@ -207,7 +209,9 @@ def test_controller(self): assert gdbmi.gdb_process is None # Test ValueError exception - self.assertRaises(ValueError, gdbmi.write, "-file-exec-and-symbols %s" % c_hello_world_binary) + self.assertRaises( + ValueError, gdbmi.write, "-file-exec-and-symbols %s" % c_hello_world_binary + ) # Respawn and test signal handling gdbmi.spawn_new_gdb_subprocess() From 236760d1267e8c2782273a9582948e8daf6e8c3f Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 15:50:28 -0400 Subject: [PATCH 09/10] Fixed linter errors. --- pygdbmi/IoManager.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index ab21613..6146420 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -17,8 +17,6 @@ GdbTimeoutError, ) -import sys -from subprocess import PIPE, Popen from threading import Thread try: @@ -26,11 +24,7 @@ except ImportError: from Queue import Queue, Empty # python 2.x -if USING_WINDOWS: - import msvcrt - from ctypes import windll, byref, wintypes, WinError, POINTER # type: ignore - from ctypes.wintypes import HANDLE, DWORD, BOOL -else: +if not USING_WINDOWS: import fcntl logger = logging.getLogger(__name__) From 3782d5d57e7df9fc9fe30e863f8d171a881b8459 Mon Sep 17 00:00:00 2001 From: Leonardo Pereira Santos Date: Wed, 19 May 2021 16:14:16 -0400 Subject: [PATCH 10/10] Fixed final linter errors. --- pygdbmi/IoManager.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pygdbmi/IoManager.py b/pygdbmi/IoManager.py index 6146420..38173ab 100644 --- a/pygdbmi/IoManager.py +++ b/pygdbmi/IoManager.py @@ -18,11 +18,7 @@ ) from threading import Thread - -try: - from queue import Queue, Empty -except ImportError: - from Queue import Queue, Empty # python 2.x +from queue import Queue, Empty if not USING_WINDOWS: import fcntl @@ -66,7 +62,7 @@ def __init__( ) if USING_WINDOWS: - self.queue_stdout = Queue() + self.queue_stdout = Queue() # type: Queue self.thread_stdout = Thread( target=enqueue_output, args=(self.stdout, self.queue_stdout) ) @@ -74,7 +70,7 @@ def __init__( self.thread_stdout.start() if self.stderr: - self.queue_stderr = Queue() + self.queue_stderr = Queue() # type: Queue self.thread_stderr = Thread( target=enqueue_output, args=(self.stderr, self.queue_stderr) )