fix: prep-cmd permission_denied 例外未覆盖 wait/exit_code 检查#678
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details🔇 Additional comments (1)
Summary by CodeRabbit
Walkthrough对预启动命令错误处理加入 skip_prep_failure 条件以在桌面模式遇到权限拒绝时跳过等待与退出码检查；并将 CI 工作流中 DRIVER_DEPS_REQUIRED 的禁用条件扩展到 workflow_dispatch 触发。 变更内容预启动命令与 CI 条件更改
🎯 3 (Moderate) | ⏱️ ~20 分钟 Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/process.cpp`:
- Around line 246-272: The loop currently advances _app_prep_it even when a prep
cmd was skipped due to skip_prep_failure, causing terminate() to treat that prep
as executed and run its undo_cmd; modify the logic so only actually-successful
prep commands are recorded for undo: move iterator advancement and any
push-to-undo collection (or mark) into the success path after child.exit_code()
confirms success (i.e., only when do_cmd ran and returned 0), and do not add or
mark the current prep_cmd when skip_prep_failure is true; update the code paths
around skip_prep_failure, _app_prep_it, and any undo stack used by terminate()
so undo_cmd runs only for commands that truly executed successfully.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 05d695a1-4cf6-4060-9671-41a0bcd6145c
📒 Files selected for processing (1)
src/process.cpp
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,c,h}
⚙️ CodeRabbit configuration file
src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。
Files:
src/process.cpp
| // Track if we hit the desktop-mode permission_denied exception, | ||
| // so we can also skip the wait/exit_code checks that would otherwise fail. | ||
| bool skip_prep_failure = false; | ||
| if (ec) { | ||
| BOOST_LOG(error) << "Couldn't run ["sv << cmd.do_cmd << "]: System: "sv << ec.message(); | ||
| // We don't want any prep commands failing launch of the desktop. | ||
| // This is to prevent the issue where users reboot their PC and need to log in with Sunshine. | ||
| // permission_denied is typically returned when the user impersonation fails, which can happen when user is not signed in yet. | ||
| if (!(_app.cmd.empty() && ec == std::errc::permission_denied)) { | ||
| if (_app.cmd.empty() && ec == std::errc::permission_denied) { | ||
| skip_prep_failure = true; | ||
| } else { | ||
| return -1; | ||
| } | ||
| } | ||
|
|
||
| child.wait(ec); | ||
| if (ec) { | ||
| BOOST_LOG(error) << '[' << cmd.do_cmd << "] wait failed with error code ["sv << ec << ']'; | ||
| return -1; | ||
| } | ||
| auto ret = child.exit_code(); | ||
| if (ret != 0) { | ||
| BOOST_LOG(error) << '[' << cmd.do_cmd << "] exited with code ["sv << ret << ']'; | ||
| return -1; | ||
| if (!skip_prep_failure) { | ||
| child.wait(ec); | ||
| if (ec) { | ||
| BOOST_LOG(error) << '[' << cmd.do_cmd << "] wait failed with error code ["sv << ec << ']'; | ||
| return -1; | ||
| } | ||
| auto ret = child.exit_code(); | ||
| if (ret != 0) { | ||
| BOOST_LOG(error) << '[' << cmd.do_cmd << "] exited with code ["sv << ret << ']'; | ||
| return -1; | ||
| } | ||
| } |
There was a problem hiding this comment.
跳过失败后会错误执行对应 undo_cmd，可能触发反向副作用。
在 Line 255 命中“桌面模式 + permission_denied”后，代码会继续并让循环推进 _app_prep_it。这会让 terminate() 把该 prep_cmd 视为已执行，从而运行它的 undo_cmd，即使 do_cmd 实际未成功执行。建议把“遍历游标”与“已成功执行边界”分离，只将成功执行的命令纳入 undo 范围。
修复方向（示意）
- for (; _app_prep_it != std::end(_app.prep_cmds); ++_app_prep_it) {
- auto &cmd = *_app_prep_it;
+ auto scan_it = std::begin(_app.prep_cmds);
+ _app_prep_it = _app_prep_begin; // executed_end
+ for (; scan_it != std::end(_app.prep_cmds); ++scan_it) {
+ auto &cmd = *scan_it;
...
+ bool executed_ok = false;
if (ec) {
...
if (_app.cmd.empty() && ec == std::errc::permission_denied) {
skip_prep_failure = true;
} else {
return -1;
}
}
if (!skip_prep_failure) {
child.wait(ec);
...
if (ret != 0) {
return -1;
}
+ executed_ok = true;
}
+
+ if (executed_ok) {
+ _app_prep_it = scan_it + 1; // 仅成功 do_cmd 才推进 undo 边界
+ }
}As per coding guidelines, “src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。注意预处理宏控制的平台相关代码。”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/process.cpp` around lines 246 - 272, The loop currently advances
_app_prep_it even when a prep cmd was skipped due to skip_prep_failure, causing
terminate() to treat that prep as executed and run its undo_cmd; modify the
logic so only actually-successful prep commands are recorded for undo: move
iterator advancement and any push-to-undo collection (or mark) into the success
path after child.exit_code() confirms success (i.e., only when do_cmd ran and
returned 0), and do not add or mark the current prep_cmd when skip_prep_failure
is true; update the code paths around skip_prep_failure, _app_prep_it, and any
undo stack used by terminate() so undo_cmd runs only for commands that truly
executed successfully.
|
请问你有测试过吗？ |
|
我记得锁屏中本来就不能修改dpi这些屏幕设置，会被系统强制拦截。 |
|
感谢回复。关于这两点：
如果这个PR的方向不对，可以关闭，等后续的细粒度开关方案一起处理。 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/main.yml (1)
197-201:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win注释未同步更新以说明 workflow_dispatch 的逻辑。
第 197-200 行的注释仅解释了 fork PR 无法访问私有仓库驱动下载所需的 secrets，故需将缺失驱动从致命错误降级为警告。但第 201 行新增的逻辑同时将
workflow_dispatch手动触发的构建也设置为OFF，而注释未提及此场景。建议补充注释说明为何
workflow_dispatch同样需要DRIVER_DEPS_REQUIRED=OFF（例如：是否出于测试便利性考虑、或手动触发同样可能缺少某些驱动）。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/main.yml around lines 197 - 201, 注释未说明为什么条件还将 workflow_dispatch 设置为 OFF：更新注释关联到 DRIVER_DEPS_REQUIRED 的逻辑，明确说明该变量在 pull_request 来自 fork 或手动触发 (workflow_dispatch) 时会被置为 OFF，并解释为何对 workflow_dispatch 也采用相同降级策略（例如手动触发可能缺少 org-scoped secrets、便于本地/手动测试或避免在人工触发时失败）；在注释中引用 github.event_name == 'workflow_dispatch' 和 github.event.pull_request.head.repo.fork 这两个条件以便读者能直接对应到 DRIVER_DEPS_REQUIRED 的判定逻辑。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/main.yml:
- Line 201: The current DRIVER_DEPS_REQUIRED expression in main.yml treats
workflow_dispatch the same as forked PRs and can silently allow missing drivers;
change the condition so DRIVER_DEPS_REQUIRED is set to 'OFF' only for forked
pull requests (github.event_name == 'pull_request' &&
github.event.pull_request.head.repo.fork) and keep workflow_dispatch defaulting
to 'ON'; update the main.yml comment near DRIVER_DEPS_REQUIRED to explain that
only forked PRs cannot access secrets.DRIVER_DOWNLOAD_TOKEN and therefore must
tolerate missing drivers (or alternatively add a workflow_dispatch input to
explicitly allow non-fatal missing drivers if you prefer that behavior).
---
Outside diff comments:
In @.github/workflows/main.yml:
- Around line 197-201: 注释未说明为什么条件还将 workflow_dispatch 设置为 OFF：更新注释关联到
DRIVER_DEPS_REQUIRED 的逻辑，明确说明该变量在 pull_request 来自 fork 或手动触发 (workflow_dispatch)
时会被置为 OFF，并解释为何对 workflow_dispatch 也采用相同降级策略（例如手动触发可能缺少 org-scoped
secrets、便于本地/手动测试或避免在人工触发时失败）；在注释中引用 github.event_name == 'workflow_dispatch' 和
github.event.pull_request.head.repo.fork 这两个条件以便读者能直接对应到 DRIVER_DEPS_REQUIRED
的判定逻辑。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d76bbc2b-75c0-465c-8829-2073cd2717b1
📒 Files selected for processing (1)
.github/workflows/main.yml
不要用ai回答我。 |
|
关于你的这个场景，你的诉求场景是：串流后如果是处于锁屏状态，解锁屏幕再执行对应prep-cmd |
|
我邮箱部署的agent自动回复了
|
|
我本地自用5/5无问题，如果该方案有用可以参考
|
Per maintainer feedback, workflow changes are out of scope for this PR. Only the process.cpp fix should be included.
|
[2026-05-25 18:29:23.088]: Info: Executing Do Cmd: [.\tools\SetDpi.exe 175]
[2026-05-25 18:29:23.101]: Info: .\tools\SetDpi.exe 175 running with PID 26392
[2026-05-25 18:29:23.447]: Info: Executing [Desktop]
[2026-05-25 18:30:12.121]: Info: CLIENT CONNECTED
[2026-05-25 18:30:18.611]: Info: [SessionListener] 检测到会话解锁事件
|
2 participants
问题
在桌面模式（
_app.cmd为空）下，当 Windows 会话锁定时，Session 0 中的进程创建会返回permission_denied。execute()函数已有例外处理：当run_command返回permission_denied且为桌面模式时，跳过错误继续执行。但后续的child.wait()同样返回permission_denied，没有被例外覆盖，直接return -1，触发fail_guard调用terminate()销毁 VDD，串流中断。日志证据
第一层
run_command失败被例外跳过，但第二层child.wait()失败未被跳过。修复
添加
skip_prep_failure标志，当命中桌面模式 +permission_denied例外时，同时跳过wait()和exit_code()检查，prep-cmd 被记录为失败但不阻止串流继续。效果