fix: use sigkill for wayland killClient()#1631
Conversation
之前 wayland 下强行停止进程的实现和关闭窗口一样,可能会导致一些程序 无法被强行停止. 现在改用使用 pid 并发送 SIGKILL 的方式强行停止进程 PMS: BUG-364201 Log:
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR changes the Wayland task manager’s killClient() implementation to terminate clients by sending SIGKILL to their PID instead of requesting a normal close, improving reliability of forced termination, and adds logging when the PID is unavailable. Sequence diagram for updated killClient forced terminationsequenceDiagram
actor User
participant TreeLandWindow
participant ForeignToplevelHandle
participant OS
User->>TreeLandWindow: killClient()
TreeLandWindow->>ForeignToplevelHandle: pid()
ForeignToplevelHandle-->>TreeLandWindow: pid
alt [pid <= 0]
TreeLandWindow->>TreeLandWindow: waylandwindowLog()
TreeLandWindow->>TreeLandWindow: qCWarning(...)
else [pid > 0]
TreeLandWindow->>OS: kill(pid, SIGKILL)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码实现了终止进程的功能,我来分析一下其安全性、逻辑和潜在改进点:
void TreeLandWindow::killClient()
{
const auto pid = m_foreignToplevelHandle->pid();
if (pid <= 0) {
qCWarning(waylandwindowLog()) << "Cannot find pid of window"
<< m_foreignToplevelHandle->appid() << m_foreignToplevelHandle->id();
return;
}
// 检查是否为系统进程
if (pid < 1000) { // 通常系统进程ID小于1000
qCWarning(waylandwindowLog()) << "Attempt to kill system process with pid:" << pid;
return;
}
// 先尝试使用SIGTERM优雅终止
if (::kill(pid, SIGTERM) == 0) {
// 可以设置一个定时器,如果进程在超时后仍在运行,再使用SIGKILL
qCDebug(waylandwindowLog()) << "Sent SIGTERM to process" << pid;
} else {
qCWarning(waylandwindowLog()) << "Failed to send SIGTERM to process" << pid
<< "Error:" << strerror(errno);
return;
}
}
这些改进可以提高代码的安全性、可靠性和用户体验,同时更好地处理各种边界情况。 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider checking the return value of
::killand logging a warning (includingerrno) when it fails so that issues like permission problems or stale PIDs are easier to diagnose. - If there’s any chance
m_foreignToplevelHandlecan become invalid or null by the timekillClient()is called, it would be safer to guard against this before dereferencing it forpid(),appid()andid().
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider checking the return value of `::kill` and logging a warning (including `errno`) when it fails so that issues like permission problems or stale PIDs are easier to diagnose.
- If there’s any chance `m_foreignToplevelHandle` can become invalid or null by the time `killClient()` is called, it would be safer to guard against this before dereferencing it for `pid()`, `appid()` and `id()`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia, tsic404, tsix404 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
之前 wayland 下强行停止进程的实现和关闭窗口一样,可能会导致一些程序
无法被强行停止. 现在改用使用 pid 并发送 SIGKILL 的方式强行停止进程
PMS: BUG-364201
Summary by Sourcery
Bug Fixes: