mirror of
https://github.com/PX4/PX4-Autopilot.git
synced 2026-10-06 09:02:52 +08:00
fix(lockstep_scheduler): stop swallowing wakeups in cond_timedwait
cond_timedwait() re-waited on a 10 ms wall-clock timeout until either a signal or its virtual timeout arrived. A signal from the caller's own signaler (px4_sem_post, or the unit test's broadcast) that lands while the waiter is between that wall-clock timeout and re-acquiring the mutex finds no waiter and is lost, so the waiter sleeps until its virtual timeout. In lockstep_scheduler_test nothing else advances time and the test hangs; in SITL a posted semaphore can report ETIMEDOUT late. The loop is not needed: the caller holds the mutex until pthread_cond_wait() releases it and set_absolute_time() takes that mutex before broadcasting, so a timeout broadcast cannot be missed. The original signal loss came from the MAX_WAKEUPS cap that the signal_next list already removed. Go back to a single wait and keep the three-phase signaling. Refs #28862 Assisted-by: Claude:claude-opus-5-5 Signed-off-by: Ramon Roche <mrpollo@gmail.com>
This commit is contained in:
@@ -107,8 +107,8 @@ void LockstepScheduler::set_absolute_time(uint64_t time_us)
|
||||
}
|
||||
|
||||
// Phase 2: signal each waiter outside _timed_waits_mutex. _signaling_mutex
|
||||
// is still held, so any waiter that sees timeout via the wall-clock
|
||||
// fallback will block in the dance until we are done, preventing
|
||||
// is still held, so any waiter woken while we are still signaling will
|
||||
// block in the dance until we are done, preventing
|
||||
// use-after-free of their stack-local passed_lock/passed_cond.
|
||||
for (TimedWait *tw = to_signal_head; tw != nullptr;) {
|
||||
TimedWait *next = tw->signal_next;
|
||||
@@ -154,32 +154,13 @@ int LockstepScheduler::cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *loc
|
||||
}
|
||||
}
|
||||
|
||||
// Use a short wall-clock timeout instead of waiting indefinitely.
|
||||
// There is a race window between releasing _timed_waits_mutex (above)
|
||||
// and entering pthread_cond_wait: if set_absolute_time() broadcasts
|
||||
// during that window, the signal is lost and we'd block forever.
|
||||
// A periodic wake-up lets us re-check the timeout flag.
|
||||
int result;
|
||||
|
||||
while (true) {
|
||||
struct timespec ts;
|
||||
clock_gettime(CLOCK_REALTIME, &ts);
|
||||
// Wake up every 10ms wall-clock to re-check
|
||||
ts.tv_nsec += 10000000; // 10ms
|
||||
|
||||
if (ts.tv_nsec >= 1000000000) {
|
||||
ts.tv_sec += 1;
|
||||
ts.tv_nsec -= 1000000000;
|
||||
}
|
||||
|
||||
result = pthread_cond_timedwait(cond, lock, &ts);
|
||||
|
||||
if (timed_wait.timeout || result == 0) {
|
||||
break;
|
||||
}
|
||||
|
||||
// ETIMEDOUT from the wall-clock timeout — just re-check
|
||||
}
|
||||
// A single wait, returned on any wakeup. The caller holds 'lock' from
|
||||
// before registering until pthread_cond_wait releases it, and
|
||||
// set_absolute_time() takes 'lock' before broadcasting, so a timeout
|
||||
// broadcast cannot be missed. Re-waiting here would instead swallow a
|
||||
// signal from the caller's own signaler (e.g. px4_sem_post) that lands
|
||||
// while this thread is re-acquiring 'lock'.
|
||||
int result = pthread_cond_wait(cond, lock);
|
||||
|
||||
const bool timeout = timed_wait.timeout;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user