Skip to content

fix: change killClient to use SIGKILL instead of xcb_kill_client_checked#1463

Merged
robertkill merged 1 commit intolinuxdeepin:masterfrom
robertkill:master
Mar 3, 2026
Merged

fix: change killClient to use SIGKILL instead of xcb_kill_client_checked#1463
robertkill merged 1 commit intolinuxdeepin:masterfrom
robertkill:master

Conversation

@robertkill
Copy link
Contributor

@robertkill robertkill commented Mar 2, 2026

  1. Modified killClient function to directly terminate the process using
    SIGKILL signal
  2. Removed X11 client message event construction for WM_DELETE_WINDOW
    protocol
  3. Added window PID lookup and validation before sending kill signal
  4. Added include for csignal header for kill() function support
  5. Added error logging when unable to retrieve window PID

The previous implementation used the X11 WM_DELETE_WINDOW protocol which
relies on applications properly handling graceful shutdown requests.
This approach was unreliable as some applications ignore or don't
implement this protocol. The new implementation directly terminates the
process using SIGKILL, ensuring the application is forcefully closed
regardless of its cooperation with X11 window management protocols. This
provides more reliable window closing functionality in the task manager.

Log: Changed window closing behavior to force terminate unresponsive
applications

Influence:

  1. Test closing various applications from task manager to ensure they
    terminate
  2. Verify that unresponsive applications can be successfully closed
  3. Test with applications that previously ignored WM_DELETE_WINDOW
    requests
  4. Check system stability after force-closing multiple applications
  5. Verify no orphaned processes remain after killClient is called
  6. Test with applications that have multiple windows to ensure proper
    process termination

fix: 修改 killClient 使用 SIGKILL 替代 WM_DELETE_WINDOW

  1. 修改 killClient 函数,直接使用 SIGKILL 信号终止进程
  2. 移除了用于 WM_DELETE_WINDOW 协议的 X11 客户端消息事件构造
  3. 添加了窗口 PID 查找和验证,在发送终止信号前进行检查
  4. 添加了 csignal 头文件包含以支持 kill() 函数
  5. 添加了无法获取窗口 PID 时的错误日志记录

之前的实现使用了 X11 WM_DELETE_WINDOW 协议,这依赖于应用程序正确处理优雅
关闭请求。这种方法不可靠,因为有些应用程序会忽略或不实现此协议。新的实现
直接使用 SIGKILL 终止进程,确保无论应用程序是否配合 X11 窗口管理协议,都
能被强制关闭。这为任务管理器提供了更可靠的窗口关闭功能。

Log: 修改窗口关闭行为,强制终止无响应的应用程序

Influence:

  1. 测试从任务管理器关闭各种应用程序,确保它们能正常终止
  2. 验证无响应的应用程序可以被成功关闭
  3. 测试之前忽略 WM_DELETE_WINDOW 请求的应用程序
  4. 检查强制关闭多个应用程序后的系统稳定性
  5. 验证调用 killClient 后没有残留的孤儿进程
  6. 测试具有多个窗口的应用程序,确保进程被正确终止

PMS: BUG-342617

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Sorry @robertkill, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@robertkill robertkill force-pushed the master branch 2 times, most recently from 10f7c23 to 7251a99 Compare March 2, 2026 09:12
@robertkill robertkill changed the title fix: replace xcb_kill_client with WM_DELETE_WINDOW protocol fix: change killClient to use SIGKILL instead of xcb_kill_client_checked Mar 2, 2026
@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, BLumia, robertkill

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

1. Modified killClient function to directly terminate the process using
SIGKILL signal
2. Removed X11 client message event construction for WM_DELETE_WINDOW
protocol
3. Added window PID lookup and validation before sending kill signal
4. Added include for csignal header for kill() function support
5. Added error logging when unable to retrieve window PID

The previous implementation used the X11 WM_DELETE_WINDOW protocol which
relies on applications properly handling graceful shutdown requests.
This approach was unreliable as some applications ignore or don't
implement this protocol. The new implementation directly terminates the
process using SIGKILL, ensuring the application is forcefully closed
regardless of its cooperation with X11 window management protocols. This
provides more reliable window closing functionality in the task manager.

Log: Changed window closing behavior to force terminate unresponsive
applications

Influence:
1. Test closing various applications from task manager to ensure they
terminate
2. Verify that unresponsive applications can be successfully closed
3. Test with applications that previously ignored WM_DELETE_WINDOW
requests
4. Check system stability after force-closing multiple applications
5. Verify no orphaned processes remain after killClient is called
6. Test with applications that have multiple windows to ensure proper
process termination

fix: 修改 killClient 使用 SIGKILL 替代 WM_DELETE_WINDOW

1. 修改 killClient 函数,直接使用 SIGKILL 信号终止进程
2. 移除了用于 WM_DELETE_WINDOW 协议的 X11 客户端消息事件构造
3. 添加了窗口 PID 查找和验证,在发送终止信号前进行检查
4. 添加了 csignal 头文件包含以支持 kill() 函数
5. 添加了无法获取窗口 PID 时的错误日志记录

之前的实现使用了 X11 WM_DELETE_WINDOW 协议,这依赖于应用程序正确处理优雅
关闭请求。这种方法不可靠,因为有些应用程序会忽略或不实现此协议。新的实现
直接使用 SIGKILL 终止进程,确保无论应用程序是否配合 X11 窗口管理协议,都
能被强制关闭。这为任务管理器提供了更可靠的窗口关闭功能。

Log: 修改窗口关闭行为,强制终止无响应的应用程序

Influence:
1. 测试从任务管理器关闭各种应用程序,确保它们能正常终止
2. 验证无响应的应用程序可以被成功关闭
3. 测试之前忽略 WM_DELETE_WINDOW 请求的应用程序
4. 检查强制关闭多个应用程序后的系统稳定性
5. 验证调用 killClient 后没有残留的孤儿进程
6. 测试具有多个窗口的应用程序,确保进程被正确终止

PMS: BUG-342617
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改了 X11Utils::killClient 函数的实现方式,从使用 XCB 协议的 xcb_kill_client_checked 改为通过 PID 直接调用 ::kill 发送 SIGKILL 信号。

以下是对这段代码的详细审查和改进意见:

1. 语法与逻辑

  • 正确性:代码语法是正确的。增加了对无效窗口 ID (XCB_WINDOW_NONE) 的检查,逻辑上是合理的。
  • 逻辑变更
    • 原逻辑xcb_kill_client 是向 X11 服务器发送请求,让 X 服务器断开与该窗口所属客户端的连接。这通常会导致应用程序收到 X 服务器断开连接的信号并退出,但也可能被应用程序捕获或忽略(取决于应用如何处理 I/O 错误)。
    • 新逻辑::kill(pid, SIGKILL) 是直接向操作系统内核发送信号,无条件强制终止进程。SIGKILL 无法被进程捕获或忽略。
    • 潜在问题:这种逻辑变更非常激进。如果该窗口属于一个包含多个窗口的大型应用程序(如 IDE、浏览器),直接 SIGKILL 可能会导致整个应用程序瞬间崩溃,用户可能来不及保存数据。原 XCB 方式通常更为温和(尽管也不保证保存数据,但给程序一点反应机会)。

2. 代码质量

  • 注释// 直接强杀进程 这个注释清楚地说明了意图,但建议补充说明为什么要这样改(例如:为了确保无响应窗口能被彻底关闭,或者 XCB 方式在某些情况下失效)。
  • 日志:使用了 qCWarning 记录无法获取 PID 的情况,这是很好的做法,有助于调试。
  • 依赖:引入了 <csignal> 是必要的,因为使用了 SIGKILL

3. 代码性能

  • 性能:性能影响微乎其微。
    • 原版:一次 XCB 请求(IPC)。
    • 新版:一次查询 PID 的 XCB 请求(getWindowPid) + 一次系统调用(kill)。
    • getWindowPid 可能会涉及 XCB 属性读取,比单纯的 kill 系统调用慢,但在任务管理器这种交互场景下,用户感知不到差异。

4. 代码安全

  • 权限问题:代码没有检查调用者是否有权限向目标 pid 发送信号。如果 getWindowPid 返回的 PID 属于其他用户(或 root),且当前进程没有相应权限,::kill 会失败,但代码忽略了 ::kill 的返回值。
  • 竞态条件 (TOCTOU):在 getWindowPid(winid)::kill(pid, ...) 之间存在一个时间窗口。如果在这个极短的瞬间,窗口被销毁且 PID 被操作系统回收并分配给了另一个新进程,那么 kill 可能会误杀一个不相关的进程。虽然概率极低,但在安全敏感的场景下需要注意。
  • PID 重用:如果窗口已经销毁但 X11Utils 内部仍有引用,获取到的 PID 可能已经无效或被重用。

5. 改进建议

为了增强代码的健壮性和安全性,建议进行以下修改:

  1. 检查 kill 返回值:处理信号发送失败的情况,并记录日志。
  2. 增加 PID 有效性的简单校验:虽然不能完全避免 TOCTOU,但可以通过 kill(pid, 0) 来检查进程是否存在以及是否有权限发送信号(注意:kill(pid, 0) 不发送信号,仅用于检查)。
  3. 回退机制:考虑到 getWindowPid 可能失败(例如某些窗口没有 _NET_WM_PID 属性),建议在无法获取 PID 或 kill 失败时,回退到原来的 xcb_kill_client 方式。这样能保证兼容性。

改进后的代码示例

#include <cerrno>    // 用于 errno
#include <cstring>  // 用于 strerror

void X11Utils::killClient(const xcb_window_t &winid)
{
    if (winid == XCB_WINDOW_NONE)
        return;

    pid_t pid = getWindowPid(winid);
    
    // 尝试通过 PID 强杀
    if (pid > 0) {
        // 先检查进程是否存在以及是否有权限 (发送 0 信号)
        if (::kill(pid, 0) == 0) {
            // 进程存在且有权限,发送 SIGKILL
            if (::kill(pid, SIGKILL) == 0) {
                return; // 成功
            } else {
                // kill 失败,记录错误
                qCWarning(x11UtilsLog()) << "Failed to kill process" << pid << ":" << strerror(errno);
                // 继续尝试下面的回退方案
            }
        } else {
            // kill(pid, 0) 失败,可能是进程不存在或无权限
            qCWarning(x11UtilsLog()) << "Process check failed for pid" << pid << ":" << strerror(errno);
            // 继续尝试下面的回退方案
        }
    } else {
        qCWarning(x11UtilsLog()) << "Cannot get pid for window:" << winid;
    }

    // 回退方案:如果通过 PID 失败(例如无法获取 PID 或 kill 失败),
    // 尝试使用 XCB 原生方式关闭连接,作为最后的手段。
    qCInfo(x11UtilsLog()) << "Fallback to xcb_kill_client for window:" << winid;
    xcb_kill_client(getXcbConnection(), winid);
}

总结

原代码修改实现了强制关闭窗口的功能,但过于激进且缺乏错误处理。改进后的代码增加了错误检查、日志记录以及回退机制,提高了代码的健壮性和可维护性,同时保留了原有的功能兼容性。

@robertkill robertkill merged commit 56460df into linuxdeepin:master Mar 3, 2026
11 of 12 checks passed
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.

4 participants