Skip to content

Conversation

@fslongjin
Copy link
Member

  • 修正父进程唤醒条件,确保无论exit_signal为何值都唤醒父进程
  • 修复线程组leader唤醒逻辑的位置错误

解决wait_test里面Waiters/WaitSpecificChildTest.WALL/0概率性的超时1min都没返回的问题

- 修正父进程唤醒条件,确保无论exit_signal为何值都唤醒父进程
- 修复线程组leader唤醒逻辑的位置错误

Signed-off-by: longjin <longjin@DragonOS.org>
@github-actions github-actions bot added Bug fix A bug is fixed in this pull request test Unitest/User space test labels Dec 20, 2025
@fslongjin
Copy link
Member Author

@claude review this PR in Chinese

@dragonos-community-ai-pr-reviewer
Copy link

dragonos-community-ai-pr-reviewer bot commented Dec 20, 2025

Claude finished @fslongjin's task in 3m 5s —— View job


代码审查:fix(process): 修复子进程退出时父进程唤醒逻辑

我已审查了PR #1516的修改,以下是我的分析:

修改概述

本次修改修正了kernel/src/process/mod.rsexit_notify()函数的唤醒逻辑:

  1. 将父进程wait_queue的唤醒移出exit_signal != Signal::INVALID条件块:确保无论exit_signal为何值都唤醒父进程
  2. 修复线程组leader唤醒逻辑的位置:将线程组leader的唤醒逻辑也移到条件块外部,与父进程唤醒保持一致层级
  3. 添加详细注释:解释了Linux语义中exit_signal只决定发送什么信号,不决定是否唤醒父进程

技术分析

原始问题

  • exit_signal == Signal::INVALID时(如线程退出),父进程的wait_queue不会被唤醒
  • 线程组leader的唤醒逻辑在条件块内部,位置错误
  • 这导致使用__WALL选项等待子进程时可能出现概率性超时(Waiters/WaitSpecificChildTest.WALL/0测试失败)

Linux语义验证

  1. exit_signal == Signal::INVALID表示这是一个线程(非线程组leader),根据Linux语义,线程退出时不发送exit_signal
  2. 然而,父进程的wait队列必须被唤醒,因为:
    • 父进程可能使用__WALL选项等待任何类型的子进程(包括线程)
    • exit_signal只控制信号发送,不控制唤醒机制
    • 唤醒是wait系统调用语义的一部分,与信号机制独立

修复正确性
语义正确:符合Linux wait/waitpid系统调用行为
逻辑修复:解决了条件判断错误导致的唤醒遗漏
位置修复:线程组leader唤醒逻辑现在在正确位置
注释清晰:新增注释准确解释了修改原因

潜在考虑

  1. 性能影响:无显著性能影响,只是移除了不必要的条件判断
  2. 兼容性:与现有代码完全兼容,不会破坏现有功能
  3. 线程安全性:唤醒操作本身是线程安全的,使用适当的锁保护

测试建议

  1. 回归测试:确保所有wait/waitpid相关测试通过
  2. 边界测试
    • exit_signal == Signal::INVALID的场景(线程退出)
    • exit_signal == 0的场景(clone调用指定不发送信号)
    • 使用__WALL选项的wait调用
  3. 并发测试:多个线程同时等待同一子进程退出

改进建议(可选)

  1. 考虑添加断言:在唤醒前可添加调试断言验证parent_pcb不为空
  2. 错误处理:当前代码忽略唤醒失败,可考虑添加日志记录(但非必需,符合现有代码风格)

总结

这是一个正确且必要的修复,解决了Linux wait语义实现中的缺陷。修改简洁明了,注释充分,符合项目编码规范。建议合并此PR。

修改详情
@@ -409,24 +409,27 @@ impl ProcessManager {
                         e
                     );
                 }
-                // 额外唤醒:父进程可能阻塞在 wait 系列调用上
-                parent_pcb
-                    .wait_queue
-                    .wakeup_all(Some(ProcessState::Blocked(true)));
-
-                // 根据 Linux wait 语义,线程组中的任何线程都可以等待同一线程组中任何线程创建的子进程。
-                // 由于子进程被添加到线程组 leader 的 children 列表中,
-                // 因此还需要唤醒线程组 leader 的 wait_queue(如果 leader 不是 parent_pcb 本身)。
-                let parent_group_leader = {
-                    let ti = parent_pcb.thread.read_irqsave();
-                    ti.group_leader()
-                };
-                if let Some(leader) = parent_group_leader {
-                    if !Arc::ptr_eq(&leader, &parent_pcb) {
-                        leader
-                            .wait_queue
-                            .wakeup_all(Some(ProcessState::Blocked(true)));
-                    }
+            }
+
+            // 无论exit_signal是什么值,都要唤醒父进程的wait_queue
+            // 因为父进程可能使用__WALL选项等待任何类型的子进程(包括exit_signal=0的clone子进程)
+            // 根据Linux语义,exit_signal只决定发送什么信号,不决定是否唤醒父进程
+            parent_pcb
+                .wait_queue
+                .wakeup_all(Some(ProcessState::Blocked(true)));
+
+            // 根据 Linux wait 语义,线程组中的任何线程都可以等待同一线程组中任何线程创建的子进程。
+            // 由于子进程被添加到线程组 leader 的 children 列表中,
+            // 因此还需要唤醒线程组 leader 的 wait_queue(如果 leader 不是 parent_pcb 本身)。
+            let parent_group_leader = {
+                let ti = parent_pcb.thread.read_irqsave();
+                ti.group_leader()
+            };
+            if let Some(leader) = parent_group_leader {
+                if !Arc::ptr_eq(&leader, &parent_pcb) {
+                    leader
+                        .wait_queue
+                        .wakeup_all(Some(ProcessState::Blocked(true)));
                 }
             }
             // todo: 这里还需要根据线程组的信息,决定信号的发送

审查状态: ✅ 通过
建议操作: 合并此PR

<进度更新>

  • 分析PR上下文和更改的文件
  • 查看修改的文件内容,理解修复的逻辑
  • 检查相关测试代码了解问题
  • 提供代码审查反馈
    </进度更新>

@fslongjin fslongjin merged commit ad30303 into DragonOS-Community:master Dec 20, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug fix A bug is fixed in this pull request test Unitest/User space test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant