fix: Listen for SIGTERM and exit the application gracefully.#444
fix: Listen for SIGTERM and exit the application gracefully.#444deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR updates deepin-camera’s signal handling so that SIGTERM triggers a graceful Qt application shutdown via QApplication::quit(), and ensures the signal handler is registered only after the QApplication instance is created to avoid crashes related to improper resource cleanup. Sequence diagram for SIGTERM-triggered graceful Qt shutdownsequenceDiagram
actor User
participant OS
participant Process as deepin_camera_process
participant SignalHandler as handleSignal
participant QApplication as QApplication_instance
User->>OS: kill deepin_camera_process (SIGTERM)
OS-->>Process: deliver SIGTERM
Process-->>SignalHandler: invoke handleSignal(SIGTERM)
SignalHandler->>SignalHandler: qWarning Received signal: SIGTERM
SignalHandler->>QApplication: quit()
QApplication-->>Process: exit event loop
Process->>Process: Qt performs normal resource cleanup
Process-->>OS: process exits gracefully
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- Calling
qWarning()and especiallyQApplication::quit()directly from a POSIX signal handler is not async-signal-safe and may lead to undefined behavior; instead, consider making the handler set anstd::atomic<bool>/flag and use a Qt timer orQMetaObject::invokeMethodin the main thread to perform the graceful quit. - If you need more robust signal handling (e.g., avoiding interruption issues), consider switching from
signal()tosigaction()and centralizing the SIGTERM handling logic so it can be extended safely for additional signals in the future.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Calling `qWarning()` and especially `QApplication::quit()` directly from a POSIX signal handler is not async-signal-safe and may lead to undefined behavior; instead, consider making the handler set an `std::atomic<bool>`/flag and use a Qt timer or `QMetaObject::invokeMethod` in the main thread to perform the graceful quit.
- If you need more robust signal handling (e.g., avoiding interruption issues), consider switching from `signal()` to `sigaction()` and centralizing the SIGTERM handling logic so it can be extended safely for additional signals in the future.
## Individual Comments
### Comment 1
<location> `src/main.cpp:129` </location>
<code_context>
qWarning() << "Received signal:" << sig;
- exit(0);
+ // 在HW机器、SW机器上,发现使用exit(0)退出会导致Qt某些函数空指针异常,从而导致崩溃,疑似在资源回收环节出了问题。所以我们使用QApplication::quit()更优雅地退出。
+ QApplication::quit();
+ //exit(0);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Calling QApplication::quit() from a POSIX signal handler is not async-signal-safe and can lead to undefined behavior.
POSIX signal handlers may only call async-signal-safe functions. Neither `QApplication::quit()` nor `qWarning()` are guaranteed safe here and can allocate, lock, or access Qt state in a non‑reentrant way, leading to deadlocks or crashes. Prefer keeping the handler minimal (e.g., set an `std::sig_atomic_t` flag or write to a pipe) and have the Qt event loop detect that (via `QSocketNotifier`, a timer, etc.) and call `QApplication::quit()` from the main thread instead.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
DDE may kill the deepin-camera circumstances, the logging system would consider it. We use QApplication::quit() to exit more gracefully. 监听 SIGTERM 信号,正常退出应用。 由于DDE在某些情况下会kill掉deepin-camera进程,同时埋点会认为deepin-camera出错,所以这里监听信号,正常退出。 在HW机器、SW机器上,发现使用exit(0)退出会导致Qt某些函数空指针异常,从而导致崩溃,疑似在资源回收环节出了问题。所以我们使用QApplication::quit()更优雅地退出。 Bug: https://pms.uniontech.com/bug-view-344677.html v20 BUG 分支合一到v25主线 Task: https://pms.uniontech.com/task-view-383481.html
deepin pr auto review这段代码的修改主要涉及信号处理函数 1. 代码逻辑与语法审查
2. 代码质量与规范
3. 代码性能
4. 代码安全
5. 改进建议虽然当前的修改已经解决了主要问题,但为了进一步提高代码的健壮性,建议考虑以下几点:
总结这段代码修改是高质量的,有效地解决了 优化后的代码示例: static void handleSignal(int sig)
{
// 注意:在信号处理函数中应尽量避免调用非 async-signal-safe 的函数。
// 此处保留 qWarning 用于调试,但在生产环境中需评估风险。
qWarning() << "Received signal:" << sig;
// 使用 QTimer::singleShot 安全地退出,确保在主事件循环中执行清理
QTimer::singleShot(0, qApp, []() {
// 增加防御性检查,确保 qApp 依然有效
if (qApp) {
QApplication::quit();
}
});
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, lzwind 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 |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
DDE may kill the deepin-camera circumstances, the logging system would consider it. We use QApplication::quit() to exit more gracefully.
监听 SIGTERM 信号,正常退出应用。
由于DDE在某些情况下会kill掉deepin-camera进程,同时埋点会认为deepin-camera出错,所以这里监听信号,正常退出。
在HW机器、SW机器上,发现使用exit(0)退出会导致Qt某些函数空指针异常,从而导致崩溃,疑似在资源回收环节出了问题。所以我们使用QApplication::quit()更优雅地退出。
Bug: https://pms.uniontech.com/bug-view-344677.html
v20 BUG 分支合一到v25主线
Task: https://pms.uniontech.com/task-view-383481.html
Summary by Sourcery
Bug Fixes: