-
Notifications
You must be signed in to change notification settings - Fork 5.3k
fix: [kernel] [thread] Resolve delayed thread wakeup bug when calling suspend repeatedly #10970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Refactor thread suspension logic to improve clarity and correctness.
|
👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread! 为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。 🛠 操作步骤 | Steps
完成后,提交将自动更新至 如有问题欢迎联系我们,再次感谢您的贡献!💐 |
📌 Code Review Assignment🏷️ Tag: kernelReviewers: GorrayLi ReviewSun hamburger-os lianux-mm wdfk-prog xu18838022837 Changed Files (Click to expand)
📊 Current Review Status (Last Updated: 2025-12-10 21:39 CST)
📝 Review Instructions
|
|
@Rbb666 麻烦review一下呢🌹 |
Removed unnecessary blank line in thread.c.
我要明天测试下哈,最近有点忙 |
👀还在吗 |
|
@Kurngsy 我测试了下,没什么太大问题 ,然后我发现有部分代码有高度重合部分,所以微调了下,请作者看下谢谢 diff --git a/src/thread.c b/src/thread.c
index 481dfebeef..1a9b8f4027 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -885,26 +885,27 @@ RTM_EXPORT(rt_thread_control);
#include <lwp_signal.h>
#endif
-static void _thread_set_suspend_state(struct rt_thread *thread, int suspend_flag)
+/* Convert suspend_flag to corresponding thread suspend state value */
+static rt_uint8_t _thread_get_suspend_state(int suspend_flag)
{
- rt_uint8_t stat = RT_THREAD_SUSPEND_UNINTERRUPTIBLE;
-
- RT_ASSERT(thread != RT_NULL);
switch (suspend_flag)
{
case RT_INTERRUPTIBLE:
- stat = RT_THREAD_SUSPEND_INTERRUPTIBLE;
- break;
+ return RT_THREAD_SUSPEND_INTERRUPTIBLE;
case RT_KILLABLE:
- stat = RT_THREAD_SUSPEND_KILLABLE;
- break;
+ return RT_THREAD_SUSPEND_KILLABLE;
case RT_UNINTERRUPTIBLE:
- stat = RT_THREAD_SUSPEND_UNINTERRUPTIBLE;
- break;
default:
- RT_ASSERT(0);
- break;
+ return RT_THREAD_SUSPEND_UNINTERRUPTIBLE;
}
+}
+
+static void _thread_set_suspend_state(struct rt_thread *thread, int suspend_flag)
+{
+ rt_uint8_t stat;
+
+ RT_ASSERT(thread != RT_NULL);
+ stat = _thread_get_suspend_state(suspend_flag);
RT_SCHED_CTX(thread).stat = stat | (RT_SCHED_CTX(thread).stat & ~RT_THREAD_STAT_MASK);
@@ -943,13 +944,25 @@ rt_err_t rt_thread_suspend_to_list(rt_thread_t thread, rt_list_t *susp_list, int
RT_ASSERT(thread != RT_NULL);
RT_ASSERT(rt_object_get_type((rt_object_t)thread) == RT_Object_Class_Thread);
- LOG_D("thread suspend: %s", thread->parent.name);
+ LOG_D("thread suspend: %s", thread->parent.name);
rt_sched_lock(&slvl);
stat = rt_sched_thread_get_stat(thread);
- if (stat == RT_THREAD_SUSPEND)
+ if (stat & RT_THREAD_SUSPEND_MASK)
{
+ if (RT_SCHED_CTX(thread).sched_flag_ttmr_set == 1)
+ {
+ /* The new suspend operation will halt the tick timer. */
+ LOG_D("Thread [%s]'s timer has been halted.\n", thread->parent.name);
+ rt_sched_thread_timer_stop(thread);
+ }
+ /* Upgrade suspend state if new state is stricter */
+ if (stat < _thread_get_suspend_state(suspend_flag))
+ {
+ _thread_set_suspend_state(thread, suspend_flag);
+ }
+
rt_sched_unlock(slvl);
/* Already suspended, just set status to success. */
return RT_EOK;测试代码: /*
* 问题:线程A调用rt_thread_mdelay()进入延时挂起后,线程B调用rt_thread_suspend(A)
* 旧代码:直接返回RT_EOK,不停止定时器,定时器超时后线程A被唤醒(BUG)
*/
#include <rtthread.h>
#define THREAD_STACK_SIZE 2048
#define THREAD_PRIORITY 25
#define THREAD_TIMESLICE 5
static volatile rt_bool_t thread_woken_up = RT_FALSE;
static rt_thread_t test_thread = RT_NULL;
/**
* 测试线程入口
*/
static void delay_thread_entry(void *parameter)
{
rt_uint32_t delay_ms = (rt_uint32_t)(rt_ubase_t)parameter;
/* 进入延时挂起状态 */
rt_thread_mdelay(delay_ms);
/* 如果执行到这里,说明线程被唤醒了 */
thread_woken_up = RT_TRUE;
rt_kprintf("[Thread] Woken up!\n");
while (1)
{
rt_thread_mdelay(10000);
}
}
/**
* 测试:验证对已延时挂起的线程再次挂起时,定时器是否被停止
*/
int test_pr10970(void)
{
rt_err_t result;
rt_base_t stat;
rt_uint32_t thread_delay_ms = 300;
rt_uint32_t wait_after_suspend = 500;
rt_kprintf("\n=== PR #10970 Test: Timer halt on double suspend ===\n");
thread_woken_up = RT_FALSE;
/* 创建并启动测试线程 */
test_thread = rt_thread_create("t_delay",
delay_thread_entry,
(void *)(rt_ubase_t)thread_delay_ms,
THREAD_STACK_SIZE,
THREAD_PRIORITY,
THREAD_TIMESLICE);
if (test_thread == RT_NULL)
{
rt_kprintf("[FAIL] Create thread failed!\n");
return -1;
}
rt_thread_startup(test_thread);
/* 等待线程进入 delay 状态 */
rt_thread_mdelay(50);
/* 对已在 delay 的线程调用 suspend */
result = rt_thread_suspend(test_thread);
rt_kprintf("rt_thread_suspend() = %d\n", result);
rt_schedule();
/* 等待超过线程原本的 delay 时间 */
rt_kprintf("Waiting %d ms (thread delay was %d ms)...\n", wait_after_suspend, thread_delay_ms);
rt_thread_mdelay(wait_after_suspend);
/* 检查结果 */
stat = RT_SCHED_CTX(test_thread).stat & RT_THREAD_STAT_MASK;
rt_bool_t test_passed = (!thread_woken_up && (stat & RT_THREAD_SUSPEND_MASK));
rt_kprintf("\n--- Result ---\n");
rt_kprintf("Thread woken up: %s\n", thread_woken_up ? "YES" : "NO");
rt_kprintf("Thread state: 0x%02x\n", (unsigned int)stat);
if (test_passed)
{
rt_kprintf("[PASS] Timer stopped, PR #10970 working!\n");
}
else
{
rt_kprintf("[FAIL] Timer NOT stopped, PR #10970 NOT applied!\n");
}
/* 清理 */
rt_thread_resume(test_thread);
rt_thread_mdelay(50);
rt_thread_delete(test_thread);
test_thread = RT_NULL;
return test_passed ? 0 : -1;
}
MSH_CMD_EXPORT(test_pr10970, Test PR 10970 - timer halt on double suspend); |
( ͡• ͜ʖ ͡• ) 很好的改动,我还需要在重新修改后提交吗 |
可以的,你这边检查没问题,可以覆盖这个PR的提交 |
|
然后PR的标题建议修改下,简要说清楚这个PR主要修改的目的是什么,解决了什么问题 |
Refactor thread suspension logic to improve clarity and maintainability.
麻烦review一下 |
src/thread.c
Outdated
| case RT_KILLABLE: | ||
| stat = RT_THREAD_SUSPEND_KILLABLE; | ||
| break; | ||
| return RT_THREAD_SUSPEND_KILLABLE; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return后一个空格
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
好
… suspend repeatedly (RT-Thread#10970) * Enhance thread suspend function with stricter checks Refactor thread suspension logic to improve clarity and correctness. * Clean up formatting in thread.c Removed unnecessary blank line in thread.c. * Refactor thread suspend state handling Refactor thread suspension logic to improve clarity and maintainability. * Update thread.c * Fix indentation for RT_THREAD_SUSPEND_KILLABLE case
Pull Request 提交信息
为什么提交这份 PR (Why to Submit This PR)
修复
rt_thread_suspend_to_list()函数对已处于延时挂起状态线程的处理缺陷,补全重复挂起场景下的定时器管理逻辑,确保线程挂起语义的一致性。线程因
rt_thread_mdelay()进入延时挂起(依赖定时器唤醒)后,调用rt_thread_suspend_to_list()对其重复挂起时,原代码仅返回成功但未停止定时器,导致定时器超时后线程被错误唤醒,违背挂起操作的设计初衷。你的解决方案是什么 (What is Your Solution)
RT_THREAD_SUSPEND状态校验,改为RT_THREAD_SUSPEND_MASK掩码校验,支持识别延时挂起等子类挂起状态。rt_sched_thread_timer_stop()停止定时器,避免线程被错误唤醒。验证的 BSP 和 Config (Provide the Config and BSP)
rt-thread-master\bsp\nxp\imx\imxrt\imxrt1064-nxp-evk当前拉取/合并请求的状态 (Intent for Your PR)
必须选择一项 Choose one (Mandatory):
代码质量 (Code Quality)
我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:
#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up