Skip to content

fix: Listen for SIGTERM and exit the application gracefully.#444

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
lichaofan2008:master
Jan 22, 2026
Merged

fix: Listen for SIGTERM and exit the application gracefully.#444
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
lichaofan2008:master

Conversation

@lichaofan2008
Copy link
Contributor

@lichaofan2008 lichaofan2008 commented Jan 21, 2026

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:

  • Prevent crashes and logging misclassification caused by using exit(0) on SIGTERM by switching to QApplication-based shutdown and registering the handler after the application is created.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 21, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This 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 shutdown

sequenceDiagram
    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
Loading

File-Level Changes

Change Details Files
Handle SIGTERM by gracefully quitting the Qt application instead of calling exit(0).
  • Replace direct exit(0) call in the generic signal handler with QApplication::quit() to allow Qt to perform its normal shutdown and resource cleanup
  • Retain the old exit(0) call as a commented-out fallback with an explanatory comment documenting prior crashes when using exit(0) on certain hardware
src/main.cpp
Register the SIGTERM signal handler after QApplication (CApplication) is constructed to ensure the handler can safely interact with the Qt application instance.
  • Move the SIGTERM signal registration from the beginning of main() to just after CApplication a(argc, argv) is created
  • Add comments explaining the rationale for registering the signal handler after QApplication creation to ensure a valid application context during signal handling
src/main.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

Hey - I've found 1 issue, and left some high level feedback:

  • 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.
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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
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-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要涉及信号处理函数 handleSignal 的实现以及其注册时机的调整。以下是对这段代码的详细审查和改进建议:

1. 代码逻辑与语法审查

  • 修改点 1:handleSignal 函数实现

    • 当前逻辑:原代码直接调用 exit(0)。新代码使用 QTimer::singleShot 异步调用 QApplication::quit()
    • 分析:这是一个非常关键的修复。
      • 问题exit() 是 C 标准库函数,它会立即终止进程,跳过栈上对象的析构函数,导致 C++ 对象(特别是 Qt 对象)无法正常释放资源。在信号处理函数(Signal Handler)中直接调用 exit() 尤其危险,因为信号处理函数是异步中断上下文,可能发生在任何函数执行中间。
      • 改进QTimer::singleShot 将退出操作投递到 Qt 的事件循环中。这意味着当前的信号处理函数会快速返回,程序回到主线程的事件循环后,再安全地执行 quit()。这允许 Qt 正常清理事件循环,析构对象,避免了"空指针异常"和"资源回收环节"的问题。
    • 评价:逻辑正确,符合 Qt 编程的最佳实践。
  • 修改点 2:信号注册时机

    • 当前逻辑:原代码在 main 函数开头注册 SIGTERM。新代码将其移到了 CApplication a(argc, argv) 实例化之后。
    • 分析
      • 问题:原代码中,如果信号在 QApplication 构造完成前到达,handleSignal 中的 qApp(即 QApplication::instance())可能还未初始化(为 nullptr)。虽然 handleSignal 中当时只是 exit(0),但如果依赖 qApp 指针,就会崩溃。
      • 改进:移到 CApplication 实例化之后,确保 qApp 指针有效,且 Qt 事件循环已准备好接收 QTimer 投递的退出事件。
    • 评价:逻辑正确,修复了潜在的空指针崩溃风险。

2. 代码质量与规范

  • 异步安全性:在信号处理函数中调用 Qt 函数(如 qWarning())在技术上属于未定义行为(Undefined Behavior),因为标准 POSIX 信号处理函数中只能调用 "async-signal-safe" 的函数(如 write)。qWarning() 内部可能涉及锁或内存分配。
    • 现状:虽然 qWarning() 在 Linux 的 glibc 实现中通常能工作,但在高并发或极端情况下可能导致死锁或崩溃。
    • 建议:如果追求极致严谨,应移除 qWarning(),或者仅使用 write(STDERR_FILENO, ...)。但在实际 Qt 应用开发中,为了保留日志信息,保留 qWarning() 是常见的权衡做法,只要意识到其中的风险即可。
  • Lambda 表达式:使用 [] 捕获是安全的,因为这里没有捕获外部变量。

3. 代码性能

  • 影响QTimer::singleShot 的开销极小,相比于 exit() 导致的资源泄漏和潜在崩溃,性能损耗可以忽略不计。
  • 评价:性能影响微乎其微,换取了稳定性,值得。

4. 代码安全

  • 竞态条件(TOCTOU):在 CApplication 构造和 signal(...) 调用之间的极短时间内,如果程序收到 SIGTERM,信号仍会被执行默认动作(终止进程)。由于这段代码执行时间极短,且发生在程序启动阶段,被攻击或利用的概率极低,属于可接受范围。
  • 资源释放:新方案通过 QApplication::quit() 触发正常退出流程,确保了 CApplication 及其子对象的析构函数被调用,资源(如文件句柄、共享内存、摄像头设备)能被正确释放,提高了系统稳定性。

5. 改进建议

虽然当前的修改已经解决了主要问题,但为了进一步提高代码的健壮性,建议考虑以下几点:

  1. 增加 qApp 空指针检查
    尽管注册时机移到了 CApplication 之后,但为了防御性编程,建议在 Lambda 中增加检查,防止未来代码移动导致回归问题。

    QTimer::singleShot(0, qApp, []() {
        if (qApp) { // 防御性检查
            QApplication::quit();
        }
    });
  2. 关于 qWarning() 的说明
    如前所述,信号处理函数中调用非异步安全函数有风险。如果这是关键安全模块,建议注释中注明这一点,或者简化日志输出。但对于桌面应用(deepin-camera),当前做法通常是可以接受的。

  3. 宏定义保护
    如果 CApplication 的构造非常复杂,且某些平台可能不支持 Qt 事件循环,可以考虑增加编译宏判断,确保在特定环境下回退到安全模式(虽然在此场景下不太需要)。

总结

这段代码修改是高质量的,有效地解决了 exit(0) 导致的资源清理不彻底和崩溃问题,通过调整注册时机修复了潜在的空指针风险。建议采纳,并补充上述关于 qApp 空指针检查的防御性代码。

优化后的代码示例:

static void handleSignal(int sig)
{
    // 注意:在信号处理函数中应尽量避免调用非 async-signal-safe 的函数。
    // 此处保留 qWarning 用于调试,但在生产环境中需评估风险。
    qWarning() << "Received signal:" << sig;
    
    // 使用 QTimer::singleShot 安全地退出,确保在主事件循环中执行清理
    QTimer::singleShot(0, qApp, []() {
        // 增加防御性检查,确保 qApp 依然有效
        if (qApp) {
            QApplication::quit();
        }
    });
}

@deepin-ci-robot
Copy link

[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.

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

@lichaofan2008
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 22, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit c5925fc into linuxdeepin:master Jan 22, 2026
16 of 18 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.

3 participants