Skip to content

fix: fix crash on close after editing text in zip archive - #640

Closed
pengfeixx wants to merge 1 commit into
linuxdeepin:masterfrom
pengfeixx:agent/pms-bug-bot/925d0bc885bd
Closed

pengfeixx wants to merge 1 commit into
linuxdeepin:masterfrom
pengfeixx:agent/pms-bug-bot/925d0bc885bd

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

fix: fix crash on close after editing text in zip archive

  1. Root cause: ~EditWrapper() destructor deleted m_pTextEdit before
    m_pWaringNotices, leaving DMessageManager with a dangling pointer
    to the already-freed m_pTextEdit (use-after-free, CWE-416)
  2. Fix: move m_pWaringNotices cleanup before m_pTextEdit deletion so
    DMessageManager processes message removal while m_pTextEdit is
    still valid; also un-comment and restore the previously disabled
    m_pWaringNotices cleanup block (was commented out for bug 78042)
  3. Impact: destructor cleanup order change only; no behavior change
    for normal edit/close workflows

Log: Fix crash when closing text editor after editing a file in a zip archive

Influence:

  1. Test opening a text file from a zip archive, editing and saving,
    then closing the tab - verify no crash
  2. Test file reload notification dialog appears and dismisses correctly
  3. Verify normal file open/edit/close workflow has no regression

fix: 修复压缩包中编辑文本文档关闭后崩溃问题

  1. 根因:~EditWrapper() 析构函数中 m_pTextEdit 的 delete 早于
    m_pWaringNotices,导致 DMessageManager 持有已释放的 m_pTextEdit
    悬空指针,触发 use-after-free 崩溃(CWE-416)
  2. 方案:将 m_pWaringNotices 的清理移至 m_pTextEdit 删除之前,使
    DMessageManager 处理消息移除时 m_pTextEdit 仍然有效;同时恢复
    此前因 bug 78042 被注释掉的 m_pWaringNotices 清理代码块
  3. 影响:仅调整析构函数清理顺序,对正常编辑/关闭流程无行为变化

Log: 修复压缩包中打开文本文档编辑保存后关闭时文本编辑器崩溃的问题

Influence:

  1. 测试从 zip 压缩包中打开文本文档,编辑保存后关闭标签页,验证不崩溃
  2. 测试文件变更重载提示弹框正常显示和关闭
  3. 验证正常文件打开/编辑/关闭流程无回归

PMS: BUG-378881

Summary by Sourcery

Bug Fixes:

  • Prevent crashes when closing an edited text file opened from a ZIP archive by ensuring warning-notice cleanup occurs while the text editor remains valid.

1. Root cause: ~EditWrapper() destructor deleted m_pTextEdit before
   m_pWaringNotices, leaving DMessageManager with a dangling pointer
   to the already-freed m_pTextEdit (use-after-free, CWE-416)
2. Fix: move m_pWaringNotices cleanup before m_pTextEdit deletion so
   DMessageManager processes message removal while m_pTextEdit is
   still valid; also un-comment and restore the previously disabled
   m_pWaringNotices cleanup block (was commented out for bug 78042)
3. Impact: destructor cleanup order change only; no behavior change
   for normal edit/close workflows

Log: Fix crash when closing text editor after editing a file in a zip archive

Influence:
1. Test opening a text file from a zip archive, editing and saving,
   then closing the tab - verify no crash
2. Test file reload notification dialog appears and dismisses correctly
3. Verify normal file open/edit/close workflow has no regression

fix: 修复压缩包中编辑文本文档关闭后崩溃问题

1. 根因:~EditWrapper() 析构函数中 m_pTextEdit 的 delete 早于
   m_pWaringNotices,导致 DMessageManager 持有已释放的 m_pTextEdit
   悬空指针,触发 use-after-free 崩溃(CWE-416)
2. 方案:将 m_pWaringNotices 的清理移至 m_pTextEdit 删除之前,使
   DMessageManager 处理消息移除时 m_pTextEdit 仍然有效;同时恢复
   此前因 bug 78042 被注释掉的 m_pWaringNotices 清理代码块
3. 影响:仅调整析构函数清理顺序,对正常编辑/关闭流程无行为变化

Log: 修复压缩包中打开文本文档编辑保存后关闭时文本编辑器崩溃的问题

Influence:
1. 测试从 zip 压缩包中打开文本文档,编辑保存后关闭标签页,验证不崩溃
2. 测试文件变更重载提示弹框正常显示和关闭
3. 验证正常文件打开/编辑/关闭流程无回归

PMS: BUG-378881
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

@sourcery-ai

sourcery-ai Bot commented Oct 9, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

The destructor now cleans up m_pWaringNotices before m_pTextEdit, eliminating the close-time use-after-free reported when editing files inside ZIP archives while preserving normal edit and close behavior.

Sequence diagram for safe EditWrapper destruction

sequenceDiagram
    participant EditWrapper
    participant DMessageManager
    participant WarningNotices
    participant TextEdit

    EditWrapper->>WarningNotices: disconnect(WarningNotices)
    EditWrapper->>WarningNotices: delete WarningNotices
    WarningNotices->>DMessageManager: remove messages while TextEdit is valid
    EditWrapper->>TextEdit: disconnect(TextEdit)
    EditWrapper->>TextEdit: delete TextEdit
Loading

File-Level Changes

Change Details Files
Reorders EditWrapper destruction to remove warning notices before destroying the text editor, preventing DMessageManager from accessing a dangling editor pointer.
  • Restores warning-notice cleanup that had been commented out.
  • Disconnects and deletes warning notices before disconnecting and deleting the text editor.
  • Clears both member pointers after deletion.
src/editor/editwrapper.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 ✨

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

AI 代码审查报告

项目: linuxdeepin/deepin-editor
PR: #640
标题: fix: fix crash on close after editing text in zip archive
作者: pengfeixx
分支: agent/pms-bug-bot/925d0bc885bd → master
审查时间: 2026-10-09 12:20:00
分析模式: 全量分析


总体评价

总分: 100分

代码审查通过。本次提交修复了在 zip 归档中编辑文本后关闭时的崩溃问题(bug 378881/78042),通过调整析构函数中 m_pWaringNotices 和 m_pTextEdit 的删除顺序,有效避免了 DMessageManager 持有悬空指针导致的 use-after-free 问题。代码修改精准,注释详尽,无安全漏洞。


四维度评分

维度 评分 权重 状态 评价
语法逻辑 25/25 25% ✓ 语法正确,逻辑清晰
代码质量 25/25 25% ✓ 代码结构清晰,注释完整
代码性能 20/20 20% ✓ 性能良好,资源使用合理
代码安全 30/30 30% ✓ 存在0个安全漏洞

漏洞统计

统计项 数量
当前漏洞总数 0
新增漏洞 0
修复漏洞 0
持平漏洞 0

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个


详细分析

维度1:语法逻辑(25分)✓

评价: 语法正确,逻辑清晰

分析内容:

修改文件: src/editor/editwrapper.cpp
修改函数: EditWrapper::~EditWrapper()(析构函数,第195-217行)

本次修改在析构函数中新增了 m_pWaringNotices 的清理代码,并将其放置在 m_pTextEdit 删除之前。代码语法完全正确:

  1. disconnect(m_pWaringNotices) — 断开信号槽连接,防止删除过程中信号触发
  2. delete m_pWaringNotices — 释放对象内存
  3. m_pWaringNotices = nullptr — 置空指针,防止二次释放

该清理模式与同文件中 m_pTextEdit(第198-203行)和 m_pBottomBar(第204-209行)的清理模式完全一致,符合 Qt 对象管理的标准做法。

空指针检查(if (m_pWaringNotices != nullptr))到位,边界处理完善。同时删除了原来被注释掉的旧代码块(第210-215行),清理了死代码。

无语法错误,无逻辑缺陷,边界处理完善。

维度2:代码质量(25分)✓

评价: 代码结构清晰,注释完整

分析内容:

  1. 注释完整性(5/5): 新增代码包含详尽的中文注释,说明了:

    • 修改原因:必须在 delete m_pTextEdit 之前清理 m_pWaringNotices
    • 问题根因:DMessageManager 持有 m_pTextEdit 悬空指针导致 use-after-free
    • 关联 bug:bug 378881 和 bug 78042
    • 修复方案:将删除顺序调整为先 m_pWaringNotices 后 m_pTextEdit
  2. 代码重复(5/5): 清理模式虽与 m_pTextEdit/m_pBottomBar 类似,但属于必要的对象生命周期管理,非重复代码。

  3. 结构合理性(5/5): 析构函数内对象删除顺序清晰合理:m_pWaringNotices → m_pTextEdit → m_pBottomBar,遵循了依赖关系的逆序释放原则。

  4. 调试信息清理(5/5): 删除了被注释的旧代码块,无残留调试代码。原有的 qDebug() 日志为项目既有风格,非本次新增。

无重复代码,可读性好,符合编码规范。

维度3:代码性能(20分)✓

评价: 性能良好,资源使用合理

分析内容:

  1. 析构函数中的 disconnect + delete + nullptr 操作都是必要的对象清理操作,无性能瓶颈
  2. 操作在对象析构时执行一次,不涉及循环或频繁调用
  3. 资源释放顺序正确,先释放依赖方(m_pWaringNotices)再释放被依赖方(m_pTextEdit),避免不必要的临时状态
  4. 无不必要的计算、拷贝或系统调用

无性能问题,算法复杂度合理,资源使用合理。

维度4:代码安全(30分)✓

存在0个安全漏洞

评价: 安全合规

分析内容:

本次修改实际上修复了一个内存安全漏洞(use-after-free):

  1. 原问题根因: 在原代码中,DMessageManager::instance()->sendMessage(self->m_pTextEdit, self->m_pWaringNotices)(第992、998、1136、1827行)将 m_pWaringNotices 附加到 m_pTextEdit 上。当析构函数先删除 m_pTextEdit 时,DMessageManager 仍持有对已删除 m_pTextEdit 的引用,后续处理消息移除时访问已释放内存,导致 use-after-free 崩溃。

  2. 修复方案: 将 m_pWaringNotices 的清理移至 m_pTextEdit 之前,确保 DMessageManager 在 m_pTextEdit 被销毁前就释放了对它的引用。

  3. 安全防护措施:

    • disconnect 操作防止了信号槽在删除过程中触发
    • nullptr 赋值防止了二次释放
    • 空指针检查避免了空指针解引用

无新增安全漏洞,无硬编码密钥,无敏感信息泄露,无注入风险。


修改文件清单

文件 变更类型 说明
src/editor/editwrapper.cpp 修改 析构函数中调整对象删除顺序,修复 use-after-free

改进建议

本次代码修改质量优秀,无需额外改进。以下为可选的后续优化建议:

  1. 建议: 考虑使用 QScopedPointer 或 std::unique_ptr 管理 m_pWaringNotices、m_pTextEdit、m_pBottomBar 的生命周期,利用 RAII 机制自动保证释放顺序,减少手动管理的心智负担。

  2. 建议: 可考虑在 m_pWaringNotices 清理时显式调用 DMessageManager::instance()->sendMessage 的逆操作(如果 API 支持),以确保 DMessageManager 内部状态完全清理。


审查结论

本次提交是一个高质量的 Bug 修复,精准地解决了 use-after-free 崩溃问题。代码修改与 commit message 描述的目的一致,注释详尽,逻辑清晰,无安全漏洞。建议合并。

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.

2 participants