Skip to content

fix(test): connect sheet destroyed signal to prevent singleton segfault - #376

Merged
pengfeixx merged 1 commit into
masterfrom
agent/agent/720bcd8dbac3
Sep 9, 2026
Merged

fix(test): connect sheet destroyed signal to prevent singleton segfault#376
pengfeixx merged 1 commit into
masterfrom
agent/agent/720bcd8dbac3

Conversation

@pengfeixx

@pengfeixx pengfeixx commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

问题

test-deepin-reader 在运行完最后一个用例 UT_ReaderImageThreadPoolManager.UT_addgetDocImageTask 后,进程退出阶段发生段错误(SIGSEGV,exit code 255)。

根因

UT_onTaskFinishedUT_addgetDocImageTask 两个用例创建了 DocSheet 对象并注册到 ReaderImageThreadPoolManager 单例中,但在删除 sheet 时未连接 destroyed 信号到 onDocProxyDestroyed 槽进行清理(同文件中 UT_setImageForDocSheet 有此连接且正常通过)。

sheet 被 delete 后,单例内部仍持有悬空指针,进程退出时单例析构访问这些悬空指针 → 段错误。

修复

UT_onTaskFinishedUT_addgetDocImageTask 中,于调用 onTaskFinished / addgetDocImageTask 之前添加:

QObject::connect(sheet, &QObject::destroyed, m_tester, &ReaderImageThreadPoolManager::onDocProxyDestroyed);

UT_setImageForDocSheet 保持一致,确保 sheet 销毁时单例正确清理内部引用。

验证

  • 全量 1120 个用例全部通过,exit code = 0(修复前 exit code = 255)
  • UT_ReaderImageThreadPoolManager 全部 9 个用例通过

变更文件

  • tests/sidebar/ut_readerimagethreadpoolmanager.cpp(+4 行)

Summary by Sourcery

Prevent dangling document-sheet references from causing singleton teardown segmentation faults in image thread pool manager tests.

Bug Fixes:

  • Prevent test-process crashes during shutdown by ensuring destroyed document sheets are removed from the image thread pool manager's tracking state.

Tests:

  • Update image thread pool manager tests to clean up sheet registrations when test objects are destroyed.

UT_onTaskFinished and UT_addgetDocImageTask registered DocSheet pointers
with the ReaderImageThreadPoolManager singleton but did not connect the
sheet's destroyed() signal to onDocProxyDestroyed for cleanup, unlike
UT_setImageForDocSheet which already had this connection.

When the tests deleted their DocSheet objects, dangling pointers remained
in the singleton's internal map. At program exit the singleton destructor
accessed these dangling pointers, causing a segmentation fault (exit 255).

Add the missing QObject::connect(sheet, &QObject::destroyed, m_tester,
&ReaderImageThreadPoolManager::onDocProxyDestroyed) to both tests so the
singleton properly cleans up when the sheet is destroyed.

@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.

Sorry @pengfeixx, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 19 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 9, 2026

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

Reviewer's Guide

Fixes a test shutdown SIGSEGV by matching the existing destroyed-signal cleanup pattern in both affected test cases, preventing ReaderImageThreadPoolManager from retaining deleted DocSheet pointers.

File-Level Changes

Change Details Files
Connect test-created sheets to the manager’s destruction cleanup slot before registering them for image-task operations.
  • Add destroyed-signal connections in the task-finished and image-task tests.
  • Ensure the singleton removes sheet references when test objects are deleted, preventing shutdown-time access to dangling pointers.
tests/sidebar/ut_readerimagethreadpoolmanager.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

@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 bffad5e into master Sep 9, 2026
11 of 13 checks passed
@pengfeixx
pengfeixx deleted the agent/agent/720bcd8dbac3 branch September 9, 2026 08:32
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