From ff1ce54869f6c2641f0a1d3ba2f2b9d5a4b86af5 Mon Sep 17 00:00:00 2001 From: Frantisek Hrbata Date: Mon, 17 Jul 2023 08:00:23 +0200 Subject: [PATCH] fix(debug_ext): CTRL+C handling while waiting on gdb process idf.py spawns gdb process within a thread and uses Thread.join() to wait for the gdb process to finish. As CTRL+C(SIGINT) is used by gdb to interrupt the running program, we catch the SIGINT while waiting on the gdb to finish, and try Thread.join() again. With cpython's commit commit a22be4943c119fecf5433d999227ff78fc2e5741 Author: Victor Stinner Date: Mon Sep 27 14:20:31 2021 +0200 bpo-45274: Fix Thread._wait_for_tstate_lock() race condition (GH-28532) this logic doesn't work anymore, because cpython internally marks the thread as stopped when join() is interrupted with an exception. IMHO this is broken in cpython and there is a bug report about this https://github.com/python/cpython/issues/90882. Problem is that waiting on a thread to finish is based on acquiring a lock. Meaning join() is waiting on _tstate_lock. If this wait is interrupted, the above referenced commit adds a logic that checks if the lock is help, meaning the thread is done and marks the thread as stopped. But there is no way to tell if the lock was acquired by us running join() or if it's held by someone else e.g. still by the thread bootstrap code. Meaning the thread is still running. I may be missing something, but I don't see any reason why to spawn gdb process within a thread. This change removes the thread and spawns gdb directly. Instead waiting on a thread, we wait on the process to finish, replacing join() with wait() and avoiding this problem. Closes https://github.com/espressif/esp-idf/issues/11871 Signed-off-by: Frantisek Hrbata --- tools/idf_py_actions/debug_ext.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/tools/idf_py_actions/debug_ext.py b/tools/idf_py_actions/debug_ext.py index 3b3dc98554..7a1ddc8a39 100644 --- a/tools/idf_py_actions/debug_ext.py +++ b/tools/idf_py_actions/debug_ext.py @@ -484,11 +484,6 @@ def action_extensions(base_actions: Dict, project_path: str) -> Dict: if task.name in ('gdb', 'gdbgui', 'gdbtui'): task.action_args['require_openocd'] = True - def run_gdb(gdb_args: List) -> int: - p = subprocess.Popen(gdb_args) - processes['gdb'] = p - return p.wait() - def gdbtui(action: str, ctx: Context, args: PropertyDict, gdbinit: str, require_openocd: bool) -> None: """ Synchronous GDB target with text ui mode @@ -510,11 +505,11 @@ def action_extensions(base_actions: Dict, project_path: str) -> Dict: args += ['-tui'] if batch: args += ['--batch'] - t = Thread(target=run_gdb, args=(args,)) - t.start() + p = subprocess.Popen(args) + processes['gdb'] = p while True: try: - t.join() + p.wait() break except KeyboardInterrupt: # Catching Keyboard interrupt, as this is used for breaking running program in gdb