Skip to content

fix: use QPointer for default sink DBus interface#413

Merged
mhduiy merged 1 commit into
linuxdeepin:masterfrom
mhduiy:crash
Jan 14, 2026
Merged

fix: use QPointer for default sink DBus interface#413
mhduiy merged 1 commit into
linuxdeepin:masterfrom
mhduiy:crash

Conversation

@mhduiy

@mhduiy mhduiy commented Jan 14, 2026

Copy link
Copy Markdown
Contributor
  1. Changed m_defaultSinkInter from raw DBusSink pointer to QPointer
  2. This prevents potential dangling pointer issues when the DBus object is destroyed
  3. QPointer automatically becomes null when the referenced QObject is deleted
  4. Improves memory safety and prevents crashes in the sound controller

Influence:

  1. Test sound plugin functionality after system sound service restarts
  2. Verify no crashes occur when default audio sink changes
  3. Test volume control and mute functionality stability
  4. Verify plugin correctly handles DBus service disappearance and reappearance

fix: 使用 QPointer 管理默认音频接收器的 DBus 接口

  1. 将 m_defaultSinkInter 从原始 DBusSink 指针改为 QPointer
  2. 防止当 DBus 对象被销毁时出现悬空指针问题
  3. QPointer 在被引用的 QObject 删除时会自动变为 null
  4. 提高内存安全性,防止声音控制器崩溃

Influence:

  1. 测试系统声音服务重启后声音插件的功能
  2. 验证默认音频接收器更改时不会发生崩溃
  3. 测试音量控制和静音功能的稳定性
  4. 验证插件正确处理 DBus 服务消失和重新出现的情况

PMS: BUG-347199

Summary by Sourcery

Bug Fixes:

  • Prevent crashes and dangling references when the default audio sink DBus object is destroyed by tracking it with a QPointer instead of a raw pointer.

1. Changed m_defaultSinkInter from raw DBusSink pointer to
QPointer<DBusSink>
2. This prevents potential dangling pointer issues when the DBus object
is destroyed
3. QPointer automatically becomes null when the referenced QObject is
deleted
4. Improves memory safety and prevents crashes in the sound controller

Influence:
1. Test sound plugin functionality after system sound service restarts
2. Verify no crashes occur when default audio sink changes
3. Test volume control and mute functionality stability
4. Verify plugin correctly handles DBus service disappearance and
reappearance

fix: 使用 QPointer 管理默认音频接收器的 DBus 接口

1. 将 m_defaultSinkInter 从原始 DBusSink 指针改为 QPointer<DBusSink>
2. 防止当 DBus 对象被销毁时出现悬空指针问题
3. QPointer 在被引用的 QObject 删除时会自动变为 null
4. 提高内存安全性,防止声音控制器崩溃

Influence:
1. 测试系统声音服务重启后声音插件的功能
2. 验证默认音频接收器更改时不会发生崩溃
3. 测试音量控制和静音功能的稳定性
4. 验证插件正确处理 DBus 服务消失和重新出现的情况

PMS: BUG-347199
@mhduiy mhduiy requested a review from 18202781743 January 14, 2026 05:28
@sourcery-ai

sourcery-ai Bot commented Jan 14, 2026

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

Reviewer's Guide

This PR replaces the raw DBusSink pointer used for the default sink interface in SoundController with a QPointer to avoid dangling pointers when the underlying QObject is destroyed, improving memory safety and crash resilience around DBus audio sink handling.

Class diagram for SoundController QPointer DBusSink change

classDiagram
    class SoundController {
        - DBusAudio* m_audioInter
        - QPointer~DBusSink~ m_defaultSinkInter
        - QSharedPointer~DConfig~ m_dconfig
    }

    class DBusAudio {
    }

    class DBusSink {
    }

    class DConfig {
    }

    class DSingleton_SoundController_ {
        <<interface>>
    }

    SoundController ..|> DSingleton_SoundController_
    SoundController --> DBusAudio : uses m_audioInter
    SoundController --> DBusSink : weak_ref m_defaultSinkInter
    SoundController --> DConfig : shared_ownership m_dconfig
Loading

File-Level Changes

Change Details Files
Use QPointer instead of a raw pointer for the default DBus audio sink interface to avoid dangling references.
  • Change the type of the default sink member from DBusSink* to QPointer.
  • Rely on QPointer semantics so the member is automatically nulled when the DBusSink QObject is destroyed, reducing risk of use-after-free in sound controller logic.
plugins/dde-dock/sound/soundcontroller.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:

  • Now that m_defaultSinkInter is a QPointer, please ensure all call sites defensively check for null before use and handle the DBus sink disappearing at runtime rather than assuming it is always valid.
  • Consider reviewing where m_defaultSinkInter is (re)assigned to ensure it is consistently reset when the DBus service reappears so the pointer doesn’t stay null after a transient disconnect.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Now that `m_defaultSinkInter` is a `QPointer`, please ensure all call sites defensively check for null before use and handle the DBus sink disappearing at runtime rather than assuming it is always valid.
- Consider reviewing where `m_defaultSinkInter` is (re)assigned to ensure it is consistently reset when the DBus service reappears so the pointer doesn’t stay null after a transient disconnect.

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.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

这段对这段 git diff 的审查意见如下:

1. 语法逻辑

  • 审查结果:通过。
  • 分析:将 DBusSink *m_defaultSinkInter 修改为 QPointer<DBusSink> m_defaultSinkInter 在 C++ 语法上是完全合法的。QPointer 是 Qt 提供的一个模板类,专门用于持有 QObject 的派生类指针。

2. 代码质量

  • 审查结果:优秀,建议采纳。
  • 分析:这是一个非常积极的改动。
    • 原因DBusSink 通常继承自 QObject。在 Qt 中,QObject 对象可能会被异步删除(例如在 DBus 信号处理中,服务断开连接导致代理对象销毁)。
    • 改进点:使用原始指针(Raw Pointer)时,如果对象被删除,指针会变成"悬空指针"(Dangling Pointer),再次访问会导致程序崩溃。而 QPointer 是一个"守护指针",它会在持有的 QObject 被销毁时自动将自己置为 nullptr
    • 效果:这允许代码在使用 m_defaultSinkInter 前通过 if (!m_defaultSinkInter) 进行安全检查,从而显著提高了代码的健壮性,防止了因对象生命周期管理不当引发的段错误。

3. 代码性能

  • 审查结果:影响极小,可忽略。
  • 分析
    • QPointer 相比于原始指针,增加了一些微小的内存开销(存储了一个额外的弱引用计数)和极小的访问开销(检查引用计数)。
    • 但在 DBus 交互这种本身就涉及 IPC(进程间通信)开销的场景下,这点 CPU 和内存的损耗完全可以忽略不计。换取的安全性收益远大于性能损耗。

4. 代码安全

  • 审查结果:显著提升。
  • 分析
    • 防止野指针访问:这是此修改最大的安全贡献。如代码质量部分所述,它避免了 Use-After-Free(释放后使用)漏洞。
    • 空指针检查:配合 QPointer,开发者应确保在使用 m_defaultSinkInter 的所有地方(例如调用其方法前)都检查其是否为空。虽然 QPointer 不会阻止空指针解引用(即如果不检查直接调用 m_defaultSinkInter->func() 依然会崩溃),但它提供了判断对象是否存活的可靠机制。

总结与建议

这是一个高质量的修改,体现了对 Qt 对象生命周期管理的正确理解。

后续建议
在修改此头文件后,请检查对应的 .cpp 实现文件。确保所有使用 m_defaultSinkInter 的地方都增加了判空逻辑,例如:

// 修改前可能直接调用
m_defaultSinkInter->SetVolume(...);

// 修改后建议增加判断
if (m_defaultSinkInter) {
    m_defaultSinkInter->SetVolume(...);
}

这样可以最大化地发挥 QPointer 的安全保护作用。

@deepin-ci-robot

Copy link
Copy Markdown

[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

@mhduiy mhduiy merged commit a671a09 into linuxdeepin:master Jan 14, 2026
10 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