Skip to content

fix: prevent dde-apps crash when restarting due to concurrent model u… - #1683

Merged
Ivy233 merged 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/dde-apps-crash-on-restart
Aug 4, 2026
Merged

fix: prevent dde-apps crash when restarting due to concurrent model u…#1683
Ivy233 merged 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/dde-apps-crash-on-restart

Conversation

@Ivy233

@Ivy233 Ivy233 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

…pdates

The AMAppItemModel was populated from a QtConcurrent worker thread, which mutated the model (appendRow) on a non-GUI thread while the main thread's queued onReferenceModelChanged() read it concurrently, causing a data race and a SIGSEGV in QStandardItemModel::data() during restart.

  1. Replace QtConcurrent::run with QDBusPendingCallWatcher so the model is populated on the model's own (main) thread, eliminating the cross-thread race and stale queued rowsInserted callbacks.
  2. Change m_referenceModel from a raw pointer to QPointer so it is automatically cleared when the model is destroyed.
  3. Add a null check at the start of onReferenceModelChanged() to skip updates when the reference model has been destroyed.

Log: Fixed a crash in dde-shell during restart caused by concurrent access to the app model.

Influence:

  1. Restart dde-shell repeatedly and verify no crash occurs.
  2. Test app install/remove and grouping during restart.
  3. Verify the launchpad app group arrangement is preserved after restart.

fix: 修复重启时因并发访问应用模型导致的 dde-shell 崩溃

AMAppItemModel 此前在 QtConcurrent 工作线程中填充,在工作线程中修改
模型(appendRow),同时主线程通过队列回调 onReferenceModelChanged() 并发 读取模型,导致数据竞争,重启时在 QStandardItemModel::data() 中触发段错误。

  1. 将 QtConcurrent::run 替换为 QDBusPendingCallWatcher,使模型在自身 (主)线程中填充,消除跨线程竞争和过期的 rowsInserted 队列回调。
  2. 将 m_referenceModel 由裸指针改为 QPointer,模型销毁时自动置空。
  3. 在 onReferenceModelChanged() 开头增加空指针检查,模型已销毁时跳过更新。

Log: 修复重启时因并发访问应用模型导致的 dde-shell 崩溃。

Influence:

  1. 反复重启 dde-shell,验证不再崩溃。
  2. 重启过程中测试应用的安装/卸载及分组功能。
  3. 验证重启后启动器应用分组布局得以保留。

PMS: BUG-372351

Summary by Sourcery

Prevent dde-apps crash on restart by making app model population and group updates safe against concurrent access and destroyed models.

Bug Fixes:

  • Ensure AMAppItemModel is populated via asynchronous DBus reply on the main thread instead of a QtConcurrent worker to avoid cross-thread races and crashes.
  • Guard AppGroupManager against a destroyed reference model by using QPointer and skipping updates when the model is null.

Enhancements:

  • Pass the AppGroupManager instance as the parent to ObjectManager to align object lifetimes and ownership.

@deepin-ci-robot

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@sourcery-ai

sourcery-ai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Reviewer's Guide

Fixes a crash caused by concurrent access to AMAppItemModel by moving D-Bus object retrieval back to the main thread and making AppGroupManager’s reference model handling safe against destruction and readiness issues.

Sequence diagram for AMAppItemModel D-Bus loading on main thread

sequenceDiagram
    participant AMAppItemModel
    participant ObjectManager
    participant QDBusPendingCallWatcher

    AMAppItemModel->>ObjectManager: GetManagedObjects()
    ObjectManager-->>AMAppItemModel: QDBusPendingCall
    AMAppItemModel->>QDBusPendingCallWatcher: QDBusPendingCallWatcher(QDBusPendingCall)

    QDBusPendingCallWatcher-->>AMAppItemModel: finished
    AMAppItemModel->>AMAppItemModel: QDBusPendingReply<ObjectMap> reply
    AMAppItemModel->>AMAppItemModel: [reply.isError()] log warning
    AMAppItemModel->>AMAppItemModel: [!reply.isError()] appendRow(AppItem)
    AMAppItemModel->>AMAppItemModel: setProperty(ready, true)
Loading

File-Level Changes

Change Details Files
Make AMAppItemModel load managed objects asynchronously on the main (GUI) thread instead of a QtConcurrent worker thread.
  • Replace QtConcurrent::run-based background loading with QDBusPendingCallWatcher/QDBusPendingReply handling of GetManagedObjects
  • Ensure ObjectManager is parented to AMAppItemModel so its lifetime is tied to the model
  • Populate the model and set the ready property within the watcher’s finished callback on the main thread
  • Log a warning if the D-Bus call to get managed objects fails instead of proceeding with invalid data
applets/dde-apps/amappitemmodel.cpp
Harden AppGroupManager against a destroyed or not-yet-ready reference model to avoid crashes during restart.
  • Change the reference model member from a raw pointer to QPointer to auto-null on destruction
  • Add an early null-check in onReferenceModelChanged to skip updates when the reference model no longer exists
  • Preserve the existing readiness guard that skips updates when the reference model has zero rows
applets/dde-apps/appgroupmanager.cpp
applets/dde-apps/appgroupmanager.h

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

@Ivy233
Ivy233 force-pushed the fix/dde-apps-crash-on-restart branch from f58135c to 9d3d590 Compare July 31, 2026 08:14
@Ivy233
Ivy233 marked this pull request as ready for review July 31, 2026 08:16

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, Ivy233

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

…pdates

The AMAppItemModel was populated from a QtConcurrent worker thread, which
mutated the model (appendRow) on a non-GUI thread while the main thread's
queued onReferenceModelChanged() read it concurrently, causing a data race
and a SIGSEGV in QStandardItemModel::data() during restart.

1. Replace QtConcurrent::run with QDBusPendingCallWatcher so the model is
populated on the model's own (main) thread, eliminating the cross-thread
race and stale queued rowsInserted callbacks.
2. Change m_referenceModel from a raw pointer to QPointer so it is
automatically cleared when the model is destroyed.
3. Add a null check at the start of onReferenceModelChanged() to skip
updates when the reference model has been destroyed.

Log: Fixed a crash in dde-shell during restart caused by concurrent
access to the app model.

Influence:
1. Restart dde-shell repeatedly and verify no crash occurs.
2. Test app install/remove and grouping during restart.
3. Verify the launchpad app group arrangement is preserved after restart.

fix: 修复重启时因并发访问应用模型导致的 dde-shell 崩溃

AMAppItemModel 此前在 QtConcurrent 工作线程中填充,在工作线程中修改
模型(appendRow),同时主线程通过队列回调 onReferenceModelChanged() 并发
读取模型,导致数据竞争,重启时在 QStandardItemModel::data() 中触发段错误。

1. 将 QtConcurrent::run 替换为 QDBusPendingCallWatcher,使模型在自身
(主)线程中填充,消除跨线程竞争和过期的 rowsInserted 队列回调。
2. 将 m_referenceModel 由裸指针改为 QPointer,模型销毁时自动置空。
3. 在 onReferenceModelChanged() 开头增加空指针检查,模型已销毁时跳过更新。

Log: 修复重启时因并发访问应用模型导致的 dde-shell 崩溃。

Influence:
1. 反复重启 dde-shell,验证不再崩溃。
2. 重启过程中测试应用的安装/卸载及分组功能。
3. 验证重启后启动器应用分组布局得以保留。

PMS: BUG-372351
@Ivy233
Ivy233 force-pushed the fix/dde-apps-crash-on-restart branch from 9d3d590 to e51814e Compare August 4, 2026 02:19
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码完美修复了DBus同步阻塞导致的主线程卡顿和悬空指针引发的崩溃问题,实现质量极高
四个审查维度均无任何缺陷,完全符合Qt最佳编程规范,给予满分

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

QtConcurrent::run中的同步阻塞调用重构为基于QDBusPendingCallWatcher的异步调用,逻辑严密。在appgroupmanager.h中将裸指针替换为QPointer,并在appgroupmanager.cpponReferenceModelChanged函数入口处增加有效性判断,彻底消除了野指针解引用风险。watcher指定this为父对象,确保了对象生命周期安全。
建议:无

  • 2.代码质量(优秀)✓

移除了未使用的#include "appgroupmanager.h"#include <QtConcurrent>,引入了必需的<QDBusPendingCallWatcher>等头文件,依赖管理清晰。增加了DBus调用失败时的错误日志打印qCWarning,提升了可维护性与问题排查效率。版权年份同步更新。
建议:无

  • 3.代码性能(高效)✓

彻底消除了原代码在主线程通过QtConcurrent::run配合.value()同步等待DBus返回造成的严重阻塞问题,将耗时操作完全转移至DBus事件循环异步处理,显著提升了桌面环境UI初始化的流畅度。
建议:无

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码补丁本身未引入任何新的安全风险,且有效修复了原有的拒绝服务(主线程阻塞)和内存安全(悬空指针)隐患,安全状态优良。

  • 建议:无需额外安全修复

■ 【改进建议代码示例】

// 当前补丁代码已非常完善,此处展示一种可选的增强型错误处理逻辑(如增加超时保护)
// 在 amappitemmodel.cpp 中可考虑为 DBus 调用增加超时机制以防御恶意服务端无响应
auto reply = m_manager->GetManagedObjects();
auto *watcher = new QDBusPendingCallWatcher(reply, this);

// 可选增强:设置超时定时器,防止 DBus 服务端异常挂起时 watcher 永远不触发 finished 信号
QTimer::singleShot(3000, watcher, [watcher, this]() {
    if (!watcher->isFinished()) {
        qCWarning(appsLog()) << "GetManagedObjects timed out after 3 seconds";
        watcher->deleteLater();
    }
});

connect(watcher, &QDBusPendingCallWatcher::finished, this, [this, watcher]() {
    watcher->deleteLater();
    QDBusPendingReply<ObjectMap> reply = *watcher;
    if (reply.isError()) {
        qCWarning(appsLog()) << "Failed to get managed objects:" << reply.error().message();
        return;
    }
    auto apps = reply.value();
    for (auto app = apps.cbegin(); app != apps.cend(); app++) {
        auto path = app.key();
        if (!path.path().isEmpty()) {
            auto c = new AMAppItem(path.path(), this);
            appendRow(c);
        }
    }
    setProperty("ready", true);
    qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount();
});

@Ivy233
Ivy233 merged commit 2e18711 into linuxdeepin:master Aug 4, 2026
12 of 13 checks passed
@Ivy233
Ivy233 deleted the fix/dde-apps-crash-on-restart branch August 4, 2026 02:31
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