Skip to content

fix(editor): harden StartManager teardown and singleton lifetime - #534

Merged
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
pengfeixx:md-stability-fixes
Aug 20, 2026
Merged

fix(editor): harden StartManager teardown and singleton lifetime#534
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
pengfeixx:md-stability-fixes

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Null static instance pointers in Settings and StartManager destructors so instance() never returns a dangling pointer after deleteLater in tests; give context to timer singleShots to skip firing on destroyed receivers; guard the tab tear-off animation with QPointer since the source EditWrapper may be closed during the 200ms animation (UAF); fall back to window size when the drag pixmap is null.

Settings 与 StartManager 析构时置空静态指针,避免测试 deleteLater 后 instance() 返回悬空指针;延时 singleShot 补 context 对象,接收者 已销毁则不触发;标签撕出动画期间源 EditWrapper 可能被关闭,以
QPointer 防悬空(UAF 根源);拖拽 pixmap 为空时回退窗口尺寸。

Log: 修复标签撕出动画 UAF 与单例悬空问题
PMS: TASK-393979
Influence: 消除全量测试套件随机段错误与关标签时的偶发崩溃。

Summary by Sourcery

Harden editor teardown and tab tear-off handling to eliminate use-after-free crashes and invalid singleton access.

Bug Fixes:

  • Prevent dangling singleton pointers after Settings or StartManager instances are destroyed.
  • Prevent delayed callbacks from running after their owning windows or StartManager have been destroyed.
  • Prevent tab tear-off crashes when the source editor is closed during the animation.
  • Handle missing drag pixmaps safely when creating a window from a tab wrapper.

Null static instance pointers in Settings and StartManager destructors
so instance() never returns a dangling pointer after deleteLater in
tests; give context to timer singleShots to skip firing on destroyed
receivers; guard the tab tear-off animation with QPointer since the
source EditWrapper may be closed during the 200ms animation (UAF); fall
back to window size when the drag pixmap is null.

Settings 与 StartManager 析构时置空静态指针,避免测试 deleteLater
后 instance() 返回悬空指针;延时 singleShot 补 context 对象,接收者
已销毁则不触发;标签撕出动画期间源 EditWrapper 可能被关闭,以
QPointer 防悬空(UAF 根源);拖拽 pixmap 为空时回退窗口尺寸。

Log: 修复标签撕出动画 UAF 与单例悬空问题
PMS: TASK-393979
Influence: 消除全量测试套件随机段错误与关标签时的偶发崩溃。
@sourcery-ai

sourcery-ai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Reviewer's Guide

Hardens singleton lifetime management and deferred operations in StartManager and Settings to avoid dangling pointers and UAFs, and makes the tab tear‑off animation and drag pixmap logic robust against destroyed objects and null pixmaps.

Sequence diagram for context-bound QTimer singleShot calls

sequenceDiagram
    participant StartManager
    participant Window
    participant QTimer

    StartManager->>Window: createWindow(true)
    Window->>Window: showCenterWindow(true)
    StartManager->>QTimer: singleShot(50, Window, lambda)
    rect rgb(230,230,230)
    note over Window,QTimer: During delay
    Window--xWindow: destroyed
    end
    QTimer-->>Window: [lambda not invoked, context destroyed]


    StartManager->>QTimer: singleShot(1000, StartManager, lambda)
    rect rgb(230,230,230)
    note over StartManager,QTimer: During delay
    StartManager--xStartManager: deleteLater()
    end
    QTimer-->>StartManager: [lambda not invoked, context destroyed]
Loading

Sequence diagram for tab tear-off animation guarded by QPointer

sequenceDiagram
    participant StartManager
    participant QParallelAnimationGroup as AnimationGroup
    participant EditWrapper as Buffer
    participant QPointer as BufferGuard
    participant Window as PWindow

    StartManager->>BufferGuard: QPointer<EditWrapper> bufferGuard(Buffer)
    StartManager->>AnimationGroup: new QParallelAnimationGroup
    StartManager->>AnimationGroup: connect(finished, lambda)

    rect rgb(230,230,230)
    note over Buffer,AnimationGroup: During ~200ms animation
    Buffer--xBuffer: destroyed
    BufferGuard->>BufferGuard: isNull() == true
    end

    AnimationGroup-->>StartManager: finished()
    StartManager->>BufferGuard: isNull()
    alt bufferGuard.isNull()
        StartManager-->>StartManager: drop tear-off
    else bufferGuard not null
        StartManager->>PWindow: show()
        StartManager->>PWindow: showCenterWindow(false)
        StartManager->>PWindow: addTabWithWrapper(Buffer, filePath, qstrTruePath, tabName)
    end
Loading

File-Level Changes

Change Details Files
Ensure singleton static pointers in Settings and StartManager are cleared on destruction to avoid instance() returning dangling pointers.
  • Add StartManager destructor that nulls the static m_instance pointer when the singleton is destroyed.
  • Update Settings destructor to null the static s_pSetting pointer when that singleton is destroyed before cleaning up the backend.
src/startmanager.cpp
src/common/settings.cpp
src/startmanager.h
Guard delayed QTimer::singleShot callbacks with QObject context so they do not fire on destroyed receivers.
  • Change delayed file-open singleShot to use the owning Window as the context object.
  • Change slotCloseWindow’s delayed quit/trim singleShot to use StartManager (this) as the context object.
src/startmanager.cpp
Make tab tear-off animation and drag pixmap handling robust against null pixmaps and deleted EditWrapper instances to prevent UAF crashes.
  • Fallback to the window’s rectangle size when the drag pixmap is null while computing the animation start rectangle.
  • Wrap the EditWrapper buffer in QPointer during the animation, and on animation finish skip window showing/tab creation if the buffer was destroyed, otherwise proceed with showing the window and adding the tab.
src/startmanager.cpp
src/startmanager.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

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

  • In createWindowFromWrapper, consider parenting QParallelAnimationGroup (and possibly the QPropertyAnimations) to an appropriate QObject (e.g., pWindow or this) instead of relying solely on deleteLater in the finished handler, to avoid leaks if the animation is interrupted or never finishes.
  • The added qDebug logging in the StartManager and Settings destructors might be noisy in normal application shutdown; consider using a categorized logging macro or limiting these messages to debug/test builds if they are primarily for tracking singleton teardown issues.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In createWindowFromWrapper, consider parenting QParallelAnimationGroup (and possibly the QPropertyAnimations) to an appropriate QObject (e.g., pWindow or this) instead of relying solely on deleteLater in the finished handler, to avoid leaks if the animation is interrupted or never finishes.
- The added qDebug logging in the StartManager and Settings destructors might be noisy in normal application shutdown; consider using a categorized logging macro or limiting these messages to debug/test builds if they are primarily for tracking singleton teardown issues.

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.

Null static instance pointers in Settings and StartManager destructors
so instance() never returns a dangling pointer after deleteLater in
tests; give context to timer singleShots to skip firing on destroyed
receivers; guard the tab tear-off animation with QPointer since the
source EditWrapper may be closed during the 200ms animation (UAF); fall
back to window size when the drag pixmap is null.

Settings 与 StartManager 析构时置空静态指针,避免测试 deleteLater
后 instance() 返回悬空指针;延时 singleShot 补 context 对象,接收者
已销毁则不触发;标签撕出动画期间源 EditWrapper 可能被关闭,以
QPointer 防悬空(UAF 根源);拖拽 pixmap 为空时回退窗口尺寸。

Log: 修复标签撕出动画 UAF 与单例悬空问题
PMS: TASK-393979
Influence: 消除全量测试套件随机段错误与关标签时的偶发崩溃。
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

★ 总体评分:95分

■ 【总体评价】

代码修复了多处单例析构和异步回调导致的悬空指针及空指针解引用问题,显著提升了内存安全性
逻辑正确且有效消除了UAF崩溃风险,代码质量高,无安全漏洞

■ 【详细分析】

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

代码修复了 StartManagerSettings 析构函数中未置空单例指针的问题,并在 QTimer::singleShot 中绑定了 context 对象,正确处理了异步回调时的对象生命周期。同时增加了对 Tabbar::sm_pDragPixmap 的空指针检查,避免了空指针解引用。
潜在问题:无
建议:无

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

代码中新增了详细的注释说明修改原因,如防悬空、回退尺寸等,符合规范。引入了 QPointer 进行智能指针保护,代码结构清晰。
潜在问题:版权年份更新为 2026 年,可能存在笔误。
建议:确认版权年份修改是否为预期操作。

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

修改未引入额外的计算开销或资源消耗,QPointer 和空指针检查的开销可忽略不计。
潜在问题:无
建议:无

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 4 个,持平 0 个
本次代码修复了多个内存安全漏洞,消除了UAF和空指针解引用的风险,未引入新的安全问题。

  • 安全漏洞1(历史已修复):[UAF] 在 [StartManager::~StartManager] 中,单例对象析构后未置空,导致后续调用 instance() 返回悬空指针 ——非常重要

  • 安全漏洞2(历史已修复):[UAF] 在 [StartManager::openFilesInTab / slotCloseWindow] 中,QTimer::singleShot 未绑定 context,对象销毁后触发悬空调用 ——非常重要

  • 安全漏洞3(历史已修复):[UAF] 在 [StartManager::createWindowFromWrapper] 中,动画期间源标签页被销毁导致使用野指针 ——非常重要

  • 安全漏洞4(历史已修复):[空指针解引用] 在 [StartManager::createWindowFromWrapper] 中,Tabbar::sm_pDragPixmap 可能为空导致崩溃 ——非常重要

  • 建议:继续保持对对象生命周期的严格管理,建议在项目级别推广 QPointer 和 context 绑定的使用规范。

■ 【改进建议代码示例】

// 当前代码已具备良好的安全性和健壮性,无需进一步修改。
// 仅作为示例展示保持现状即可。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

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

@pengfeixx

Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot
deepin-bot Bot merged commit 783d949 into linuxdeepin:master Aug 20, 2026
20 checks passed
@pengfeixx
pengfeixx deleted the md-stability-fixes branch August 20, 2026 03:50
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