Files
firmware/src/Observer.h
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

127 lines
3.6 KiB
C++

#pragma once
#include <Arduino.h>
#include <algorithm>
#include <list>
template <class T> class Observable;
/**
* An observer which can be mixed in as a baseclass. Implement onNotify as a method in your class.
*/
template <class T> class Observer
{
std::list<Observable<T> *> observables;
public:
virtual ~Observer();
/// Stop watching the observable
void unobserve(Observable<T> *o);
/// Start watching a specified observable
void observe(Observable<T> *o);
private:
friend class Observable<T>;
protected:
/**
* returns 0 if other observers should continue to be called
* returns !0 if the observe calls should be aborted and this result code returned for notifyObservers
**/
virtual int onNotify(T arg) = 0;
};
/**
* An observer that calls an arbitrary method
*/
template <class Callback, class T> class CallbackObserver : public Observer<T>
{
typedef int (Callback::*ObserverCallback)(T arg);
Callback *objPtr;
ObserverCallback method;
public:
CallbackObserver(Callback *_objPtr, ObserverCallback _method) : objPtr(_objPtr), method(_method) {}
protected:
virtual int onNotify(T arg) override { return (objPtr->*method)(arg); }
};
/**
* An observable class that will notify observers anytime notifyObservers is called. Argument type T can be any type, but for
* performance reasons a pointer or word sized object is recommended.
*/
template <class T> class Observable
{
std::list<Observer<T> *> observers;
public:
~Observable();
/**
* Tell all observers about a change, observers can process arg as they wish
*
* returns !0 if an observer chose to abort processing by returning this code
*/
int notifyObservers(T arg)
{
for (typename std::list<Observer<T> *>::const_iterator iterator = observers.begin(); iterator != observers.end();
++iterator) {
int result = (*iterator)->onNotify(arg);
if (result != 0)
return result;
}
return 0;
}
private:
friend class Observer<T>;
// Not called directly, instead call observer.observe
void addObserver(Observer<T> *o) { observers.push_back(o); }
// cppcheck-suppress constParameterPointer ; std::list::remove() needs a non-const pointer
void removeObserver(Observer<T> *o) { observers.remove(o); }
};
template <class T> Observer<T>::~Observer()
{
for (typename std::list<Observable<T> *>::const_iterator iterator = observables.begin(); iterator != observables.end();
++iterator) {
(*iterator)->removeObserver(this);
}
observables.clear();
}
template <class T> void Observer<T>::unobserve(Observable<T> *o)
{
// Membership before any read through `o`: ~Observable has already dropped a destroyed
// observable from this list, so an unobserve() that runs after it is a no-op, not a use-after-free.
if (std::find(observables.begin(), observables.end(), o) == observables.end())
return;
o->removeObserver(this);
observables.remove(o);
}
template <class T> void Observer<T>::observe(Observable<T> *o)
{
observables.push_back(o);
o->addObserver(this);
}
// Symmetric to ~Observer: an observable destroyed while it still has watchers must drop itself from
// their lists, or their later unobserve() / ~Observer reads through the freed object.
template <class T> Observable<T>::~Observable()
{
for (typename std::list<Observer<T> *>::const_iterator iterator = observers.begin(); iterator != observers.end();
++iterator) {
(*iterator)->observables.remove(this);
}
observers.clear();
}