fix: Listen for SIGTERM and exit the application gracefully.#442
Conversation
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideHandle SIGTERM more gracefully by quitting the Qt application via QApplication::quit() after the Q(C)Application instance is constructed, instead of calling exit(0) directly from the signal handler before application initialization. Sequence diagram for graceful SIGTERM handling with QApplication_quitsequenceDiagram
participant OS
participant Process as deepin_camera_process
participant QtApp as QApplication
participant Handler as handleSignal
OS->>Process: SIGTERM
activate Process
Process->>Handler: handleSignal(SIGTERM)
activate Handler
Handler->>QtApp: QApplication::quit()
deactivate Handler
QtApp-->>Process: Event loop exit
Process-->>OS: Normal process termination
Flow diagram for main initialization and SIGTERM handler registrationflowchart TD
A[Start main] --> B[Configure logging via DLogManager]
B --> C[Other initialization steps]
C --> D[Create CApplication a]
D --> E[Register SIGTERM handler with signal]
E --> F[Enter Qt event loop]
F --> G[handleSignal called on SIGTERM]
G --> H[QApplication::quit]
H --> I[Event loop exits]
I --> J[Graceful shutdown and resource cleanup]
J --> K[End process]
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
QApplication::quit()directly from a POSIX signal handler is not async-signal-safe and can cause undefined behavior; consider restricting the handler to setting an atomic flag or writing to a pipe and then triggeringquit()from the Qt event loop instead. - If you need custom signal handling semantics (e.g., avoiding automatic reset of handlers), consider switching from
signal()tosigaction()to make the SIGTERM handling more robust and explicit.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Calling `QApplication::quit()` directly from a POSIX signal handler is not async-signal-safe and can cause undefined behavior; consider restricting the handler to setting an atomic flag or writing to a pipe and then triggering `quit()` from the Qt event loop instead.
- If you need custom signal handling semantics (e.g., avoiding automatic reset of handlers), consider switching from `signal()` to `sigaction()` to make the SIGTERM handling more robust and explicit.
## Individual Comments
### Comment 1
<location> `src/main.cpp:103` </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()` directly from a POSIX signal handler is not async-signal-safe.
Within a POSIX signal handler you should only call async-signal-safe functions (e.g. `write`, `_exit`). `QApplication::quit()` may access Qt internals and cause undefined behavior in this context. Instead, have the handler set an `std::atomic<bool>` or write to a pipe, and then call `QCoreApplication::quit()` from the main/event loop when that flag or FD is detected.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review我来对这个 diff 进行详细的代码审查:
建议:
signal(SIGINT, handleSignal); // 处理 Ctrl+C
signal(SIGTERM, handleSignal); // 处理 kill 命令
潜在风险:
static volatile sig_atomic_t shouldQuit = 0;
static void handleSignal(int sig) {
shouldQuit = 1;
}
// 在主循环中
if (shouldQuit) {
QApplication::quit();
}
if (signal(SIGTERM, handleSignal) == SIG_ERR) {
qWarning() << "Failed to register SIGTERM handler";
}
总体来说,这是一个很好的改进,解决了程序退出时的资源清理问题。建议进一步完善信号处理的健壮性,并考虑添加更多信号类型的处理。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lichaofan2008, max-lvs 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) |
a8dd239
into
linuxdeepin:release/eagle
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
Summary by Sourcery
Bug Fixes: