Skip to content

fix: replace raw pointers with QPointer to prevent wild pointers#1494

Open
18202781743 wants to merge 1 commit intolinuxdeepin:masterfrom
18202781743:master
Open

fix: replace raw pointers with QPointer to prevent wild pointers#1494
18202781743 wants to merge 1 commit intolinuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Mar 10, 2026

Changed raw QWaylandSurface pointers to QPointer smart pointers in
PluginManagerExtensionPrivate and PluginManagerExtension classes to
prevent wild pointer issues. This ensures automatic nullification when
the referenced QWaylandSurface objects are destroyed, preventing crashes
and undefined behavior.

The raw pointers were vulnerable to becoming wild pointers if the
QWaylandSurface objects were deleted while still being referenced. Using
QPointer provides safe weak referencing with automatic null checks,
improving memory safety and stability.

Influence:

  1. Test plugin management functionality with dynamic surface creation
    and destruction
  2. Verify no crashes occur when surfaces are removed while being
    referenced
  3. Test docking panel behavior with multiple plugin instances
  4. Validate memory safety during compositor operations
  5. Check for any regression in plugin loading/unloading

fix: 使用 QPointer 替换原始指针以防止野指针问题

将 PluginManagerExtensionPrivate 和 PluginManagerExtension 类中的原始
QWaylandSurface 指针替换为 QPointer 智能指针,以防止野指针问题。这确保当
引用的 QWaylandSurface 对象被销毁时自动置空,防止崩溃和未定义行为。

原始指针在引用的 QWaylandSurface 对象被删除时容易变成野指针。使用
QPointer 提供安全的弱引用和自动空值检查,提高了内存安全性和稳定性。

#0  0x00007f906aeedae6 in QList<QWaylandView*>::size (this=<optimized out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:399
#1  QtPrivate::indexOf<QWaylandView*, QWaylandView*> (from=<optimized out>, u=<optimized out>, vector=<optimized out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:933
#2  QListSpecialMethodsBase<QWaylandView*>::indexOf<QWaylandView*> (from=<optimized out>, t=<optimized out>, this=<optimized out>, this=<optimized out>, t=<optimized out>, from=<optimized out>)
    at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:966
#3  QListSpecialMethodsBase<QWaylandView*>::contains<QWaylandView*> (t=<optimized out>, this=<optimized out>, this=<optimized out>, t=<optimized out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:48
#4  QWaylandSurfacePrivate::refView (this=0x18, view=view@entry=0x55879c897370) at ./src/compositor/compositor_api/qwaylandsurface.cpp:1014
#5  0x00007f906aef1403 in QWaylandViewPrivate::setSurface (this=0x55879c897390, newSurface=<optimized out>) at ./src/compositor/compositor_api/qwaylandview.cpp:121
#5  0x00007f906aef1403 in QWaylandViewPrivate::setSurface (this=0x55879c897390, newSurface=<optimized out>) at ./src/compositor/compositor_api/qwaylandview.cpp:121
de/x86_64-linux-gnu/qt6/QtCore/qlist.h:48

#6  0x00007f906aef1498 in QWaylandView::setSurface (this=0x55879c897370, newSurface=<optimized out>) at ./src/compositor/compositor_api/qwaylandview.cpp:141
#7  0x00007f906af47b06 in QWaylandQuickItem::setSurface (this=0x55879c897010, surface=0x55879c55e060) at ./src/compositor/compositor_api/qwaylandquickitem.cpp:560
#8  0x00007f908006733d in PluginManagerIntegration::PluginManagerIntegration (this=this@entry=0x55879c4745a0, item=item@entry=0x55879c897010) at ./panels/dock/pluginmanagerintegration.cpp:16
#9  0x00007f9080060d25 in PluginSurface::createIntegration (this=<optimized out>, item=0x55879c897010) at ./panels/dock/pluginmanagerextension.cpp:187
#10 0x00007f906af5074d in QWaylandQuickShellSurfaceItem::setShellSurface (this=0x55879c897010, shellSurface=0x55879c55cf10) at ./src/compositor/extensions/qwaylandquickshellsurfaceitem.cpp:117

Influence:

  1. 测试动态表面创建和销毁时的插件管理功能
  2. 验证当表面被删除时仍被引用时不会发生崩溃
  3. 测试多个插件实例时的停靠面板行为
  4. 验证合成器操作期间的内存安全性
  5. 检查插件加载/卸载功能是否有回归问题

Summary by Sourcery

Replace raw QWaylandSurface pointers with safer weak pointers in dock plugin manager surfaces to avoid dangling references when surfaces are destroyed.

Bug Fixes:

  • Prevent crashes and undefined behavior by changing PluginSurface and PluginPopup to hold QWaylandSurface via QPointer instead of raw pointers.

Enhancements:

  • Include QPointer in the dock plugin manager extension header to support safer surface pointer management.

@sourcery-ai
Copy link

sourcery-ai bot commented Mar 10, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replaces raw QWaylandSurface* members with QPointer in plugin manager docking surfaces to avoid wild pointer crashes when Wayland surfaces are destroyed, and adjusts includes accordingly.

Sequence diagram for safe handling of destroyed QWaylandSurface via QPointer

sequenceDiagram
    participant Compositor as QWaylandCompositor
    participant Surface as QWaylandSurface
    participant PluginSurface

    Compositor->>Surface: create()
    PluginSurface->>Surface: store_in_m_surface(QPointer)

    Compositor-->>Surface: destroy()
    Surface-->>PluginSurface: m_surface_auto_nulls

    PluginSurface->>PluginSurface: handleEvent()
    PluginSurface->>PluginSurface: if m_surface == nullptr
    PluginSurface-->>PluginSurface: skip_surface_access
    PluginSurface-->>Compositor: no_crash_no_UB
Loading

Class diagram for PluginSurface and PluginPopup QPointer migration

classDiagram
    class PluginManager

    class QWaylandSurface

    class PluginSurface {
        - PluginManager* m_manager
        - QPointer~QWaylandSurface~ m_surface
        - QString m_itemKey
        - QString m_pluginId
    }

    class PluginPopup {
        - PluginManager* m_manager
        - QPointer~QWaylandSurface~ m_surface
        - QString m_itemKey
        - QString m_pluginId
    }

    PluginSurface --> PluginManager : uses
    PluginSurface --> QWaylandSurface : weak_reference

    PluginPopup --> PluginManager : uses
    PluginPopup --> QWaylandSurface : weak_reference
Loading

File-Level Changes

Change Details Files
Use QPointer for stored QWaylandSurface references in plugin surface types to avoid dangling pointers when surfaces are destroyed.
  • Replaced raw QWaylandSurface* member in PluginSurface with QPointer.
  • Replaced raw QWaylandSurface* member in PluginPopup with QPointer.
  • Updated pluginmanagerextension_p.h includes to add and reorder QtWaylandCompositor includes.
panels/dock/pluginmanagerextension_p.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

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 left some high level feedback:

  • Now that m_surface is a QPointer<QWaylandSurface>, review all its usage sites to ensure they either guard against nullptr or handle the null case gracefully rather than assuming it is always valid.
  • If there are code paths that conceptually require a valid surface (e.g., logic that cannot proceed without it), consider adding explicit checks or assertions at those entry points to fail fast instead of relying on later QPointer null checks.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Now that `m_surface` is a `QPointer<QWaylandSurface>`, review all its usage sites to ensure they either guard against `nullptr` or handle the null case gracefully rather than assuming it is always valid.
- If there are code paths that conceptually require a valid surface (e.g., logic that cannot proceed without it), consider adding explicit checks or assertions at those entry points to fail fast instead of relying on later `QPointer` null checks.

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

[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

Changed raw QWaylandSurface pointers to QPointer smart pointers in
PluginManagerExtensionPrivate and PluginManagerExtension classes to
prevent wild pointer issues. This ensures automatic nullification when
the referenced QWaylandSurface objects are destroyed, preventing crashes
and undefined behavior.

The raw pointers were vulnerable to becoming wild pointers if the
QWaylandSurface objects were deleted while still being referenced. Using
QPointer provides safe weak referencing with automatic null checks,
improving memory safety and stability.

Influence:
1. Test plugin management functionality with dynamic surface creation
and destruction
2. Verify no crashes occur when surfaces are removed while being
referenced
3. Test docking panel behavior with multiple plugin instances
4. Validate memory safety during compositor operations
5. Check for any regression in plugin loading/unloading

fix: 使用 QPointer 替换原始指针以防止野指针问题

将 PluginManagerExtensionPrivate 和 PluginManagerExtension 类中的原始
QWaylandSurface 指针替换为 QPointer 智能指针,以防止野指针问题。这确保当
引用的 QWaylandSurface 对象被销毁时自动置空,防止崩溃和未定义行为。

原始指针在引用的 QWaylandSurface 对象被删除时容易变成野指针。使用
QPointer 提供安全的弱引用和自动空值检查,提高了内存安全性和稳定性。

```
#0  0x00007f906aeedae6 in QList<QWaylandView*>::size (this=<optimized out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:399
#1  QtPrivate::indexOf<QWaylandView*, QWaylandView*> (from=<optimized out>, u=<optimized out>, vector=<optimized out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:933
#2  QListSpecialMethodsBase<QWaylandView*>::indexOf<QWaylandView*> (from=<optimized out>, t=<optimized out>, this=<optimized out>, this=<optimized out>, t=<optimized out>, from=<optimized out>)
    at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:966
#3  QListSpecialMethodsBase<QWaylandView*>::contains<QWaylandView*> (t=<optimized out>, this=<optimized out>, this=<optimized out>, t=<optimized out>) at /usr/include/x86_64-linux-gnu/qt6/QtCore/qlist.h:48
#4  QWaylandSurfacePrivate::refView (this=0x18, view=view@entry=0x55879c897370) at ./src/compositor/compositor_api/qwaylandsurface.cpp:1014
#5  0x00007f906aef1403 in QWaylandViewPrivate::setSurface (this=0x55879c897390, newSurface=<optimized out>) at ./src/compositor/compositor_api/qwaylandview.cpp:121
#5  0x00007f906aef1403 in QWaylandViewPrivate::setSurface (this=0x55879c897390, newSurface=<optimized out>) at ./src/compositor/compositor_api/qwaylandview.cpp:121
de/x86_64-linux-gnu/qt6/QtCore/qlist.h:48

#6  0x00007f906aef1498 in QWaylandView::setSurface (this=0x55879c897370, newSurface=<optimized out>) at ./src/compositor/compositor_api/qwaylandview.cpp:141
#7  0x00007f906af47b06 in QWaylandQuickItem::setSurface (this=0x55879c897010, surface=0x55879c55e060) at ./src/compositor/compositor_api/qwaylandquickitem.cpp:560
#8  0x00007f908006733d in PluginManagerIntegration::PluginManagerIntegration (this=this@entry=0x55879c4745a0, item=item@entry=0x55879c897010) at ./panels/dock/pluginmanagerintegration.cpp:16
#9  0x00007f9080060d25 in PluginSurface::createIntegration (this=<optimized out>, item=0x55879c897010) at ./panels/dock/pluginmanagerextension.cpp:187
#10 0x00007f906af5074d in QWaylandQuickShellSurfaceItem::setShellSurface (this=0x55879c897010, shellSurface=0x55879c55cf10) at ./src/compositor/extensions/qwaylandquickshellsurfaceitem.cpp:117

```

Influence:
1. 测试动态表面创建和销毁时的插件管理功能
2. 验证当表面被删除时仍被引用时不会发生崩溃
3. 测试多个插件实例时的停靠面板行为
4. 验证合成器操作期间的内存安全性
5. 检查插件加载/卸载功能是否有回归问题
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改主要涉及将 QWaylandSurface* 类型的指针成员变量替换为 QPointer<QWaylandSurface>。这是一个很好的改进,以下是针对语法逻辑、代码质量、代码性能和代码安全的详细审查意见:

1. 语法逻辑

  • 结论:通过
  • 分析QPointer 是 Qt 提供的模板类,用于继承自 QObject 的对象。QWaylandSurface 继承自 QObject,因此语法上是完全合法的。代码的包含顺序调整(#include <QPointer>)也符合逻辑,确保了依赖关系的正确性。

2. 代码质量

  • 结论:优秀
  • 分析
    • 自动置空QPointer 的主要特性是当其指向的对象被销毁(deleted)时,它会自动变为 nullptr。这极大地提高了代码的健壮性,防止了"悬空指针"(Dangling Pointer)的产生。
    • 头文件整理:Diff 中将头文件按字母顺序或逻辑顺序进行了重新排列(例如将 <QPointer> 放在前面,相关的 Wayland 头文件归类),这有助于代码的可读性和长期维护。

3. 代码性能

  • 结论:影响极小,可忽略
  • 分析
    • QPointer 内部维护了一个连接到目标对象 destroyed 信号的连接。相比于裸指针,这增加了极小的内存开销(存储连接信息)和对象销毁时的微小开销(发出信号并断开连接)。
    • 访问开销:对 QPointer 的解引用操作(->*)与裸指针几乎相同,没有显著的性能损失。
    • 权衡:在 GUI 事件驱动的程序中,对象生命周期复杂,为了防止崩溃而牺牲的这点性能是非常值得的。

4. 代码安全

  • 结论:显著提升
  • 分析
    • 防止悬空指针访问:这是本次修改的核心目的。在 Wayland Compositor 的架构中,QWaylandSurface 的生命周期通常由客户端或 Compositor 核心管理,可能会在 PluginSurfacePluginPopup 对象仍然存在时被意外销毁。
    • 使用前检查:修改后,代码中在使用 m_surface 之前,可以更安全地进行判空检查(if (m_surface) { ... })。如果是裸指针,即使判空了,对象也可能在判空之后、使用之前被异步销毁,导致崩溃。QPointer 保证了如果对象被销毁,指针会立即失效。

改进建议与注意事项

虽然这次修改非常好,但在后续使用 QPointer 时,建议注意以下几点以确保万无一失:

  1. 判空检查
    确保在所有访问 m_surface 成员的地方都添加了判空逻辑,或者逻辑上能保证 m_surface 一定存在。

    // 推荐写法
    if (m_surface) {
        m_surface->requestSize(size);
    }
  2. 避免隐式布尔转换陷阱
    虽然 QPointer 支持隐式转换为 bool,但在某些复杂表达式中,为了代码清晰,建议显式判断。

    // 明确的写法
    if (m_surface.isNull()) { ... }
    //
    if (m_surface != nullptr) { ... }
  3. 数据成员初始化
    QPointer 默认初始化为 nullptr,这比未初始化的裸指针更安全。请确保构造函数中没有遗漏对其他成员的初始化,保持一致性。

  4. 包含头文件
    Diff 中已经正确添加了 #include <QPointer>,请确保该文件没有被后续的清理操作误删。

总结

这是一个高质量的代码改进。将原始指针替换为 QPointer 是 Qt 编程中管理 QObject 派生类生命周期的最佳实践之一,有效地解决了潜在的内存安全问题,对性能的影响微乎其微。建议采纳此修改。

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