Skip to content

Commit

Permalink
fix(debug_ext): CTRL+C handling while waiting on gdb process
Browse files Browse the repository at this point in the history
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 <vstinner@python.org>
	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
python/cpython#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 espressif#11871

Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>
  • Loading branch information
fhrbata authored and ilutchenko committed Jul 24, 2023
1 parent 41729fe commit 99841be
Showing 1 changed file with 3 additions and 8 deletions.
11 changes: 3 additions & 8 deletions tools/idf_py_actions/debug_ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -484,11 +484,6 @@ def move_to_front(task_name: str) -> None:
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
Expand All @@ -510,11 +505,11 @@ def gdb(action: str, ctx: Context, args: PropertyDict, batch: bool, gdb_tui: Opt
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
Expand Down

0 comments on commit 99841be

Please sign in to comment.