Skip to content

fix: fix application tray crash due to parent-child ownership and null pointer issues#465

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
18202781743:fix-tray-crash
Jun 8, 2026
Merged

fix: fix application tray crash due to parent-child ownership and null pointer issues#465
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
18202781743:fix-tray-crash

Conversation

@18202781743

@18202781743 18202781743 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

问题描述

应用程序托盘在 SNI (StatusNotifierItem) 托盘项注销时崩溃。

根本原因

  1. 双重生命周期管理SniTrayProtocolHandler 同时被 QSharedPointer 和 QObject 父子关系管理,导致双重删除
  2. 悬空指针m_tooltip 中存储的 widget 指针在 handler 销毁后变成悬空指针
  3. 空指针访问eventFilter 中使用 parent() 获取 widget,但移除 setParent(this) 后返回错误对象

修复内容

traywidget.cpp

  • 移除 m_handler->setParent(this) 避免双重生命周期管理
  • showEventpaintEvent 中添加 m_handler 空指针检查

sniprotocolhandler.cpp

  • eventFilter 中将 parent() 替换为 window()
  • 添加 windowHandle() 的空指针检查

ddeindicatortrayprotocol.cpp

  • updateContent 中将 q->parent() 替换为 q->window()
  • eventFilter 中将 parent() 替换为 window(),添加空指针检查

trayplugin.cpp

  • remove lambda 中,先清理 m_tooltip,再调用 itemRemoved,避免访问悬空指针

测试

修复后需要验证:

  1. SNI 托盘项正常显示和移除
  2. 右键菜单正常弹出
  3. 左键激活功能正常
  4. 不再出现崩溃

🤖 Generated with Claude Code

Summary by Sourcery

Prevent crashes and dangling pointers in the application tray by aligning tray handler ownership with Qt windowing semantics and cleaning up tooltip references safely.

Bug Fixes:

  • Avoid double deletion of tray protocol handlers by relying on shared pointer ownership instead of QObject parent-child relationships.
  • Prevent null and dangling pointer access in tray widget show/paint events and tooltip handling when tray items are removed.
  • Ensure SNI and indicator tray event filters operate on window objects with proper null checks to avoid crashes when windows or window handles are missing.

Enhancements:

  • Update tray-related event handling to use window-based access instead of parent-based casting for more robust widget updates and painting.

pointer issues

1. Fix crash in DDEindicatorTrayProtocol by using window() instead of
parent() for event filter and content updates
2. Add null pointer checks in DDEindicatorTrayProtocol event filter
3. Fix crash in SniTrayProtocolHandler by using window() instead of
parent() and adding null checks
4. Fix tooltip removal order in TrayPlugin to avoid dangling pointer
references
5. Fix double-delete crash in TrayWidget by not setting handler as child
widget
6. Add null handler checks in TrayWidget showEvent and paintEvent

Log: Fixed application tray crashes when tray items are added/removed

Influence:
1. Test adding and removing multiple tray items
2. Test tray indicator protocol operations
3. Test sni protocol tray operations
4. Test tray widget paint and show events
5. Test tray icon updates and tooltip display
6. Test application tray with various indicator types

feat: 修复应用托盘因父子关系和空指针导致的崩溃问题

1. 修复 DDEindicatorTrayProtocol 中,使用 window() 替代 parent() 进行事
件过滤和内容更新
2. 在 DDEindicatorTrayProtocol 的事件过滤器中添加空指针检查
3. 修复 SniTrayProtocolHandler 中,使用 window() 替代 parent() 并添加空
指针检查
4. 修复 TrayPlugin 中 tooltip 移除顺序,避免悬空指针引用
5. 修复 TrayWidget 中不将 handler 设置为子控件导致的二次删除崩溃
6. 在 TrayWidget 的 showEvent 和 paintEvent 中添加 handler 空指针检查

Log: 修复添加/移除托盘项时应用托盘崩溃的问题

Influence:
1. 测试添加和移除多个托盘项
2. 测试托盘指示协议操作
3. 测试 sni 协议托盘操作
4. 测试托盘控件的绘制和显示事件
5. 测试托盘图标更新和提示信息显示
6. 测试各种指示类型下的应用托盘功能
@sourcery-ai

sourcery-ai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Reviewer's Guide

Fixes crashes in the application tray by removing conflicting QObject parent ownership for tray protocol handlers, switching to window-based lookups instead of parent() in event filters, and adding null checks and cleanup ordering for tooltip and window handle usage.

Sequence diagram for tray item removal and tooltip cleanup

sequenceDiagram
    participant TrayPlugin
    participant TooltipMap as m_tooltip
    participant ProxyInterface as m_proyInter
    participant WidgetMap as m_widget

    TrayPlugin->>TooltipMap: remove(id)
    TrayPlugin->>ProxyInterface: itemRemoved(TrayPlugin,id)
    TrayPlugin->>WidgetMap: value(id)
    TrayPlugin->>WidgetMap: remove(id)
Loading

File-Level Changes

Change Details Files
Avoid double-deletion of tray protocol handlers and guard usage in widget lifecycle hooks.
  • Removed setting the QObject parent on the handler so it is only managed by QSharedPointer ownership.
  • Added null checks for the tray protocol handler in showEvent before installing the event filter and configuring the window.
  • Added null checks for the tray protocol handler in paintEvent before performing any painting logic.
plugins/application-tray/traywidget.cpp
Use window-based widgets and add null checks to prevent crashes in DDE indicator tray protocol event handling.
  • Replaced parent() with window() when fetching the widget to update tray content, ensuring the correct top-level widget is used.
  • Changed the eventFilter condition to compare watched against window() instead of parent().
  • Replaced static_cast to parent() with window(), and added a null check before painting when handling QEvent::Paint.
plugins/application-tray/ddeindicatortrayprotocol.cpp
Prevent null pointer dereferences when showing SNI context menus by validating window handles.
  • In the SniTrayProtocolHandler eventFilter, replaced use of parent() and nested window()/windowHandle() calls with direct use of window()->windowHandle().
  • Added a null check on the retrieved window handle, returning early if it is null before accessing the EmbedPlugin.
plugins/application-tray/sniprotocolhandler.cpp
Fix tooltip lifetime and dangling pointer issues when removing tray items.
  • In the tray handler removal lambda, remove the tooltip entry from the tooltip map before notifying itemRemoved so any dangling tooltip pointer is cleared while the handler is still valid.
  • Kept widget cleanup and map removal after the protocol interface notification to preserve behavior while avoiding access to freed tooltip widgets.
plugins/application-tray/trayplugin.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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次代码变更主要涉及对Qt控件生命周期的修正、空指针防护、悬空指针清理顺序的调整以及版权声明的格式化。

整体来看,这次修改质量很高,修复了几个非常关键的潜在Bug(如双重释放、空指针解引用、悬空指针访问等)。以下是针对各个修改点的详细审查意见:

1. 语法与逻辑

👍 亮点:移除 setParent(this) 避免双重释放
traywidget.cpp 中,移除了 m_handler->setParent(this); 并添加了注释说明 handler 的生命周期由 QSharedPointer 管理。这是一个非常关键的修复。如果 Qt 控件(TrayWidget)在析构时自动删除子对象(handler),而 QSharedPointer 引用计数归零时也会删除 handler,就会导致 Double-Free(双重释放),引发严重的崩溃。

👍 亮点:调整 m_tooltip.remove(id) 的顺序
trayplugin.cppremove lambda 中,将 m_tooltip.remove(id) 移到了最前面。因为 tooltip widget 是 handler 的成员变量,当 handler 销毁时会被 Qt 的对象树机制自动删除。如果先执行 m_widget.remove(id) 或延迟删除 widget,可能导致其他代码路径访问到 m_tooltip 中已失效的指针(悬空指针)。提前移除是正确的逻辑。

⚠️ 改进建议:showEvent 中的空指针解引用风险
traywidget.cppshowEvent 中:

void TrayWidget::showEvent(QShowEvent* event)
{
    Q_UNUSED(event)
    if (!m_handler)
        return;

    m_handler->setWindow(window());
    window()->installEventFilter(m_handler); // 潜在风险
    window()->setMouseTracking(true);
}

虽然增加了 if (!m_handler) return; 的判断,但 window() 函数在控件尚未被加入任何顶层窗口时可能返回 nullptr。如果 window() 为空,window()->installEventFilter(...) 将引发空指针解引用崩溃。
建议修改为:

void TrayWidget::showEvent(QShowEvent* event)
{
    Q_UNUSED(event)
    if (!m_handler)
        return;

    QWindow *win = window();
    if (!win)
        return;

    m_handler->setWindow(win);
    win->installEventFilter(m_handler);
    win->setMouseTracking(true);
}

2. 代码质量

👍 亮点:使用 window() 替代 static_cast<QWidget*>(parent())
ddeindicatortrayprotocol.cpp 中,将 static_cast<QWidget*>(q->parent())static_cast<QWidget*>(parent()) 替换为 q->window()window()。这不仅去除了不安全的向下转型(static_cast),而且语义更清晰。在 Qt 中,获取顶层窗口用于绘制或事件过滤,使用 window() 是最标准、最安全的做法。

📝 建议:QPointer 的判空一致性
代码中使用了 QPointer<AbstractTrayProtocolHandler> handlerQPointer 在引用的对象被删除时会自动置为 nullptr。因此在 showEventpaintEvent 中增加 if (!m_handler) return; 是非常必要的。
请检查项目中是否还有其他使用 m_handler 的重写函数(如 mousePressEvent, resizeEvent 等),如果有,建议也统一加上 if (!m_handler) return; 的防护,保持代码的健壮性和一致性。

3. 代码性能

👍 逻辑正确,无性能劣化

  • window() 函数内部只是遍历父节点链表寻找顶层窗口,开销极小,与 parent() 相比在性能上没有显著差异,但安全性大幅提升。
  • 提前 m_tooltip.remove(id) 只是改变了 QMap/QHash 的操作顺序,时间复杂度不变,无性能影响。
  • showEventpaintEvent 中增加判空,引入了极微小的分支预测开销,但相比于避免崩溃,这点开销完全可以忽略不计。

4. 代码安全

👍 亮点:修复多处空指针解引用漏洞

  • ddeindicatortrayprotocol.cpp 中增加了 if (!widget) return false;,防止在 QEvent::Paint 事件中对空指针进行 QPainter painter(widget) 绘制,这会导致程序直接崩溃。
  • sniprotocolhandler.cpp 中增加了 if (!win) return false;,防止对没有 windowHandle() 的控件调用底层平台插件代码。
  • 这些都是极其重要的安全修复,因为在 Linux 桌面环境中,托盘图标的显示/隐藏、窗口的嵌套经常会导致短暂的生命周期不同步,极易触发此类空指针异常。

⚠️ 改进建议:sniprotocolhandler.cpp 中的后续逻辑安全
sniprotocolhandler.cpp 中:

auto *win = window()->windowHandle();
if (!win)
    return false;

auto plugin = Plugin::EmbedPlugin::get(win);
auto geometry = plugin->pluginPos(); // 潜在风险

增加了 win 的空指针判断,很好。但是 Plugin::EmbedPlugin::get(win) 有可能返回 nullptr(例如如果该窗口尚未注册到插件系统中)。如果 plugin 为空,下一行 plugin->pluginPos() 依然会导致空指针崩溃。
建议修改为:

auto *win = window()->windowHandle();
if (!win)
    return false;

auto plugin = Plugin::EmbedPlugin::get(win);
if (!plugin)
    return false;

auto geometry = plugin->pluginPos();

总结

本次 Diff 是一次高质量的缺陷修复,核心逻辑非常正确,显著提升了插件的稳定性和安全性。建议采纳上述关于 window()Plugin::EmbedPlugin::get() 返回值的二次空指针校验建议,代码将会更加健壮。

@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 found 2 issues, and left some high level feedback:

  • In TrayWidget::showEvent you guard against a null m_handler but still unconditionally call window()->installEventFilter and window()->setMouseTracking; consider also checking that window() is non-null before using it to avoid potential crashes during early lifecycle.
  • In SniTrayProtocolHandler::eventFilter, window()->windowHandle() is called before any null check on window(); if window() can be null in some cases, add a guard for window() itself before dereferencing it to prevent rare null-dereference crashes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In TrayWidget::showEvent you guard against a null m_handler but still unconditionally call window()->installEventFilter and window()->setMouseTracking; consider also checking that window() is non-null before using it to avoid potential crashes during early lifecycle.
- In SniTrayProtocolHandler::eventFilter, window()->windowHandle() is called before any null check on window(); if window() can be null in some cases, add a guard for window() itself before dereferencing it to prevent rare null-dereference crashes.

## Individual Comments

### Comment 1
<location path="plugins/application-tray/sniprotocolhandler.cpp" line_range="331" />
<code_context>

-                auto widget = static_cast<QWidget*>(parent());
-                auto plugin = Plugin::EmbedPlugin::get(widget->window()->windowHandle());
+                auto *win = window()->windowHandle();
+                if (!win)
+                    return false;
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against a potential nullptr from window() before calling windowHandle().

`window()` might return `nullptr` depending on when this is called. To avoid a potential null dereference, first store the result in a local variable, check it, and only then access `windowHandle()`, e.g. `auto *w = window(); if (!w) return false; auto *win = w->windowHandle();` to match the defensive pattern used for `win` below.
</issue_to_address>

### Comment 2
<location path="plugins/application-tray/traywidget.cpp" line_range="48-55" />
<code_context>
 void TrayWidget::showEvent(QShowEvent* event)
 {
     Q_UNUSED(event)
+    if (!m_handler)
+        return;
+
     m_handler->setWindow(window());
     window()->installEventFilter(m_handler);
     window()->setMouseTracking(true);
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid repeatedly installing the same event filter on each showEvent.

`window()->installEventFilter(m_handler)` runs on every show, and Qt will invoke the filter once per installation, causing duplicate handling per event. Please ensure the filter is installed only once (e.g., track an `installed` flag, detect `window()` changes, or move installation to a place that runs only once when the window becomes available).

Suggested implementation:

```cpp
void TrayWidget::showEvent(QShowEvent* event)
{
    Q_UNUSED(event)

    if (!m_handler)
        return;

    QWidget *w = window();
    if (!w)
        return;

    m_handler->setWindow(w);

    if (m_lastWindowWithFilter != w) {
        w->installEventFilter(m_handler);
        m_lastWindowWithFilter = w;
    }

    w->setMouseTracking(true);

```

1. In `traywidget.h`, add a member to track the last window where the filter was installed, e.g.:
   `QWidget *m_lastWindowWithFilter = nullptr;`
2. Ensure `m_lastWindowWithFilter` is initialized to `nullptr` in the constructor if in-class initialization is not used.
</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.

Comment thread plugins/application-tray/sniprotocolhandler.cpp
Comment thread plugins/application-tray/traywidget.cpp
@18202781743 18202781743 requested review from BLumia and mhduiy June 8, 2026 02:56
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@18202781743

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 2496163 into linuxdeepin:master Jun 8, 2026
9 of 11 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