Skip to content

Conversation

@Rbb666
Copy link
Member

@Rbb666 Rbb666 commented Nov 23, 2025

拉取/合并请求描述:(PR description)

[

为什么提交这份PR (why to submit this PR)

修复如下issue的问题:
#6768

你的解决方案是什么 (what is your solution)

添加哨兵标记线程状态,用于检测线程的状态码是否被IPC/信号的唤醒机制修改

image

请提供验证的bsp和config (provide the config and bsp)

  • BSP: QEMU-A9

]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • 代码是高质量的 Code in this PR is of high quality
  • 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

@github-actions
Copy link

👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).


🛠 操作步骤 | Steps

  1. 前往 Actions 页面 | Go to the Actions page
    点击进入工作流 → | Click to open workflow →

  2. 点击 Run workflow | Click Run workflow

  • 设置需排除的文件/目录(目录请以"/"结尾)
    Set files/directories to exclude (directories should end with "/")
  • 将目标分支设置为 \ Set the target branch to:fix-thread-error-status
  • 设置PR number为 \ Set the PR number to:10972
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将自动推送至你的分支。
    The formatted code will be automatically pushed to your branch.

完成后,提交将自动更新至 fix-thread-error-status 分支,关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the fix-thread-error-status branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们,再次感谢您的贡献!💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

@github-actions github-actions bot added the Kernel PR has src relate code label Nov 23, 2025
@github-actions
Copy link

github-actions bot commented Nov 23, 2025

📌 Code Review Assignment

🏷️ Tag: kernel

Reviewers: GorrayLi ReviewSun hamburger-os lianux-mm wdfk-prog xu18838022837

Changed Files (Click to expand)
  • src/ipc.c
  • src/klibc/kerrno.c
  • src/thread.c

📊 Current Review Status (Last Updated: 2025-11-23 22:35 CST)

  • GorrayLi Pending Review
  • ReviewSun Pending Review
  • hamburger-os Pending Review
  • lianux-mm Pending Review
  • wdfk-prog Pending Review
  • xu18838022837 Pending Review

📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

Copilot finished reviewing on behalf of Rbb666 November 23, 2025 06:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes issue #6768 by introducing a sentinel error code RT_EWAITPEND to avoid spurious error displays in the list_thread command. The solution prevents temporary waiting states from being shown as errors by using an internal sentinel value that gets normalized to the appropriate error code after thread scheduling.

Key changes:

  • Introduces RT_EWAITPEND as an internal sentinel error code mapped to "OK" in error string display
  • Adds _ipc_wait_result() helper function to normalize sentinel values back to RT_EINTR
  • Updates all IPC wait operations (semaphore, mutex, event, mailbox, message queue) and thread sleep to use the sentinel pattern

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
include/klibc/kerrno.h Defines RT_EWAITPEND error code (19 for non-libc, EINPROGRESS for libc builds)
src/klibc/kerrno.c Maps RT_EWAITPEND to "OK" string to avoid spurious error display
src/thread.c Updates thread sleep to use RT_EWAITPEND sentinel before scheduling
src/ipc.c Adds _ipc_wait_result() normalization function and updates all IPC wait operations to use RT_EWAITPEND sentinel pattern

src/ipc.c Outdated
* When the return value is RT_EINTR, the operation was interrupted (interrupted system call).
* When the return value is RT_ETIMEOUT, the operation timed out.
*
* @see _ipc_mark_thread_waiting()
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Documentation/文档]: Invalid function reference in @see tag / @see 标签中的无效函数引用

English: The @see _ipc_mark_thread_waiting() reference is invalid - this function does not exist in the codebase. Please remove this line or update it to reference a valid function.

中文:@see _ipc_mark_thread_waiting() 引用无效 - 此函数在代码库中不存在。请删除此行或更新为有效的函数引用。

Suggested change
* @see _ipc_mark_thread_waiting()

Copilot uses AI. Check for mistakes.
#define RT_ENOBUFS 16 /**< No buffer space is available */
#define RT_ESCHEDISR 17 /**< scheduler failure in isr context */
#define RT_ESCHEDLOCKED 18 /**< scheduler failure in critical region */
#define RT_EWAITPEND 19 /**< Wait operation pending */
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Documentation/文档]: Inconsistent documentation between LIBC and non-LIBC builds / LIBC 和非 LIBC 构建之间的文档不一致

English: The documentation comment for RT_EWAITPEND is inconsistent between the two definitions. Line 36 says "Wait operation pending (internal sentinel)" while line 63 only says "Wait operation pending". Please add "(internal sentinel)" to line 63's comment for consistency, as this is a crucial detail indicating it's not meant to be exposed to users.

中文:RT_EWAITPEND 在两个定义之间的文档注释不一致。第 36 行说"Wait operation pending (internal sentinel)",而第 63 行只说"Wait operation pending"。请在第 63 行的注释中添加"(internal sentinel)"以保持一致性,因为这是一个关键细节,表明它不应向用户公开。

Suggested/建议:

#define RT_EWAITPEND                    19              /**< Wait operation pending (internal sentinel) */
Suggested change
#define RT_EWAITPEND 19 /**< Wait operation pending */
#define RT_EWAITPEND 19 /**< Wait operation pending (internal sentinel) */

Copilot uses AI. Check for mistakes.
src/ipc.c Outdated
Comment on lines 96 to 97
* the thread was interrupted by an interrupted system call rather than successfully
* acquiring the resource or timing out.
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Spelling/拼写]: Typo in comment / 注释中的拼写错误

English: "interrupted system call" is redundant. The phrase should be "interrupted" since EINTR already means "interrupted system call". Alternatively, you could say "the thread was interrupted" or "the operation was interrupted by a signal".

中文:"interrupted system call" 有冗余。应该使用 "interrupted",因为 EINTR 本身就表示"中断的系统调用"。或者可以说"the thread was interrupted"或"the operation was interrupted by a signal"。

Original: "the thread was interrupted by an interrupted system call"
Suggested/建议: "the thread was interrupted" or "the operation was interrupted by a signal"

Suggested change
* the thread was interrupted by an interrupted system call rather than successfully
* acquiring the resource or timing out.
* the thread was interrupted rather than successfully acquiring the resource or timing out.
* (i.e. the operation was interrupted by a signal)

Copilot uses AI. Check for mistakes.
src/ipc.c Outdated
* 2022-10-16 Bernard add prioceiling feature in mutex
* 2023-04-16 Xin-zheqi redesigen queue recv and send function return real message size
* 2023-09-15 xqyjlj perf rt_hw_interrupt_disable/enable
* 2025-11-23 Rbb666 fix thread error code to avoid spurious error display in ps
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Spelling/拼写]: Typo in changelog comment / 变更日志注释中的拼写错误

English: "ps" should be "list_thread" to match the PR title and description. The issue is about spurious error display in list_thread command, not ps command.

中文:应将 "ps" 改为 "list_thread" 以匹配 PR 标题和描述。该问题是关于 list_thread 命令中的错误显示,而不是 ps 命令。

Original: "fix thread error code to avoid spurious error display in ps"
Corrected/更正: "fix thread error code to avoid spurious error display in list_thread"

Suggested change
* 2025-11-23 Rbb666 fix thread error code to avoid spurious error display in ps
* 2025-11-23 Rbb666 fix thread error code to avoid spurious error display in list_thread

Copilot uses AI. Check for mistakes.
src/ipc.c Outdated
Comment on lines 96 to 97
* the thread was interrupted by an interrupted system call rather than successfully
* acquiring the resource or timing out.
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Documentation/文档]: Inaccurate function documentation / 函数文档不准确

English: The documentation states "the thread was interrupted by an interrupted system call rather than successfully acquiring the resource or timing out", but this is confusing. It should clarify that RT_EWAITPEND indicates the thread is still in a waiting state and hasn't been woken up by resource acquisition or timeout. When it remains RT_EWAITPEND after scheduling, it means the thread was woken by a signal/interrupt instead.

中文:文档说"the thread was interrupted by an interrupted system call rather than successfully acquiring the resource or timing out",但这令人困惑。应该澄清 RT_EWAITPEND 表示线程仍处于等待状态,尚未被资源获取或超时唤醒。当调度后仍为 RT_EWAITPEND 时,表示线程是被信号/中断唤醒的。

Suggested/建议:

 * @note     This function translates the internal RT_EWAITPEND sentinel value back to RT_EINTR
 *           for the caller. If the error code is still RT_EWAITPEND after scheduling, it means
 *           the thread was woken by a signal rather than by successfully acquiring the resource 
 *           or timing out.
Suggested change
* the thread was interrupted by an interrupted system call rather than successfully
* acquiring the resource or timing out.
* the thread is still in a waiting state and hasn't been woken up by resource acquisition or timeout.
* When it remains RT_EWAITPEND after scheduling, it means the thread was woken by a signal/interrupt instead.

Copilot uses AI. Check for mistakes.
src/ipc.c Outdated
Comment on lines 2239 to 2241
/* unwrap sentinel: signal -> -RT_EINTR, timeout already set */
/* convert sentinel back to EINTR/timeout semantics */
/* translate sentinel back to EINTR or timeout */
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Maintainability/可维护性]: Redundant comments / 冗余注释

English: Four consecutive comments on lines 2239-2242 are saying the same thing. Please keep only one clear comment.

中文:2239-2242 行的四个连续注释表达了相同的意思。请只保留一个清晰的注释。

Suggested/建议:

/* normalize sentinel before reporting to caller */
ret = _ipc_wait_result(thread);
Suggested change
/* unwrap sentinel: signal -> -RT_EINTR, timeout already set */
/* convert sentinel back to EINTR/timeout semantics */
/* translate sentinel back to EINTR or timeout */

Copilot uses AI. Check for mistakes.
@Rbb666 Rbb666 force-pushed the fix-thread-error-status branch 3 times, most recently from 32aeba5 to 76325bf Compare November 23, 2025 07:12
@Rbb666 Rbb666 force-pushed the fix-thread-error-status branch from 76325bf to 7d74213 Compare November 23, 2025 14:33
{RT_ENOENT , "ENOENT "}, /**< No such file or directory. */
{RT_ENOSPC , "ENOSPC "}, /**< No space left on device. */
{RT_EPERM , "EPERM "}, /**< Operation not permitted. */
{RT_EWAITPEND, "OK "}, /**< Wait pending, internal sentinel for scheduler. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • 这个显示成"ok"是不是不大好,显示成"WAITPEND".这样准确的?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

那这个PR貌似意义就不大了hh,这个PR就是修复ps的时候显示错误状态的

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

应该是有一个比较好的状态信息显示。

* Note: assert to avoid an unexpected resume
* Note: clear any temporary error from interruptible wait
*/
RT_ASSERT(thread->error == RT_EOK);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里应该保留原来的逻辑

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Kernel PR has src relate code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants