Commit Graph
1 Commits
Author SHA1 Message Date
Jonathan BennettandClaude Opus 5 329b7ea918 Fail the native simulator job on a sanitizer fault, and fix the one it found (#12205)
* Fail the native simulator job on a sanitizer fault, and fix the one it found

The integration step backgrounded meshtasticd and ended in `wait "$PID"
2>/dev/null || true`, which discards the simulator's exit status. The coverage
env is built with -fsanitize=address, so a fault there prints a full report and
exits non-zero on the way out, after the Python harness has already printed its
own success - and the job concluded green with an AddressSanitizer
heap-use-after-free sitting in the log.

Gate it: capture the simulator's output, read the status `wait` reports, and
scan the output for a sanitizer report. The pattern moves to bin/lib/sanitizer.sh
so bin/run-tests.sh and the new bin/check-sanitizer-log.sh share one matcher
rather than drifting apart; the output is uploaded as an artifact, since a fault
on the exit path prints after the last test has passed. Leak detection is off,
as in bin/test-config-check.sh - exit_simulator exit()s with the node's globals
still live.

The fault it had been hiding: OSThread::run() points currentThread at the
running thread for the whole of runOnce(), and exit_simulator calls exit(0) from
AdminModule while ServerAPI is that thread. ServerAPI is declared
`public StreamAPI, private OSThread`, so its Thread base frees ThreadName first
and PhoneAPI::~PhoneAPI's own LOG_DEBUG then reads it back through
currentThread. Clear the pointer in ~OSThread when it names the thread being
destroyed, which covers any thread deleted from inside its own runOnce, not
just this one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JjAvRt6xuztC3hKqWkW9Dw

* Detach observers when an Observable is destroyed

The new sanitizer gate turned the simulator job red on a second
heap-use-after-free on the same exit path, this one in
Observable::removeObserver.

~Observer removes itself from every observable it watches, but Observable has
no destructor and dies with its watchers still pointing at it. exit_simulator
exit()s from AdminModule, and static destruction takes the global xModem - whose
packetReady is an Observable that PhoneAPI::handleStartConfig attached to -
before the heap-allocated ServerAPI that is watching it. PhoneAPI::close() then
calls unobserve(&xModem.packetReady) on the dead observable, and ~Observer would
walk it again afterwards.

Both halves of the contract, so neither side has to outlive the other:
~Observable drops itself from each watcher's list, and unobserve() checks
membership before reading through the pointer it is given.

Also take CodeRabbit's grep -m 4 in bin/check-sanitizer-log.sh: with more than
four matching lines, head closing the pipe is a SIGPIPE that pipefail reports
and set -e exits on, so the script gave 141 where it documents 1.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JjAvRt6xuztC3hKqWkW9Dw

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
2026-10-20 14:07:34 +00:00