Skip to content

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

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle1
Dec 26, 2025
Merged

fix: Listen for SIGTERM and exit the application gracefully.#442
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle1

Conversation

@lichaofan2008
Copy link
Contributor

@lichaofan2008 lichaofan2008 commented Dec 26, 2025

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:

  • Prevent crashes during shutdown by replacing direct process exit on SIGTERM with QApplication::quit() and ensuring the SIGTERM handler is registered only after the application instance exists.

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
@sourcery-ai
Copy link

sourcery-ai bot commented Dec 26, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Handle 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_quit

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

Flow diagram for main initialization and SIGTERM handler registration

flowchart 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]
Loading

File-Level Changes

Change Details Files
Change SIGTERM handling to quit the Qt application gracefully instead of forcing an immediate process exit.
  • Replace exit(0) in the SIGTERM handler with QApplication::quit() to allow Qt to perform orderly shutdown and resource cleanup.
  • Leave a commented-out exit(0) as a fallback/reference and document the crash issue caused by direct exit() on certain hardware/software configurations.
  • Move SIGTERM signal registration from the top of main() to after CApplication a(argc, argv) is constructed so that QApplication::quit() is safe to call.
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 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.
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>

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.

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这个 diff 进行详细的代码审查:

  1. 信号处理函数的改进:
    优点:
  • 将 exit(0) 改为 QApplication::quit() 是一个很好的改进。这确实能避免直接退出导致的资源清理问题,让 Qt 有机会优雅地清理资源。
  • 注释清晰地解释了修改原因,增加了代码的可维护性。

建议:

  • 可以考虑在 handleSignal 中添加对其他重要信号的处理,如 SIGINT (Ctrl+C):
signal(SIGINT, handleSignal);  // 处理 Ctrl+C
signal(SIGTERM, handleSignal);  // 处理 kill 命令
  1. 信号注册位置的改进:
    优点:
  • 将信号注册移到 QApplication 实例创建之后是正确的做法。这确保了 Qt 事件循环已经准备好处理退出请求。

潜在风险:

  • 目前只处理了 SIGTERM 信号。在实际使用中,程序可能还会收到其他信号,建议考虑处理更多信号类型。
  1. 代码安全性建议:
  • 信号处理函数中应该尽量减少操作,因为信号处理是异步的。虽然使用 QApplication::quit() 比直接 exit(0) 更安全,但仍然建议在信号处理函数中只设置一个标志位,然后在主循环中检查这个标志位:
static volatile sig_atomic_t shouldQuit = 0;

static void handleSignal(int sig) {
    shouldQuit = 1;
}

// 在主循环中
if (shouldQuit) {
    QApplication::quit();
}
  1. 其他改进建议:
  • 可以添加对信号处理函数返回值的检查:
if (signal(SIGTERM, handleSignal) == SIG_ERR) {
    qWarning() << "Failed to register SIGTERM handler";
}
  1. 代码质量:
  • 注释清晰,解释了修改原因
  • 代码格式规范
  • 修改范围合理,没有引入不必要的变更

总体来说,这是一个很好的改进,解决了程序退出时的资源清理问题。建议进一步完善信号处理的健壮性,并考虑添加更多信号类型的处理。

@deepin-ci-robot
Copy link

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

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 Dec 26, 2025

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit a8dd239 into linuxdeepin:release/eagle Dec 26, 2025
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