Skip to content

Conversation

@Vitus213
Copy link
Contributor

修复 gvisor ppoll 测试套件中的多个失败用例:

  1. timespec 验证

    • 添加 tv_sec < 0 检查,返回 EINVAL
    • 添加 tv_nsec 范围检查 [0, 999999999],越界返回 EINVAL - 修复 InvalidTimeoutNegative 和 InvalidTimeoutNotNormalized 测试
  2. sigsetsize 参数 - 添加第 5 个参数 sigsetsize(原只有 4 个参数) - 验证 sigsetsize 必须等于 sizeof(SigSet) - 修复 InvalidMaskSize 测试

  3. 信号掩码处理

    • 使用 has_pending_not_masked_signal() 替代 signal_pending_state()
    • 确保被屏蔽的信号不会中断 ppoll,只处理未屏蔽的信号 - 修复 SignalMaskBlocksSignal 和 SignalMaskAllowsSignal 测试
  4. 错误码转换 - 在 poll_select_finish 中添加 ERESTART_RESTARTBLOCK → EINTR 转换 - 移除 ppoll 中不必要的 restart block 设置

  修复 gvisor ppoll 测试套件中的多个失败用例:

  1. timespec 验证
     - 添加 tv_sec < 0 检查,返回 EINVAL
     - 添加 tv_nsec 范围检查 [0, 999999999],越界返回 EINVAL
     - 修复 InvalidTimeoutNegative 和 InvalidTimeoutNotNormalized 测试

  2. sigsetsize 参数
     - 添加第 5 个参数 sigsetsize(原只有 4 个参数)
     - 验证 sigsetsize 必须等于 sizeof(SigSet)
     - 修复 InvalidMaskSize 测试

  3. 信号掩码处理
     - 使用 has_pending_not_masked_signal() 替代 signal_pending_state()
     - 确保被屏蔽的信号不会中断 ppoll,只处理未屏蔽的信号
     - 修复 SignalMaskBlocksSignal 和 SignalMaskAllowsSignal 测试

  4. 错误码转换
     - 在 poll_select_finish 中添加 ERESTART_RESTARTBLOCK → EINTR 转换
     - 移除 ppoll 中不必要的 restart block 设置
@github-actions github-actions bot added ambiguous The title of PR/issue doesn't match the format Bug fix A bug is fixed in this pull request labels Jan 15, 2026
@Vitus213
Copy link
Contributor Author

代码审查

发现 1 个问题:

  1. 缺少快速路径检查 has_pending_signal_fast() 导致性能回退

PR 将原来的两层检查模式:

if current_pcb.has_pending_signal_fast()
    && Signal::signal_pending_state(true, false, &current_pcb)

改为只调用:

if current_pcb.has_pending_not_masked_signal() {

has_pending_not_masked_signal() 内部需要获取锁、读取信号掩码、执行集合操作等,这是一个相对昂贵的操作。而 has_pending_signal_fast() 只是一个 O(1) 的标志位检查。

代码库中其他类似位置都使用两层检查模式以避免不必要的开销:

建议恢复两层检查:

🤖 Generated with Claude Code

- 如果此代码审查有用,请回复 👍。否则请回复 👎。

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

Labels

ambiguous The title of PR/issue doesn't match the format Bug fix A bug is fixed in this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant