Skip to content

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

Open
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/dde-apps-crash-on-restart
Open

fix: prevent dde-apps crash when restarting due to concurrent model u…#1683
Ivy233 wants to merge 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

…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 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

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码通过引入异步DBus调用和QPointer有效修复了潜在的主线程阻塞和空指针解引用问题
逻辑正确且提升了性能与稳定性,仅因异步错误处理不够完善扣5分

■ 【详细分析】

  • 1.语法逻辑(基本正确)✓

代码修改在amappitemmodel.cpp中将同步调用改为异步回调,逻辑结构完整;appgroupmanager.cpp中增加了空指针检查,避免了潜在的崩溃问题
潜在问题:无
建议:无需修改

  • 2.代码质量(良好)✓

引入QDBusPendingCallWatcher和QPointer符合Qt最佳实践,内存管理通过parent机制和deleteLater得到保障
潜在问题:异步回调中的错误处理仅打印了日志,未通知上层或进行重试
建议:可根据业务需求考虑增加错误重试机制或状态通知

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

将QtConcurrent::run中的同步DBus调用替换为原生的异步DBus调用,避免了线程池资源的浪费和主线程的潜在阻塞
潜在问题:无
建议:无需修改

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
代码未引入任何安全漏洞,修复了原有的空指针风险,提升了代码安全性

  • 建议:继续保持当前的安全编码实践

■ 【改进建议代码示例】

// 在 amappitemmodel.cpp 中增强错误处理逻辑
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();
        // 可选:发出错误信号通知UI层或触发重试机制
        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(), app.value());
            appendRow(c);
        }
    }
    setProperty("ready", true);
    qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount();
});

@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

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