From 640662071239b7b2cd2935b99c9c15210fd47f97 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 ab4d90d879..33e8c16751 100644 --- a/tools/idf_py_actions/debug_ext.py +++ b/tools/idf_py_actions/debug_ext.py @@ -473,11 +473,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 @@ -499,11 +494,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