[Bug] rt_event_send can race with an in-flight thread timeout and resume a thread twice

[illustriousness]

RT-Thread Version

master at 997de9433af42aacb1988489ccb08b6688f440cc.

The affected code is still present in current master:

• src/timer.c: _timer_check() removes/deactivates a timer and unlocks before calling its callback
• src/scheduler_comm.c: rt_sched_thread_timer_stop() clears sched_flag_ttmr_set even when rt_timer_stop() fails
• src/ipc.c: rt_event_send() ignores the return value of rt_sched_thread_ready()
• src/thread.c: _thread_timeout() asserts that the thread must still be suspended

Relevant configuration:

RT_TICK_PER_SECOND=1000
RT_USING_EVENT=y
single-core Cortex-M
hard thread timers (RT_USING_TIMER_SOFT is disabled)

Hardware Type/Architectures

GD32F303ZKT6, Cortex-M4 / ARMv7E-M, single core.

The same race should apply to other single-core targets where an interrupt that calls rt_event_send() can preempt the system tick handler.

Interrupt priorities in the failing system:

SysTick/PendSV effective priority: 0xF0 (lowest)
USART IRQ priority:                0x00
SCB->SHPR3 at failure:             0xf0f00000

Develop Toolchain

GCC: arm-none-eabi-gcc 14.2.1 20241119

Describe the bug

A thread waits on an event with a finite timeout. When its thread timer expires, _timer_check() removes the timer from the timer list and clears RT_TIMER_FLAG_ACTIVATED, then temporarily restores interrupts before calling _thread_timeout().

A higher-priority ISR can enter this window and call rt_event_send() more than once. The first wakeup sees sched_flag_ttmr_set == 1, but rt_timer_stop() fails because the timer has already been removed/deactivated by _timer_check(). rt_sched_thread_timer_stop() nevertheless clears sched_flag_ttmr_set.

A second event send can then see sched_flag_ttmr_set == 0 and successfully make the same thread ready/running. When execution returns to SysTick, _thread_timeout() is still called for the old timer and fails this assertion:

RT_ASSERT(rt_sched_thread_is_suspended(thread));

Observed backtrace

#0  rt_assert_handler
#1  _thread_timeout        src/thread.c
#2  _timer_check           src/timer.c
#3  rt_timer_check         src/timer.c
#4  rt_tick_increase       src/clock.c
#5  SysTick_Handler

Thread/timer state captured with GDB

thread.name                = "run_mtr"
thread.stat                = 3     (RT_THREAD_RUNNING)
sched_flag_ttmr_set        = 0
thread_timer.parent.flag   = 0x08  (THREAD_TIMER set, ACTIVATED cleared)
thread_timer.timeout_tick  = 735028
current_tick               = 735028
thread.event_set           = 0x02
thread.event_info          = 0x06  (OR | CLEAR)

A context switch from idle to the event waiter had been requested but PendSV had not executed yet:

rt_interrupt_from_thread        = &idle_thread.sp
rt_interrupt_to_thread          = &event_waiter.sp
rt_thread_switch_interrupt_flag = 1

This rules out a stack overflow or a corrupted thread object: the event path had already marked the waiter running while the timeout callback was still in flight.

Race sequence

SysTick: _timer_check()
  remove thread timer from timer list
  clear RT_TIMER_FLAG_ACTIVATED
  restore interrupts
                         |
                         +-- higher-priority ISR
                         |     rt_event_send(event)
                         |       rt_timer_stop() -> -RT_ERROR
                         |       sched_flag_ttmr_set is cleared anyway
                         |       rt_sched_thread_ready() failure is ignored
                         |
                         |     rt_event_send(event) again
                         |       sched_flag_ttmr_set == 0
                         |       thread is moved to READY/RUNNING
                         |
SysTick resumes
  _thread_timeout(thread)
  assert(thread is suspended) fails

Hardware stress reproducer

A minimal application can use one event waiter and a hardware timer IRQ whose priority is higher than SysTick. Use a hardware timer period close to, but not exactly equal to, one OS tick so its phase sweeps through the timeout window.

static struct rt_event race_event;

static void race_waiter(void *parameter)
{
    rt_uint32_t received;

    RT_UNUSED(parameter);

    while (1)
    {
        received = 0;
        (void)rt_event_recv(&race_event,
                            0x01,
                            RT_EVENT_FLAG_OR | RT_EVENT_FLAG_CLEAR,
                            1,
                            &received);
    }
}

void HIGH_PRIORITY_TIMER_IRQHandler(void)
{
    rt_interrupt_enter();
    clear_hardware_timer_irq();

    /* Two producers or two notifications are required to expose the
       incorrect clearing of sched_flag_ttmr_set. */
    (void)rt_event_send(&race_event, 0x01);
    (void)rt_event_send(&race_event, 0x01);

    rt_interrupt_leave();
}

Suggested setup:

OS tick:             1000 us
hardware timer:      approximately 1001 us
hardware timer IRQ:  higher priority than SysTick
wait timeout:        1 tick

Deterministic regression test

For an upstream kernel regression test, timing dependence can be removed with a test-only hook immediately after _timer_check() restores interrupts and immediately before t->timeout_func() is called:

rt_spin_unlock_irqrestore(lock, level);

/* Test-only fault-injection hook. */
thread_timer_expiry_window_hook(t);

t->timeout_func(t->parameter);

The hook sends the waited event twice. Before the fix, this deterministically makes the second send resume the thread and _thread_timeout() asserts. After the fix, timeout remains the single owner, the callback completes, and the pending event is consumed by the next rt_event_recv().

Expected behavior

Only one path should win:

• If event wakeup stops the timer successfully, the event path resumes the thread.
• If _timer_check() has already taken the timer, all competing wakeups must leave the thread suspended until _thread_timeout() completes.
• An event must only be cleared and reported as delivered when rt_sched_thread_ready() succeeds.

Proposed fix

I tested the following three coordinated changes locally:

1. In rt_sched_thread_timer_stop(), clear sched_flag_ttmr_set only when rt_timer_stop() succeeds. A failed stop keeps later wakeups from bypassing the in-flight timeout callback.
2. In _thread_timeout(), clear sched_flag_ttmr_set after acquiring the scheduler lock and confirming the thread is suspended. The timeout callback now owns completion of the timer.
3. In rt_event_send(), only clear the event, set thread->error = RT_EOK, and request scheduling when rt_sched_thread_ready() returns RT_EOK.

Core logic of the patch:

--- a/src/scheduler_comm.c
+++ b/src/scheduler_comm.c
@@
     error = rt_timer_stop(&thread->thread_timer);
-    RT_SCHED_CTX(thread).sched_flag_ttmr_set = 0;
+    if (error == RT_EOK)
+        RT_SCHED_CTX(thread).sched_flag_ttmr_set = 0;

--- a/src/thread.c
+++ b/src/thread.c
@@
     RT_ASSERT(rt_sched_thread_is_suspended(thread));
+    RT_SCHED_CTX(thread).sched_flag_ttmr_set = 0;

--- a/src/ipc.c
+++ b/src/ipc.c
@@
-    rt_sched_thread_ready(thread);
-    thread->error = RT_EOK;
-    need_schedule = RT_TRUE;
+    status = rt_sched_thread_ready(thread);
+    if (status == RT_EOK)
+    {
+        /* Consume/clear the event here, after successful resume. */
+        thread->error = RT_EOK;
+        need_schedule = RT_TRUE;
+    }

The complete local firmware builds successfully with this change. Simply removing the assertion or lowering the producer IRQ priority hides the failure but does not fix event consumption or the duplicate-resume race.

Other additional context

The original failure occurred naturally with two independent high-priority UART RX sources that both notify the same event waiter. It occurred about 735 seconds after boot.

No application memory corruption was observed:

• The thread object type and object-list links were valid.
• The defunct list was empty.
• The 1 KiB waiter stack had approximately 668 bytes free.
• The captured timer and pending PendSV state consistently identify the event/timeout race described above.

[illustriousness]

Follow-up: static audit of other timed IPC wait paths

The original report is a hardware-reproduced rt_event failure. I also audited other IPC users of rt_sched_thread_ready() / rt_susp_list_dequeue() at the same commit. The following is a static audit, not a claim that every item has already been reproduced on hardware.

The common sched_flag_ttmr_set race is not event-specific. rt_susp_list_dequeue() correctly returns RT_NULL when rt_sched_thread_ready() loses to an in-flight timeout, but several IPC callers ignore that return value. With the old unconditional timer-flag clear, the sequence can be:

1. A first producer tries to wake the waiter after its timer has expired.
2. rt_sched_thread_timer_stop() fails because the timeout callback owns the timer, but incorrectly clears sched_flag_ttmr_set.
3. A second producer now bypasses timer ownership and readies the same thread.
4. _thread_timeout() resumes it again and hits RT_ASSERT(rt_sched_thread_is_suspended(thread)).

Potentially affected paths:

IPC	Wake path	Static-audit result
event	rt_event_send()	Reproduced; the original report covers the lost event bits and double-ready assertion.
semaphore	rt_sem_release()	Repeated releases can reach the same old-kernel double-ready race. There is also a semantic follow-up: after the common timer fix, if dequeue loses to timeout and returns RT_NULL, the released token should be retained in sem->value; the current suspended-list branch appears to drop it.
mailbox	send/urgent/receive wakeups	Several dequeue return values are ignored. The old flag bug permits the same double-ready pattern. With the common fix, the mail remains in the ring, although the boundary waiter may legitimately finish with timeout.
message queue	send/urgent/receive wakeups	Same class as mailbox: old double-ready exposure; after the common fix, the message/resource remains queued, but the boundary waiter can time out.
memory pool	rt_mp_free()	The ready result is checked, so resource accounting is better, but the old timer-flag bug still makes a repeated producer sequence unsafe. The common fix should leave the block free when timeout wins.
data queue	push/pop counterpart wakeups	The ready result is checked; it still shares the underlying timer ownership race, while queue data/space remains observable if timeout wins.
mutex	rt_mutex_release()	The ready result is checked and the producer runs in thread context, so the specific SysTick preemption window is less exposed on UP. It should still be included in scheduler-level regression coverage.
completion	completion flag plus waiter wake	Different/coalescing state semantics; I did not find the same immediate resource-loss pattern, but it should still be covered by timed-wait tests.

This suggests splitting the fix/verification into two layers:

1. Common scheduler fix: only clear sched_flag_ttmr_set after successfully removing the timer. This prevents timeout and producer paths from both owning the same waiter for every caller.
2. Per-IPC ownership semantics: when rt_susp_list_dequeue() / rt_sched_thread_ready() reports that timeout already owns the waiter, retain the event bit, semaphore token, message, free block, or queue capacity as appropriate. The event change in the original report solves this specifically for events; semaphore release looks like the highest-priority additional audit item.

Suggested deterministic regression matrix:

• Use a finite wait of one tick.
• At the timer-expiry point, inject two producer operations before _thread_timeout() resumes the waiter (the test hook described in the issue makes this deterministic).
• Cover two event sends, two semaphore releases, two mailbox sends, two message-queue sends, two memory-pool frees where valid, and the corresponding data-queue operation.
• Assert that there is no double-ready/assertion and exactly one side owns completion of the wait.
• If the timeout side already owns the waiter, assert that the produced resource remains observable: event bits stay set, semaphore count increases, messages remain queued, and freed blocks remain free.

So I believe the scheduler timer-ownership fix should be treated as a common kernel fix, while the event and semaphore resource-delivery behavior needs separate IPC-level regression assertions.

[Rbb666]

分析了下问题，相当于recv线程本来是suspend状态被ISR打断了，在ISR中错误修改了recv线程的状态，导致异常

[illustriousness]

是的，这个判断方向是对的。补充一下具体边界：ISR 中调用 rt_event_send() 本身是合法用法，错误不是应用层在中断里发送 event，而是内核在 timeout callback 已经取得所有权之后，错误地允许 ISR 改变了 recv 线程状态。

精确的状态变化是：

1. SysTick 的 _timer_check() 已经摘除并 deactivate 线程定时器，然后重新开放中断，但还没调用 _thread_timeout()；此时 recv 线程仍是 suspend。
2. ISR 第一次 rt_event_send() 调用 rt_sched_thread_ready()，其中 rt_timer_stop() 因定时器已被摘除而失败。这里应该保留 sched_flag_ttmr_set = 1，表示 timeout callback 仍拥有该线程；但当前代码无论成功失败都把它清零，因此第一次 ready 虽然失败，却破坏了 ownership 状态。
3. ISR 第二次 rt_event_send() 看到 flag 已为 0，绕过 timer stop，把同一个 recv 线程改成 READY/RUNNING。
4. 返回 SysTick 后 _thread_timeout() 继续执行，发现线程已不是 suspend，于是触发断言。

我刚核对了当前 master 5363e4ed4cff5f300b3773aa3422a99c8ac687e5，上述逻辑仍然存在。

因此建议修复点仍是：

• rt_sched_thread_timer_stop() 只有在 stop 成功时才清 sched_flag_ttmr_set；
• _thread_timeout() 在确认线程仍 suspend、正式接管超时完成后清该 flag；
• rt_event_send() 只有在 rt_sched_thread_ready() 成功时，才消费 event、设置 RT_EOK 并触发调度。

这样无需限制 ISR 里发送 event，也不依赖调整中断优先级。若这个处理方向认可，我可以继续整理成最小 PR，并附上可稳定打中该窗口的回归测试。

[Rbb666]

是的，这个判断方向是对的。补充一下具体边界：ISR 中调用 rt_event_send() 本身是合法用法，错误不是应用层在中断里发送 event，而是内核在 timeout callback 已经取得所有权之后，错误地允许 ISR 改变了 recv 线程状态。

精确的状态变化是：

1. SysTick 的 _timer_check() 已经摘除并 deactivate 线程定时器，然后重新开放中断，但还没调用 _thread_timeout()；此时 recv 线程仍是 suspend。
2. ISR 第一次 rt_event_send() 调用 rt_sched_thread_ready()，其中 rt_timer_stop() 因定时器已被摘除而失败。这里应该保留 sched_flag_ttmr_set = 1，表示 timeout callback 仍拥有该线程；但当前代码无论成功失败都把它清零，因此第一次 ready 虽然失败，却破坏了 ownership 状态。
3. ISR 第二次 rt_event_send() 看到 flag 已为 0，绕过 timer stop，把同一个 recv 线程改成 READY/RUNNING。
4. 返回 SysTick 后 _thread_timeout() 继续执行，发现线程已不是 suspend，于是触发断言。

我刚核对了当前 master 5363e4ed4cff5f300b3773aa3422a99c8ac687e5，上述逻辑仍然存在。

因此建议修复点仍是：

• rt_sched_thread_timer_stop() 只有在 stop 成功时才清 sched_flag_ttmr_set；
• _thread_timeout() 在确认线程仍 suspend、正式接管超时完成后清该 flag；
• rt_event_send() 只有在 rt_sched_thread_ready() 成功时，才消费 event、设置 RT_EOK 并触发调度。

这样无需限制 ISR 里发送 event，也不依赖调整中断优先级。若这个处理方向认可，我可以继续整理成最小 PR，并附上可稳定打中该窗口的回归测试。

我在 STM32 平台上对该问题进行了复现测试，确认该现象确实存在。此外，类似的异常在信号量（Semaphore）、消息队列（Queue）、邮箱（Mailbox）和事件集（Event）等IPC 机制中也同样存在。

我认为可以参考该方向进行补丁（Patch）修复。并添加对应的测试用例（包含压力测试），方便大家进行验证。

谢谢！