Skip to content

fix: revert_settings返回false方便调用方重试 #585

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.

Sign up for GitHub

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

Open
ShadowLemoon wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: revert_settings返回false方便调用方重试 #585

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions src/platform/windows/display_device/settings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -981,28 +981,29 @@ namespace display_device {
// 尝试恢复设置
bool data_updated { false };
bool success = try_revert_settings(*persistent_data, data_updated, skip_vdd_destroy);
if (!success) {

if (success) {
// 恢复成功，清理持久化数据
remove_file(filepath);
persistent_data = nullptr;
BOOST_LOG(info) << "显示设备配置已恢复";
}
else {
if (data_updated) {
save_settings(filepath, *persistent_data); // 忽略返回值
save_settings(filepath, *persistent_data); // 保存部分恢复的状态以便重试
}
Comment on lines +985 to 994
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

持久化操作的失败不能被静默忽略。

这段新逻辑把“可重试”建立在 persistent_data 的最新快照之上，但成功分支里无论 remove_file(filepath) 是否真的删掉文件都会立刻清掉内存态，失败分支里 save_settings() 的返回值也被忽略。只要任一持久化操作失败，进程重启后就可能按过期 JSON 继续回滚，重复执行已经成功的步骤。建议把删除/保存失败显式纳入结果处理，而不是只保留一个普通的 success/false。

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/display_device/settings.cpp` around lines 985 - 994, The
current logic ignores failures from remove_file(filepath) and
save_settings(filepath, *persistent_data), which can cause loss of the correct
retry state; update the block around success/data_updated to check and handle
each persistence call's return value: call remove_file(filepath) and only set
persistent_data = nullptr and log success if remove_file returns true, otherwise
log an error and keep persistent_data so restart can retry; similarly, capture
the return of save_settings(...) when data_updated is true, log an error and
avoid discarding or overwriting the in-memory state on failure, and propagate a
failure status (e.g., return/throw or set an error flag) instead of only using
the top-level success boolean. Ensure you reference and update the existing
symbols remove_file, save_settings, persistent_data, filepath, success, and
data_updated so callers can respond to persistence failures.

BOOST_LOG(error) << "恢复显示设备设置失败！如有异常请尝试关闭基地显示器，或手动修改系统显示设置~";
}

// 清理持久化数据
remove_file(filepath);
persistent_data = nullptr;

// 释放音频数据
// 不管成败都释放音频数据（串流已结束）
if (reason != revert_reason_e::topology_switch) {
if (audio_data) {
BOOST_LOG(debug) << "释放捕获的音频接收器";
audio_data = nullptr;
}
}

if (success) {
BOOST_LOG(info) << "显示设备配置已恢复";
}
return success;
Comment on lines +998 to +1006
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 8, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

失败后仍释放 audio_data，会把后续重试需要的音频会话提前拆掉。

apply_config() 里持有 audio_data 的目的，是把音频会话延长到“显示设置已经恢复”为止。现在失败时会保留 persistent_data 供后续重试，但这里仍然在非 topology_switch 场景下释放 audio_data；如果失败点恰好依赖这段保活，下一次重试就失去了原本的兜底。建议至少只在 success == true 时再释放它。

可参考的最小改动 
-      // 不管成败都释放音频数据（串流已结束）
-      if (reason != revert_reason_e::topology_switch) {
+      // 仅在恢复完成后释放音频数据；失败时保留，便于调用方继续重试
+      if (success && reason != revert_reason_e::topology_switch) {
         if (audio_data) {
           BOOST_LOG(debug) << "释放捕获的音频接收器";
           audio_data = nullptr;
         }
       }

As per coding guidelines, src/**/*.{cpp,c,h}: Sunshine 核心 C++ 源码，自托管游戏串流服务器。审查要点：内存安全、 线程安全、RAII 资源管理、安全漏洞。

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/platform/windows/display_device/settings.cpp` around lines 998 - 1006,
The code currently releases audio_data regardless of operation success (inside
the block guarded by revert_reason_e::topology_switch), which breaks retries
that rely on apply_config() holding the session; change the release logic so
audio_data is only cleared when the operation succeeded (i.e., require success
== true in addition to reason != revert_reason_e::topology_switch) or move the
release into the success path before return; update the release check around
audio_data in settings.cpp (the block handling revert_reason_e and audio_data)
to depend on success so persistent_data retries retain the audio session.

}
return true;
}
Expand Down
Loading