Skip to content

fix: Implement process timeout handling in SpawnCmd#572

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
re2zero:bugfix
Sep 25, 2025
Merged

fix: Implement process timeout handling in SpawnCmd#572
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
re2zero:bugfix

Conversation

@re2zero
Copy link
Contributor

@re2zero re2zero commented Sep 25, 2025

  • Updated SpawnCmd to wait for process completion with a default timeout. This change improves the robustness of command execution by preventing indefinite hangs.

Log: Fix the database may be incompletely created, resulting in no searches.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: re2zero

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

- Updated SpawnCmd to wait for process completion with a default timeout.
This change improves the robustness of command execution by preventing indefinite hangs.

Log: Fix the database may be incompletely created, resulting in no searches.
Bug: https://pms.uniontech.com/bug-view-335337.html
@github-actions
Copy link

TAG Bot

TAG: 6.5.40
EXISTED: no
DISTRIBUTION: unstable

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 25, 2025

This pr cannot be merged! (status: unstable)

@re2zero
Copy link
Contributor Author

re2zero commented Sep 25, 2025

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Sep 25, 2025

This pr force merged! (status: unstable)

@deepin-ci-robot
Copy link

deepin pr auto review

根据提供的git diff,我将对代码进行审查,并提出改进意见:

  1. 版本更新部分:
  • 版本号从6.5.39.1更新到6.5.40.1,符合语义化版本规范
  • changelog中添加了适当的更新说明,描述了修复的问题
  • 建议在changelog中添加更详细的修复说明,包括具体的问题表现和修复方案
  1. 代码变更部分(src/base/command.cpp):
// 原代码
process.waitForFinished(-1);

// 新代码
bool finished = process.waitForFinished();
if (!finished) {
    qCWarning(app) << "Command timed out, terminating process";
    process.kill();
    process.waitForFinished(500);
    return false;
}

改进建议:

a) 性能方面:

  • 原代码使用-1表示无限等待,可能导致进程长时间挂起
  • 新代码添加了超时机制,这是很好的改进
  • 建议将超时时间定义为可配置的常量,而不是硬编码,例如:
const int DEFAULT_TIMEOUT = 30000; // 30秒
bool finished = process.waitForFinished(DEFAULT_TIMEOUT);

b) 错误处理:

  • 新代码添加了超时处理,提高了程序的健壮性
  • 建议在超时处理后添加更多的错误信息,比如记录超时的命令和参数
  • 考虑添加重试机制,对于关键命令可以重试几次

c) 代码质量:

  • 建议将超时处理逻辑封装为一个单独的函数,提高代码复用性
  • 可以考虑添加进程状态检查,确保进程确实在运行状态时才进行超时处理

d) 安全性:

  • 在执行外部命令时,建议添加命令白名单检查,防止命令注入
  • 对于敏感操作,建议添加额外的权限检查
  1. 其他建议:
  • 建议在changelog中添加测试验证信息,说明如何验证修复的有效性
  • 考虑添加单元测试,特别是对命令执行超时场景的测试

总体来说,这次修改提高了程序的健壮性,特别是添加了命令执行超时处理,这是一个很好的改进。建议按照上述建议进一步完善代码,提高其可维护性和安全性。

@lzwind
Copy link
Contributor

lzwind commented Sep 26, 2025

/retest

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants