Skip to content

fix: avoid StartManager singleton creation in destructor - #637

Merged
pengfeixx merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:agent/bug/5c69cfaa011f
Oct 9, 2026
Merged

pengfeixx merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:agent/bug/5c69cfaa011f

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

fix: avoid StartManager singleton creation in destructor

  1. Root cause: ~EditorApplication() called StartManager::instance()
    which lazily creates a new singleton when m_instance is null
  2. Fix: add StartManager::instanceOrNull() that returns m_instance
    without creating, use it in destructor for null-check and delete
  3. Impact: second process exiting after D-Bus file forwarding no
    longer triggers spurious StartManager creation/destruction crash

Log: Fixed crash when closing editor after opening text file from desktop

Influence:

  1. Test opening a text file by double-clicking on desktop
  2. Test launching a second instance that forwards files via D-Bus
  3. Verify normal startup and shutdown of the editor

fix: 修复析构函数中单例懒加载创建导致的崩溃

  1. 根因:~EditorApplication() 调用 StartManager::instance(),当
    m_instance 为 null 时会懒加载创建新实例
  2. 方案:新增 StartManager::instanceOrNull() 不创建实例的只读
    访问器,析构函数改用该方法判空和删除
  3. 影响:D-Bus 转发文件后退出的第二进程不再意外创建/销毁
    StartManager 导致 SIGSEGV

Log: 修复双击桌面文本文档时文本编辑器崩溃的问题

Influence:

  1. 测试双击桌面文本文档打开编辑器
  2. 测试第二进程通过 D-Bus 转发文件后退出
  3. 验证编辑器正常启动和关闭流程

PMS: BUG-378901

Summary by Sourcery

Prevent StartManager from being created during editor destruction and ensure its lifecycle is managed explicitly during normal application startup and shutdown.

Bug Fixes:

  • Prevent editor shutdown crashes caused by unintentionally creating the StartManager singleton while a D-Bus forwarding process exits.

Enhancements:

  • Make StartManager creation explicit in the main startup path and transfer its cleanup to main after the event loop ends.
  • Change singleton access so callers can query the existing instance without triggering lazy initialization.

Tests:

  • Update application and StartManager unit tests to verify non-creating singleton access and the revised lifecycle.

@sourcery-ai

sourcery-ai Bot commented Oct 9, 2026 •

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

Reviewer's Guide

The PR fixes shutdown crashes in secondary editor processes by preventing EditorApplication’s destructor from lazily creating a StartManager singleton when none exists, while preserving deletion of an existing instance.

Sequence diagram for safe editor shutdown

sequenceDiagram
    participant EditorApplication
    participant StartManager

    EditorApplication->>StartManager: instanceOrNull()
    StartManager-->>EditorApplication: m_instance
    alt existing instance
        EditorApplication->>StartManager: delete
        StartManager-->>EditorApplication: destructor completes
    else no instance
        EditorApplication->>EditorApplication: skip deletion
    end
Loading

File-Level Changes

Change Details Files
Add a non-creating singleton accessor and use it during application teardown to avoid recreating StartManager.
  • Expose instanceOrNull() to return the existing singleton pointer without lazy initialization.
  • Use the non-creating accessor for both the null check and deletion in EditorApplication’s destructor.
src/startmanager.h
src/startmanager.cpp
src/editorapplication.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 ✨

@pengfeixx
pengfeixx force-pushed the agent/bug/5c69cfaa011f branch from f098b3a to 76e9fd4 Compare October 9, 2026 06:02
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 97 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交修复了析构函数中单例懒加载创建导致的崩溃问题,代码变更聚焦、逻辑正确,无安全漏洞。仅存在轻微的代码质量和性能优化建议。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 语法正确,逻辑清晰,边界处理完善


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. src/editorapplication.cpp:50 - 析构函数中两次调用 StartManager::instanceOrNull()(一次判空、一次 delete),建议先存储到局部变量再判断,减少冗余调用

建议: 建议在 instanceOrNull() 方法上方添加简短注释,说明与 instance() 的区别(只读访问,不创建实例);析构函数中可将 instanceOrNull() 结果存储到局部变量避免重复调用


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. src/editorapplication.cpp:50 - 析构函数中两次调用 instanceOrNull() 可优化为一次调用并存储到局部变量,避免冗余函数调用

建议: 将 instanceOrNull() 结果存储到局部变量,减少一次函数调用


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: 无安全漏洞,安全合规


💡 改进建议代码示例

// 析构函数优化:避免重复调用 instanceOrNull()
EditorApplication::~EditorApplication()
{
    qDebug() << "Enter EditorApplication destructor";
    StartManager *mgr = StartManager::instanceOrNull();
    if (mgr) {
        qDebug() << "Deleting StartManager instance";
        delete mgr;
    } else {
        qDebug() << "StartManager instance is already null";
    }
    qDebug() << "Exit EditorApplication destructor";
}

本报告由 AI 代码审查工具自动生成

1. Root cause: StartManager::instance() lazily creates the singleton,
   and ~EditorApplication() called it during teardown; a forwarding
   (second) process that never created StartManager would instantiate
   a full instance (login1 inhibit D-Bus call, QTimer, Iflytek AI
   probe QtConcurrent task) and destroy it right away, leaving the
   worker thread logging during process teardown and crashing in
   libdtk6log (SIGSEGV)
2. Fix:
   - instance() is now a pure non-creating accessor
   - creation is explicit via create(), called exactly once in main()
   - the first (service-owning) process releases the singleton in
     main() right after the event loop ends; ~EditorApplication()
     no longer touches StartManager at all
3. Impact: the forwarding process exit path never instantiates
   StartManager, removing the teardown crash; accidental singleton
   creation is now impossible for all callers by construction

Log: Fixed crash when opening a text file from desktop with the editor already running

Influence:
1. Test opening a text file by double-clicking on desktop with editor open
2. Test launching a second instance that forwards files via D-Bus
3. Test single-window startup and normal exit of the editor
4. Run StartManager / EditorApplication / Window / Controls unit tests
   (updated for create() / non-creating instance() semantics)

fix: 单例生命周期显式化,修复退出崩溃

1. 根因: StartManager::instance() 是懒创建访问器,~EditorApplication()
   在退出期调用它;D-Bus 转发进程从未创建过单例,析构时意外实例化完整
   StartManager(login1 inhibit 同步 D-Bus 调用、QTimer、Iflytek AI 探测
   QtConcurrent 任务)并立即销毁,工作线程在进程 teardown 期间打日志,
   于 libdtk6log 中 SIGSEGV
2. 方案:
   - instance() 改为纯查询访问器,不再有创建副作用
   - 创建显式化为 create(),仅在 main() 中调用一次
   - 首进程在事件循环结束后于 main() 中显式释放单例,
     ~EditorApplication() 不再触碰 StartManager
3. 影响: 转发进程退出路径不再实例化 StartManager,消除崩溃;对所有
   调用方而言,意外创建单例在构造上已不可能

Log: 修复已打开编辑器时双击桌面文本文档产生的崩溃

Influence:
1. 测试已打开编辑器时双击桌面文本文档
2. 测试第二进程通过 D-Bus 转发文件后退出
3. 测试单窗口启动与正常退出
4. 运行 StartManager / EditorApplication / Window / Controls 单元测试
   (已随 create() / 非创建 instance() 语义同步更新)

PMS: BUG-378901
@pengfeixx
pengfeixx force-pushed the agent/bug/5c69cfaa011f branch from 76e9fd4 to 8d0e4a3 Compare October 9, 2026 07:49
@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
pengfeixx merged commit 4022040 into linuxdeepin:master Oct 9, 2026
16 of 17 checks passed
@pengfeixx
pengfeixx deleted the agent/bug/5c69cfaa011f branch October 9, 2026 08:55
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