Branch: refs/heads/bbiiggppiigg/fix-test-thread-1
Home: https://github.com/dyninst/dyninst
Commit: e9df01dae0a1deef73de8e76f08786e8cf7c2572
https://github.com/dyninst/dyninst/commit/e9df01dae0a1deef73de8e76f08786e8cf7c2572
Author: Hsuan-Heng Wu <bbiiggppiigg@xxxxxxxxx>
Date: 2026-07-07 (Tue, 07 Jul 2026)
Changed paths:
M proccontrol/src/event.C
M proccontrol/src/handler.C
M proccontrol/src/int_event.h
M proccontrol/src/int_thread_db.C
M proccontrol/src/linux.C
M proccontrol/src/process.C
M proccontrol/src/response.C
M proccontrol/src/response.h
Log Message:
-----------
proccontrol: fix thread-exit crash/hang races
Repeatedly running the testsuite's test_thread_1 (a mutatee that spawns
threads which exit in a burst) sporadically crashed or hung the mutator.
Investigation with core dumps, live-process interrogation, and event
tracing found a chain of independent races in proccontrol, each masked
by the more frequent one before it. They are only jointly testable --
each fix unmasks the next -- so they land as one commit:
1. Use-after-free of int_thread cached in events (SIGSEGV).
int_eventBreakpoint::thrd and int_eventNewUserThread::thr held raw
int_thread* pointers. A breakpoint/thread-create event can outlive
the thread that triggered it (the thread exits while the event is
still queued or in the callback/muxer layer); the int_thread is then
deleted and the cached pointer dangles. Store the reference-counted
Thread wrapper instead: it survives thread destruction and its
accessors are exit-safe (llthrd() returns NULL, getLWP() falls back
to the exit state). Guard the dereference sites accordingly
(lookupInstalledBreakpoint, getBreakpoints, the breakpoint handler
family, ThreadDBCreateHandler, getBreakpoint's NULL mem).
2. Handlers leaking proc-wide state desyncs (hang risk).
The NULL-thread guards in HandleBreakpointContinue/Clear/Restore
must not skip the paired restoreStateProc() of the BreakpointHold /
Breakpoint / BreakpointResume state layers: a skipped restore leaves
every surviving thread force-stopped forever. The restore is a
process-wide walk that only needs a live thread as its handle, so
route it through any thread still in the pool. Likewise, a thread
that dies mid single-step over a cleared breakpoint leaves no
BreakpointRestore event to re-arm the breakpoint and restore the
resume states; perform that recovery in HandleThreadCleanup.
3. thread_db assertion failures (SIGABRT).
getEventForThread() asserts all threads are handler-stopped before
reading thread_db state out of the mutatee. Two races broke it:
exiting/exited threads whose handler state is transiently stale
(allHandlerStopped now skips them; they cannot run), and threads
created after the proc-stopping event was released but before the
dispatch handler ran (the dispatch now defers with ret_again until
the pending thread-create events stop them).
4. Lock use after static destruction (SIGABRT via lock_error).
The response.C id_lock mutexes were plain statics; static destructor
order allowed them to be destroyed before int_cleanup stops the
handler thread, which then locked a destroyed mutex during teardown
(glibc marks it __kind=-1, boost throws). Heap-allocate and
intentionally leak them, mirroring the existing Counter::locks fix.
5. Generator eviction livelock at exit (hang, 100% CPU).
GeneratorLinux::evictFromWaitpid() spins until its SIGUSR2 handler
fires, but the generator can exit on its own initiative concurrently
and the queued signal is then discarded, so the flag never fires.
Bound the spin: the eviction is best-effort during process exit.
6. Undeliverable SIGSTOP to a dying thread (deadlock; the main hang).
If intStop() sends SIGSTOP to a thread whose exit event is already
in flight (PTRACE_EVENT_EXIT observed), the stop can never be
delivered: the thread just dies. The PendingStop state layer then
stays desync'd to running forever, which (a) blocks isStopped() for
every higher layer, so held proc-stopper breakpoint events are never
released, and (b) forces a run target for a thread that can only
die, deadlocking the process with the mutatee ptrace-stopped and the
generator idle. LinuxHandleLWPDestroy was written for exactly this
case (clear pending stops on pre-LWPDestroy; its comment describes
the bug) but was never registered with the handler pool -- register
it, guard it against already-destroyed threads, and additionally
skip intStop() for threads already observed exiting. The two halves
are intentionally redundant: the intStop() guard covers stops
requested after the exit was observed, the handler covers stops
requested before it (SIGSTOP landing on a thread already inside the
exit syscall).
Also make the prepEvent/checkEvents debug prints safe for held events
whose thread has already exited.
Notes for reviewers:
- allHandlerStopped() has one other caller, in the FreeBSD startup
path; skipping exiting threads is believed equally correct there (a
thread that cannot run cannot violate quiescence) but was not tested
on that platform.
- Handler::ret_again existed but had no in-tree users before this
change; the thread_db dispatch deferral is its first use.
Verified with a tuned reproducer (test_thread_1 with a small thread
count, which widens the exit-race windows): each failure mode was
reproduced and bisected via control experiments before fixing, and the
full stack survives thousands of consecutive runs (individual failure
rates were between 1/10 and 1/850 beforehand).
Co-Authored-By: Claude Fable 5 <noreply@xxxxxxxxxxxxx>
To unsubscribe from these emails, change your notification settings at https://github.com/dyninst/dyninst/settings/notifications
|