Skip to content

fix: improve tray applet lifecycle management and model safety - #1687

Merged
wjyrich merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-coredumpWithDconfig
Aug 4, 2026
Merged

fix: improve tray applet lifecycle management and model safety#1687
wjyrich merged 1 commit into
linuxdeepin:masterfrom
wjyrich:fix-coredumpWithDconfig

Conversation

@wjyrich

@wjyrich wjyrich commented Aug 4, 2026

Copy link
Copy Markdown
Contributor
  1. Refactor tray applet retrieval into a dedicated trayApplet() method that accepts a QPointer<QObject> instead of raw pointers, safely managing the connection lifecycle and handling applet recreation
  2. Remove the repeated timer-based polling logic that created a new bridge connection each time
  3. Properly disconnect and reconnect the pluginsChanged signal when the tray applet instance changes, using Qt::UniqueConnection to avoid duplicate connections
  4. Add null checks and index validation in DockItemModel::data() and mapToSource() to prevent potential crashes from invalid model indices or missing source models
  5. Change the timer to be parented to this for proper cleanup

Log: Fixed potential issues with tray applet lifecycle management and model access safety

Influence:

  1. Verify the dock tray area displays correctly with various applications running
  2. Test applet updates when applications add/remove tray icons dynamically
  3. Validate the dock list remains stable during frequent tray updates
  4. Test with edge cases: empty model, invalid indices
  5. Verify the initial plugin list logging still works after the timer refactor

fix: 改进托盘小程序生命周期管理和模型安全性

  1. 将托盘小程序获取重构为专用的 trayApplet() 方法,使用 QPointer<QObject> 替代原始指针,安全管理连接生命周期并处理小程序重建
  2. 移除重复的基于定时器的轮询逻辑,该逻辑每次都会创建新的桥接连接
  3. 当托盘小程序实例变化时正确断开和重新连接 pluginsChanged 信号,使用 Qt::UniqueConnection 避免重复连接
  4. DockItemModel::data()mapToSource() 中添加空指针检查和索引 验证,防止无效模型索引或缺失源模型导致的潜在崩溃
  5. 将定时器设置为 this 的子对象以确保正确的清理

Log: 修复托盘小程序生命周期管理和模型访问安全性的潜在问题

Influence:

  1. 验证在各种应用运行时,dock 托盘区域显示是否正确
  2. 测试应用动态添加/移除托盘图标时的小程序更新
  3. 验证在频繁托盘更新期间 dock 列表是否保持稳定
  4. 测试边界情况:空模型、无效索引
  5. 验证定时器重构后初始插件列表日志记录功能是否正常

PMS: TASK-393637

Summary by Sourcery

Improve tray applet lifecycle handling and dock model safety.

Bug Fixes:

  • Prevent crashes and inconsistent behavior when the tray applet instance is recreated or unavailable.
  • Guard dock item model access against null source models and invalid indices.

Enhancements:

  • Introduce a dedicated trayApplet() helper that manages tray applet retrieval and signal connections with QPointer and unique connections.
  • Simplify tray initialization by removing repeated timer-based polling and ensuring the timer is properly parented for cleanup.

@sourcery-ai

sourcery-ai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Reviewer's Guide

Refactors tray applet handling to use a dedicated, safer retrieval method with QPointer and proper signal management, removes redundant timer polling, and hardens DockItemModel against invalid indices and null source models.

Sequence diagram for tray applet lifecycle management

sequenceDiagram
    participant DockDBusProxy
    participant QTimer
    participant DAppletBridge
    participant TrayApplet

    QTimer->>DockDBusProxy: timeout()
    DockDBusProxy->>DockDBusProxy: trayApplet()
    DockDBusProxy->>DAppletBridge: DAppletBridge("org.deepin.ds.dock.tray")
    DAppletBridge-->>DockDBusProxy: applet()
    alt [applet changed]
        DockDBusProxy->>TrayApplet: QObject::disconnect(pluginsChanged)
        DockDBusProxy->>TrayApplet: QObject::connect(pluginsChanged, Qt::UniqueConnection)
        DockDBusProxy-->>DockDBusProxy: m_trayApplet = currentApplet
    else [applet unchanged]
        DockDBusProxy-->>DockDBusProxy: reuse m_trayApplet
    end
    DockDBusProxy-->>QTimer: stop()
    DockDBusProxy-->>QTimer: deleteLater()
    DockDBusProxy-->>DockDBusProxy: QTimer::singleShot(30000, logInitialPluginState())
Loading

File-Level Changes

Change Details Files
Introduce a dedicated trayApplet() helper that safely manages the tray applet instance, signal connections, and reconnections using QPointer.
  • Add trayApplet() private method to fetch the tray applet via DAppletBridge and cache it
  • Use QPointer for m_trayApplet instead of a raw DS_NAMESPACE::DAppletProxy pointer
  • On applet change, disconnect old pluginsChanged signal and connect the new one with Qt::UniqueConnection
panels/dock/dockdbusproxy.cpp
panels/dock/dockdbusproxy.h
Simplify and make safe the timer-based initialization of the tray applet, removing repeated polling and ensuring proper cleanup.
  • Parent the initialization QTimer to DockDBusProxy (this) for automatic cleanup
  • Replace lambda-based getOtherApplet() with calls to trayApplet()
  • Stop and delete the timer once a tray applet is available, keeping the delayed logInitialPluginState logic intact
panels/dock/dockdbusproxy.cpp
Route tray-related operations through trayApplet() to ensure they use the current, valid applet instance.
  • Use trayApplet() when invoking dockItemInfos via QMetaObject::invokeMethod
  • Use trayApplet() when calling setItemOnDock and emitting pluginVisibleChanged
panels/dock/dockdbusproxy.cpp
Add defensive checks in DockItemModel to avoid crashes from null source models and invalid indices.
  • In data(), return an empty QVariant when sourceModel is null or index/sourceIndex is invalid before accessing data
  • In mapToSource(), check for null sourceModel and invalid proxyIndex and return a default QModelIndex in those cases
panels/dock/taskmanager/dockitemmodel.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

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

Hey - I've reviewed your changes and they look great!


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.

return m_trayApplet.data();
}

if (m_trayApplet) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

当m_trayApplet被析构了,这个QPointer的值是个空的吧,不过为了以后可能多个applet,这个留着也行,

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, wjyrich

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

1. Refactor tray applet retrieval into a dedicated `trayApplet()` method
that accepts a `QPointer<QObject>` instead of raw pointers, safely
managing the connection lifecycle and handling applet recreation
2. Remove the repeated timer-based polling logic that created a new
bridge connection each time
3. Properly disconnect and reconnect the `pluginsChanged` signal when
the tray applet instance changes, using `Qt::UniqueConnection` to avoid
duplicate connections
4. Add null checks and index validation in `DockItemModel::data()` and
`mapToSource()` to prevent potential crashes from invalid model indices
or missing source models
5. Change the timer to be parented to `this` for proper cleanup

Log: Fixed potential issues with tray applet lifecycle management and
model access safety

Influence:
1. Verify the dock tray area displays correctly with various
applications running
2. Test applet updates when applications add/remove tray icons
dynamically
3. Validate the dock list remains stable during frequent tray updates
4. Test with edge cases: empty model, invalid indices
5. Verify the initial plugin list logging still works after the timer
refactor

fix: 改进托盘小程序生命周期管理和模型安全性

1. 将托盘小程序获取重构为专用的 `trayApplet()` 方法,使用
`QPointer<QObject>` 替代原始指针,安全管理连接生命周期并处理小程序重建
2. 移除重复的基于定时器的轮询逻辑,该逻辑每次都会创建新的桥接连接
3. 当托盘小程序实例变化时正确断开和重新连接 `pluginsChanged` 信号,使用
`Qt::UniqueConnection` 避免重复连接
4. 在 `DockItemModel::data()` 和 `mapToSource()` 中添加空指针检查和索引
验证,防止无效模型索引或缺失源模型导致的潜在崩溃
5. 将定时器设置为 `this` 的子对象以确保正确的清理

Log: 修复托盘小程序生命周期管理和模型访问安全性的潜在问题

Influence:
1. 验证在各种应用运行时,dock 托盘区域显示是否正确
2. 测试应用动态添加/移除托盘图标时的小程序更新
3. 验证在频繁托盘更新期间 dock 列表是否保持稳定
4. 测试边界情况:空模型、无效索引
5. 验证定时器重构后初始插件列表日志记录功能是否正常

PMS: TASK-393637
@wjyrich
wjyrich force-pushed the fix-coredumpWithDconfig branch from 22c9f85 to 0ea0748 Compare August 4, 2026 02:45
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码有效修复了悬空指针和空指针解引用导致的潜在崩溃问题,质量良好
逻辑完全正确且无安全漏洞,仅因使用传统字符串信号槽宏扣5分

■ 【详细分析】

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

在 dockdbusproxy.cpp 中将 m_trayApplet 从裸指针重构为 QPointer,并在 trayApplet() 方法中实现了安全的断连与重连逻辑,彻底解决了原对象被销毁后指针悬空的问题。在 dockitemmodel.cpp 的 data() 和 mapToSource() 中增加了对 sourceModel 和 index 的有效性校验,防止了空指针解引用。同时修复了构造函数中 new QTimer 未指定 parent 导致的潜在内存泄漏。

建议:无

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

代码结构清晰,将获取 tray applet 的逻辑封装为独立的 trayApplet() 方法,提高了复用性并集中管理了生命周期。使用 QPointer 是管理跨模块 QObject 指针的最佳实践。由于 m_trayApplet 被泛化为 QObject* 以兼容 QPointer,导致无法使用 Qt5 推荐的基于函数指针的 connect 语法,只能退而使用 SIGNAL/SLOT 字符串宏,这在当前类型约束下是合理的妥协。

建议:若后续能确定 tray applet 的具体类型或引入接口基类,可考虑将 SIGNAL 宏替换为函数指针以获得编译期类型检查

  • 3.代码性能(无性能问题)✓

trayApplet() 方法在每次调用时都会实例化 DAppletBridge 并通过 IPC 获取 applet 指针。虽然存在微小的对象构造和 IPC 查询开销,但该函数仅在 DBus 接口被外部调用时触发,频率极低,不会对系统性能造成影响。

建议:无

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次提交为纯粹的安全修复,消除了因悬空指针和空指针解引用引发的本地拒绝服务风险,未引入新的安全漏洞。通过 QPointer 严格约束了对象生命周期,通过有效性检查拦截了非法索引访问。

  • 建议:保持现有的防御性编程风格

■ 【改进建议代码示例】

// dockdbusproxy.cpp —— 若后续引入 ITrayApplet 接口,可优化信号连接的编译期检查
// 假设定义了接口类 ITrayApplet : public QObject
QObject *DockDBusProxy::trayApplet()
{
    DAppletBridge bridge("org.deepin.ds.dock.tray");
    QObject *currentApplet = bridge.applet();
    if (m_trayApplet.data() == currentApplet) {
        return m_trayApplet.data();
    }

    if (m_trayApplet) {
        QObject::disconnect(m_trayApplet.data(), SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged()));
    }

    m_trayApplet = currentApplet;
    if (m_trayApplet) {
        // 理想情况:若能安全转换为目标接口类型,可使用函数指针替代字符串宏
        // if (auto *tray = qobject_cast<ITrayApplet*>(m_trayApplet.data())) {
        //     connect(tray, &ITrayApplet::pluginsChanged, this, &DockDBusProxy::pluginsChanged, Qt::UniqueConnection);
        // } else 
        {
            QObject::connect(m_trayApplet.data(), SIGNAL(pluginsChanged()), this, SIGNAL(pluginsChanged()), Qt::UniqueConnection);
        }
    }
    return m_trayApplet.data();
}

@wjyrich
wjyrich merged commit f446e11 into linuxdeepin:master Aug 4, 2026
12 of 13 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