Skip to content

Comments

fix: fix wild pointer issue when screen switching#1450

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
18202781743:crash
Feb 24, 2026
Merged

fix: fix wild pointer issue when screen switching#1450
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
18202781743:crash

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Feb 12, 2026

  1. Changed QScreen* to QPointer for m_dockScreen member
    variable
  2. This prevents dangling pointer issues when screens are removed or
    changed
  3. QPointer automatically becomes null when the QScreen object is
    deleted
  4. Added include for QPointer header file
  5. Updated copyright year range from 2023 to 2023-2026

Influence:

  1. Test dock behavior when switching between different screen
    configurations
  2. Verify dock doesn't crash when monitors are disconnected or
    reconnected
  3. Test multi-monitor setup with dock moving between screens
  4. Verify screen-related dock properties update correctly after screen
    changes
  5. Test system behavior during display configuration changes

fix: 修复屏幕切换时的野指针问题

  1. 将 m_dockScreen 成员变量从 QScreen* 改为 QPointer
  2. 防止屏幕被移除或更改时出现悬空指针问题
  3. QPointer 在 QScreen 对象被删除时会自动变为空指针
  4. 添加了 QPointer 头文件包含
  5. 更新了版权年份范围从 2023 到 2023-2026

Influence:

  1. 测试在不同屏幕配置切换时的停靠栏行为
  2. 验证断开或重新连接显示器时停靠栏不会崩溃
  3. 测试多显示器设置中停靠栏在屏幕间移动的情况
  4. 验证屏幕更改后停靠栏的屏幕相关属性正确更新
  5. 测试显示配置更改时的系统行为

Summary by Sourcery

Prevent dock panel crashes due to invalid screen references when displays are removed or changed.

Bug Fixes:

  • Guard dock panel screen tracking against dangling pointer issues during screen configuration changes.

Enhancements:

  • Use a safe Qt smart pointer type for the dock panel's associated screen reference to handle dynamic screen lifecycles more robustly.

Chores:

  • Update dock panel header copyright years to 2023–2026.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 12, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replaces a raw QScreen* member with a QPointer in the dock panel to prevent dangling pointer crashes when screens are removed or changed, adds the corresponding include, and updates the file copyright header years.

Sequence diagram for dock screen deletion with QPointer

sequenceDiagram
    participant DockPanel
    participant QScreen
    participant QtObjectSystem

    DockPanel->>QScreen: store pointer in m_dockScreen (QPointer<QScreen>)
    QtObjectSystem-->>QScreen: delete QScreen due to screen removal
    QScreen-->>QtObjectSystem: destroyed
    QtObjectSystem-->>DockPanel: m_dockScreen automatically set to null
    DockPanel->>DockPanel: check m_dockScreen before screen-dependent logic
    DockPanel-->>DockPanel: skip or update behavior if m_dockScreen is null
Loading

Class diagram for DockPanel using QPointer for dock screen

classDiagram
    class DockPanel {
        QPointer~QScreen~ m_dockScreen
    }

    class QScreen

    DockPanel --> QScreen : references
Loading

File-Level Changes

Change Details Files
Use a guarded Qt pointer for the dock’s associated screen to avoid wild/dangling pointer access when displays change.
  • Change the dock panel member that stores the associated screen from a raw QScreen* to QPointer so it is automatically nulled when the screen is destroyed.
  • Include the QPointer header so the guarded pointer type is available in the dock panel declaration.
  • Ensure any existing code that uses m_dockScreen will now safely handle the case where the screen has been deleted instead of reading a dangling pointer.
panels/dock/dockpanel.h
Refresh the copyright header range to include years up to 2026.
  • Update SPDX-FileCopyrightText line to use a 2023 - 2026 year range.
panels/dock/dockpanel.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 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.

@18202781743
Copy link
Contributor Author

image

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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. Changed QScreen* to QPointer<QScreen> for m_dockScreen member
variable
2. This prevents dangling pointer issues when screens are removed or
changed
3. QPointer automatically becomes null when the QScreen object is
deleted
4. Added include for QPointer header file
5. Updated copyright year range from 2023 to 2023-2026

Influence:
1. Test dock behavior when switching between different screen
configurations
2. Verify dock doesn't crash when monitors are disconnected or
reconnected
3. Test multi-monitor setup with dock moving between screens
4. Verify screen-related dock properties update correctly after screen
changes
5. Test system behavior during display configuration changes

fix: 修复屏幕切换时的野指针问题

1. 将 m_dockScreen 成员变量从 QScreen* 改为 QPointer<QScreen>
2. 防止屏幕被移除或更改时出现悬空指针问题
3. QPointer 在 QScreen 对象被删除时会自动变为空指针
4. 添加了 QPointer 头文件包含
5. 更新了版权年份范围从 2023 到 2023-2026

Influence:
1. 测试在不同屏幕配置切换时的停靠栏行为
2. 验证断开或重新连接显示器时停靠栏不会崩溃
3. 测试多显示器设置中停靠栏在屏幕间移动的情况
4. 验证屏幕更改后停靠栏的屏幕相关属性正确更新
5. 测试显示配置更改时的系统行为
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要是将一个普通的裸指针 QScreen *m_dockScreen 修改为 QPointer<QScreen> m_dockScreen,并更新了版权年份范围。这是一个非常积极且必要的改动。

以下是对该 diff 的详细审查和改进意见:

1. 代码逻辑与内存安全(核心改进)

  • 现状分析
    • 原代码使用 QScreen *m_dockScreen。在 Qt 中,QScreen 对象由 QGuiApplication 管理,其生命周期不由 DockPanel 控制。当系统显示配置发生变化(如拔掉显示器、更改分辨率)时,Qt 可能会销毁旧的 QScreen 对象并创建新的。
    • 如果 DockPanel 持有旧屏幕的裸指针,且未能在 QScreen 销毁时及时将其置空,后续访问 m_dockScreen 将导致 悬空指针,进而引发程序崩溃。
  • 改进评价
    • 使用 QPointer 是处理此类问题的标准 Qt 惯用法。QPointer 是一个弱引用指针,当其引用的对象被销毁时,它会自动置为 nullptr
    • 优点:极大地提高了代码的健壮性,防止了因屏幕热插拔导致的野指针崩溃。
    • 建议:代码中所有使用 m_dockScreen 的地方,在使用前应检查其是否为 nullptr。虽然 QPointer 保证了置空,但业务逻辑需要处理屏幕为空的情况(例如,Dock 失去了显示目标)。

2. 代码质量

  • 头文件包含
    • 新增 #include <QPointer> 是必要的,因为 QPointer 定义在此头文件中。这符合规范。
  • 版权声明
    • 将年份从 2023 更新为 2023 - 2026 是维护版权声明的常规操作,表明该项目预计维护至 2026 年,符合开源规范。

3. 代码性能

  • 影响分析
    • QPointer 相比于裸指针,在对象析构时会有微小的开销(需要从 QObject 的内部列表中移除监听),但在 DockPanel 这种 UI 组件的生命周期和对象数量级下,这种开销完全可以忽略不计。
    • 结论:性能影响极小,换取的安全性收益巨大。

4. 代码安全

  • 安全性提升
    • 如第 1 点所述,这是典型的防御性编程。它消除了潜在的 Use-After-Free(释放后重用)漏洞风险。

5. 进一步改进建议

虽然这个改动本身很好,但在实际使用该成员变量时,建议遵循以下最佳实践以确保万无一失:

  1. 访问前检查
    在任何使用 m_dockScreen 的函数中,务必进行判空处理。

    // 示例
    if (m_dockScreen) {
        // 安全使用
        QRect geometry = m_dockScreen->geometry();
    } else {
        // 处理异常情况,例如寻找主屏幕或隐藏 Dock
        qWarning() << "Current dock screen is null, handling fallback...";
    }
  2. 信号槽连接
    确保类中监听了 QGuiApplication::screenAddedQScreen::destroyed 信号,以便在屏幕移除或销毁时,能够逻辑上将 DockPanel 迁移到新的有效屏幕上,而不仅仅是依赖 QPointer 防止崩溃。

  3. 一致性检查
    建议检查 DockPanel 类中是否还有其他指向 QObject 派生类(如 QWidget, QWindow 等)的成员变量,它们的生命周期如果也不由 DockPanel 控制,同样应该考虑使用 QPointer

总结

这是一个高质量的代码改动,主要解决了潜在的内存安全问题(悬空指针),符合 Qt 编程规范,对性能无负面影响。建议合并。

@18202781743
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Feb 24, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 0a29a83 into linuxdeepin:master Feb 24, 2026
8 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