fix: disconnect D-Bus system bus connections in destructor before thread exit#556
Merged
Ivy233 merged 1 commit intoJun 5, 2026
Conversation
Ivy233
requested review from
18202781743,
caixr23,
fly602,
mhduiy,
robertkill and
xionglinlin
Reviewer's GuideEnsures all D-Bus system bus signal connections established in NetManagerThreadPrivate::doInit() are explicitly disconnected in the destructor before the worker thread quits, preventing use-after-free crashes during global QDBusConnectionManager cleanup. Sequence diagram for NetManagerThreadPrivate destructor D-Bus disconnectionsequenceDiagram
participant NetManagerThreadPrivate
participant WorkerThread as m_thread
participant QDBusConnection_systemBus as QDBusConnection_systemBus
participant QDBusConnectionManager
NetManagerThreadPrivate->>NetManagerThreadPrivate: disconnect()
NetManagerThreadPrivate->>QDBusConnection_systemBus: disconnect(com.deepin.defender.netcheck PropertiesChanged this)
NetManagerThreadPrivate->>QDBusConnection_systemBus: disconnect(org.freedesktop.login1 PrepareForSleep this)
NetManagerThreadPrivate->>QDBusConnection_systemBus: disconnect(org.deepin.dde.Network1 PortalDetected this)
NetManagerThreadPrivate->>QDBusConnection_systemBus: disconnect(org.deepin.dde.Bluetooth1 AdapterAdded this)
NetManagerThreadPrivate->>QDBusConnection_systemBus: disconnect(org.deepin.dde.Bluetooth1 AdapterRemoved this)
NetManagerThreadPrivate->>QDBusConnection_systemBus: disconnect(org.deepin.dde.AirplaneMode1 PropertiesChanged this)
NetManagerThreadPrivate->>WorkerThread: quit()
NetManagerThreadPrivate->>WorkerThread: wait(QDeadlineTimer)
QDBusConnectionManager->>QDBusConnection_systemBus: close(qt_default_system_bus)
QDBusConnection_systemBus-->>QDBusConnectionManager: cleanup without callbacks to NetManagerThreadPrivate
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The destructor now duplicates the signal list from doInit(); consider extracting shared helper methods for registering/unregistering the D-Bus connections so changes to one side can't get out of sync.
- Relying on ConfigSetting::instance() in the destructor can be fragile if that singleton's lifetime changes; it may be safer to cache the supportPortalPromp flag during initialization and reuse the cached value here.
- To reduce the risk of missing future signals, consider using a broader disconnect pattern (e.g., grouping related connections or storing QMetaObject::Connection handles) instead of repeating all systemBus() signature strings in the destructor.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The destructor now duplicates the signal list from doInit(); consider extracting shared helper methods for registering/unregistering the D-Bus connections so changes to one side can't get out of sync.
- Relying on ConfigSetting::instance() in the destructor can be fragile if that singleton's lifetime changes; it may be safer to cache the supportPortalPromp flag during initialization and reuse the cached value here.
- To reduce the risk of missing future signals, consider using a broader disconnect pattern (e.g., grouping related connections or storing QMetaObject::Connection handles) instead of repeating all systemBus() signature strings in the destructor.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
reviewed
Jun 4, 2026
| // 断开 doInit() 中注册的 D-Bus 系统总线连接 | ||
| // 必须在 m_thread->quit() 之前执行，否则 QDBusConnectionManager 线程 | ||
| // 在全局清理时会访问已释放的工作线程对象，导致 use-after-free 崩溃 | ||
| QDBusConnection::systemBus().disconnect("com.deepin.defender.netcheck", "/com/deepin/defender/netcheck", "org.freedesktop.DBus.Properties", "PropertiesChanged", this, SLOT(onNetCheckPropertiesChanged(QString, QVariantMap, QStringList))); |
Contributor
There was a problem hiding this comment.
上面的disconnect(); 不会断开所有跟它的连接么，
Contributor
Author
There was a problem hiding this comment.
QObject::disconnect()（第111行）只会断开 Qt QMetaObject 管理的 signal-slot 连接，即通过 connect(sender, signal, receiver, slot) 注册的那些连接，以及清除队列中的 QMetaObject::invokeMethod 调用。
而 QDBusConnection::systemBus().connect() 在 doInit() 中注册的 D-Bus 信号监听，是存储在 QDBusConnection 内部独立数据结构中的。它在底层做的是：
- 向 D-Bus 守护进程添加匹配规则（match rule）
- 在 QDBusConnection 内部维护一个回调注册表
QObject::disconnect() 感知不到这个注册表，因此无法断开这些 D-Bus 连接。
如果没有第 116-130 行的显式 QDBusConnection::systemBus().disconnect() 调用，当应用退出时 QDBusConnectionManager 全局清理并关闭 qt_default_system_bus 时，仍然会通过其内部注册表尝试向已销毁的 this 对象派发信号，从而导致 use-after-free 崩溃——这正是我们要修复的那个崩溃。
所以这两者是必须的，并不多余。
Contributor
There was a problem hiding this comment.
QDBusConnection::systemBus().connect()连接的槽函数，receiver是this，这样this->disconnect也不行么？
Contributor
Author
There was a problem hiding this comment.
这个 disconnect() 断开的是 this 作为 sender 的连接，而 QDBusConnection::systemBus().connect() 注册的信号钩子中，this 是 receiver。两者作用方向不同。
关键崩溃点在 Qt6 qdbusintegrator.cpp:1140-1152，QDBusConnectionPrivate::closeConnection() 在全局清理时会遍历内部的 signalHooks 表直接访问 hook.obj（即 this）。而这个表里的条目要通过 destroyed 信号以 BlockingQueuedConnection 投递到 D-Bus 线程来清理。如果退出时 D-Bus 线程事件循环已停止，objectDestroyed 回调不会执行，signalHooks 里就留下指向已释放对象的悬空指针。
所以两者解决不同层次的问题：this->disconnect() 清理 this 作为 sender 的连接；QDBusConnection::systemBus().disconnect() 主动从 signalHooks 中移除条目，确保 closeConnection() 不会踩到悬空指针。详见 Qt6 qdbusintegrator.cpp:1343 中 hook.obj = receiver 和 qdbusintegrator.cpp:1237-1251 的 objectDestroyed 正常清理流程。
Contributor
Author
There was a problem hiding this comment.
同上，这个 disconnect() 断开的是 this 作为 sender 的连接，D-Bus 的 signalHooks 中 this 是 receiver 存在 hook.obj 里。所以需要显式 QDBusConnection::systemBus().disconnect() 来清理 QDBusConnectionPrivate 内部的 signalHooks 表，详见上一条回复中的 Qt6 源码分析。
…ead exit Disconnect all QDBusConnection::systemBus() signals registered in doInit() from the worker thread before quitting m_thread. This prevents a use-after-free crash in QDBusConnectionManager during application exit when it tries to close qt_default_system_bus and accesses already-freed worker-thread objects. Log: Fix dcc-network plugin crash on exit due to D-Bus connection leak fix: 在析构函数中断开D-Bus系统总线连接，避免线程退出后崩溃 在退出工作线程(m_thread)之前，断开doInit()中注册的所有 QDBusConnection::systemBus()信号连接。防止应用退出时 QDBusConnectionManager清理qt_default_system_bus时 访问已释放的工作线程对象导致use-after-free崩溃。 Log: 修复dcc-network插件退出时因D-Bus连接未清理导致的崩溃 PMS: BUG-363425
Ivy233
force-pushed
the
bugfix/dbus-disconnect-crash-netmanagerthread
branch
from
1dff451 to
cb793d0
Compare
deepin pr auto review你好！我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。这次修改的主要目的是在析构函数中提前断开D-Bus系统总线的连接，以防止在对象析构及子线程退出期间，D-Bus信号回调访问已释放的对象，从而引发 以下是我对这段代码在语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见及改进建议： 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
综合改进后的代码示例基于以上分析，我为你重构了这部分代码，提升了可读性、性能和安全性： NetManagerThreadPrivate::~NetManagerThreadPrivate()
{
// 先断开所有信号，防止析构期间再有新任务（如singleShot）入队
disconnect();
// 断开 doInit() 中注册的 D-Bus 系统总线连接
// 必须在 m_thread->quit() 之前执行，否则 QDBusConnectionManager 线程
// 在全局清理时会访问已释放的工作线程对象，导致 use-after-free 崩溃
uninitDBusConnections();
m_thread->quit();
// 增大等待时间至1000ms，避免50ms定时器回调等正在执行的任务被terminate强杀
m_thread->wait(QDeadlineTimer(1000));
}
// 建议在类中新增该私有函数，与 doInit() 形成对称逻辑
void NetManagerThreadPrivate::uninitDBusConnections()
{
const QDBusConnection &bus = QDBusConnection::systemBus();
// 1. 断开 netcheck 相关
bus.disconnect("com.deepin.defender.netcheck",
"/com/deepin/defender/netcheck",
"org.freedesktop.DBus.Properties",
"PropertiesChanged",
this, SLOT(onNetCheckPropertiesChanged(QString, QVariantMap, QStringList)));
// 2. 断开 login1 相关
bus.disconnect("org.freedesktop.login1",
"/org/freedesktop/login1",
"org.freedesktop.login1.Manager",
"PrepareForSleep",
this, SLOT(onPrepareForSleep(bool)));
// 3. 断开 Portal 相关
if (ConfigSetting::instance()->supportPortalPromp()) {
bus.disconnect("org.deepin.dde.Network1",
"/org/deepin/service/SystemNetwork",
"org.deepin.service.SystemNetwork",
"PortalDetected",
this,
SLOT(onPortalDetected(const QString &)));
}
// 4. 断开飞行模式相关
// 注意：如果是单标志位判断，建议改为 testFlag (单数)
if (m_flags.testFlag(NetType::NetManagerFlag::Net_Airplane)) {
bus.disconnect("org.deepin.dde.Bluetooth1",
"/org/deepin/dde/Bluetooth1",
"org.deepin.dde.Bluetooth1",
"AdapterAdded",
this, SLOT(getAirplaneModeEnabled()));
bus.disconnect("org.deepin.dde.Bluetooth1",
"/org/deepin/dde/Bluetooth1",
"org.deepin.dde.Bluetooth1",
"AdapterRemoved",
this, SLOT(getAirplaneModeEnabled()));
bus.disconnect("org.deepin.dde.AirplaneMode1",
"/org/deepin/dde/AirplaneMode1",
"org.freedesktop.DBus.Properties",
"PropertiesChanged",
this, SLOT(onAirplaneModeEnabledPropertiesChanged(QString, QVariantMap, QStringList)));
}
}进一步优化的思考： |
18202781743
approved these changes
Jun 5, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, Ivy233 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Disconnect all QDBusConnection::systemBus() signals registered in doInit() from the worker thread before quitting m_thread. This prevents a use-after-free crash in QDBusConnectionManager during application exit when it tries to close qt_default_system_bus and accesses already-freed worker-thread objects.
Log: Fix dcc-network plugin crash on exit due to D-Bus connection leak
fix: 在析构函数中断开D-Bus系统总线连接，避免线程退出后崩溃
在退出工作线程(m_thread)之前，断开doInit()中注册的所有QDBusConnection::systemBus()信号连接。防止应用退出时QDBusConnectionManager清理qt_default_system_bus时访问已释放的工作线程对象导致use-after-free崩溃。
Log: 修复dcc-network插件退出时因D-Bus连接未清理导致的崩溃
PMS: BUG-363425
Summary by Sourcery
Bug Fixes: