fix(editor): Disconnect D-Bus signals in TextEdit destructor to preve…#426
Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom Mar 3, 2026
Merged
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengzhongyuan365-dev 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 |
Reviewer's GuideThis PR updates the TextEdit destructor to explicitly disconnect gesture and audio-related D-Bus signal handlers based on system and Qt versions, preventing callbacks to already-destroyed TextEdit instances and eliminating a crash risk on teardown. Sequence diagram for TextEdit destruction and D-Bus signal disconnectionsequenceDiagram
participant TextEdit
participant Utils
participant QDBusConnection
participant SystemBus
participant SessionBus
participant DBusGestureService
participant DBusAudioService
TextEdit->>TextEdit: ~TextEdit()
TextEdit->>Utils: getSystemVersion()
Utils-->>TextEdit: SystemVersion
TextEdit->>QDBusConnection: sessionBus()
QDBusConnection-->>TextEdit: dbus
alt systemVersion is V23
TextEdit->>SystemBus: disconnect(org.deepin.dde.Gesture1, /org/deepin/dde/Gesture1, org.deepin.dde.Gesture1, Event, this, fingerZoom(QString, QString, int))
SystemBus-->>DBusGestureService: unregister handler for fingerZoom
else other systemVersion
TextEdit->>SystemBus: disconnect(com.deepin.daemon.Gesture, /com/deepin/daemon/Gesture, com.deepin.daemon.Gesture, Event, this, fingerZoom(QString, QString, int))
SystemBus-->>DBusGestureService: unregister handler for fingerZoom
end
alt Qt version >= 6.0.0
TextEdit->>SessionBus: disconnect(org.deepin.dde.Audio1, /org/deepin/dde/Audio1, org.deepin.dde.Audio1, PortEnabledChanged, this, onAudioPortEnabledChanged(quint32, QString, bool))
SessionBus-->>DBusAudioService: unregister handler for onAudioPortEnabledChanged
else Qt version < 6.0.0
TextEdit->>SessionBus: disconnect(com.deepin.daemon.Audio, /com/deepin/daemon/Audio, com.deepin.daemon.Audio, PortEnabledChanged, this, onAudioPortEnabledChanged(quint32, QString, bool))
SessionBus-->>DBusAudioService: unregister handler for onAudioPortEnabledChanged
end
TextEdit-->>TextEdit: continue destructor teardown without pending callbacks
Class diagram for updated TextEdit destructor D-Bus cleanupclassDiagram
class TextEdit {
+TextEdit(QWidget *parent)
+~TextEdit()
-QAbstractAnimation *m_scrollAnimation
+void fingerZoom(QString device, QString gesture, int value)
+void onAudioPortEnabledChanged(quint32 port, QString name, bool enabled)
}
class Utils {
<<static>>
+SystemVersion getSystemVersion()
<<enum>> SystemVersion
+SystemVersion V23
}
class QDBusConnection {
+static QDBusConnection sessionBus()
+static QDBusConnection systemBus()
+bool disconnect(QString service, QString path, QString interface, QString name, QObject *receiver, const char *slot)
}
class DBusGestureService_V23 {
+QString serviceName = org.deepin.dde.Gesture1
+QString path = /org/deepin/dde/Gesture1
+QString interfaceName = org.deepin.dde.Gesture1
+QString signalName = Event
}
class DBusGestureService_Default {
+QString serviceName = com.deepin.daemon.Gesture
+QString path = /com/deepin/daemon/Gesture
+QString interfaceName = com.deepin.daemon.Gesture
+QString signalName = Event
}
class DBusAudioService_Qt6 {
+QString serviceName = org.deepin.dde.Audio1
+QString path = /org/deepin/dde/Audio1
+QString interfaceName = org.deepin.dde.Audio1
+QString signalName = PortEnabledChanged
}
class DBusAudioService_Qt5 {
+QString serviceName = com.deepin.daemon.Audio
+QString path = /com/deepin/daemon/Audio
+QString interfaceName = com.deepin.daemon.Audio
+QString signalName = PortEnabledChanged
}
TextEdit ..> Utils : uses getSystemVersion
TextEdit ..> QDBusConnection : disconnects signals in destructor
TextEdit ..> DBusGestureService_V23 : disconnects fingerZoom
TextEdit ..> DBusGestureService_Default : disconnects fingerZoom
TextEdit ..> DBusAudioService_Qt6 : disconnects onAudioPortEnabledChanged
TextEdit ..> DBusAudioService_Qt5 : disconnects onAudioPortEnabledChanged
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
QDBusConnection dbus = QDBusConnection::sessionBus();local variable is misleading since you immediately call the staticsystemBus()/sessionBus()methods on it; consider either using the staticQDBusConnection::systemBus()/sessionBus()directly or storing the specific bus you actually want to disconnect from. - The new destructor block has inconsistent indentation (extra leading spaces before the comment, switch, and
#if), which makes the code harder to scan compared to the surrounding style; aligning it with the existing destructor body would improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `QDBusConnection dbus = QDBusConnection::sessionBus();` local variable is misleading since you immediately call the static `systemBus()`/`sessionBus()` methods on it; consider either using the static `QDBusConnection::systemBus()`/`sessionBus()` directly or storing the specific bus you actually want to disconnect from.
- The new destructor block has inconsistent indentation (extra leading spaces before the comment, switch, and `#if`), which makes the code harder to scan compared to the surrounding style; aligning it with the existing destructor body would improve readability.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…nt callbacks to destroyed objects - Added disconnection of D-Bus signals for gesture and audio events in the TextEdit destructor. - Ensured proper handling based on system version and Qt version to avoid potential crashes. Log: Prevent callbacks to destroyed TextEdit instances by disconnecting D-Bus signals. bug: https://pms.uniontech.com/bug-view-350765.html
dengzhongyuan365-dev
force-pushed
the
master
branch
from
642f032 to
0b05d0a
Compare
deepin pr auto review这段代码主要是在 以下是对这段代码的详细审查和改进建议： 1. 代码逻辑与安全性优点：
问题与改进建议：
2. 代码性能
3. 改进后的代码示例假设手势服务确实应该连接在 TextEdit::~TextEdit()
{
qDebug() << "Destroying TextEdit component";
// Disconnect D-Bus signals to prevent callbacks to destroyed object
// 修正：统一使用 sessionBus，除非手势服务明确在 systemBus 上
QDBusConnection sessionBus = QDBusConnection::sessionBus();
// 1. 断开手势信号
bool disconnectedGesture = false;
switch (Utils::getSystemVersion()) {
case Utils::V23:
disconnectedGesture = sessionBus.disconnect("org.deepin.dde.Gesture1",
"/org/deepin/dde/Gesture1",
"org.deepin.dde.Gesture1",
"Event",
this, SLOT(fingerZoom(QString, QString, int)));
break;
default:
disconnectedGesture = sessionBus.disconnect("com.deepin.daemon.Gesture",
"/com/deepin/daemon/Gesture",
"com.deepin.daemon.Gesture",
"Event",
this, SLOT(fingerZoom(QString, QString, int)));
break;
}
if (!disconnectedGesture) {
// 使用 qWarning 而不是 qDebug，因为断开失败可能意味着潜在的逻辑错误
qWarning() << "Failed to disconnect Gesture signal in TextEdit destructor.";
}
// 2. 断开音频信号
bool disconnectedAudio = false;
#if (QT_VERSION >= QT_VERSION_CHECK(6, 0, 0))
disconnectedAudio = sessionBus.disconnect("org.deepin.dde.Audio1",
"/org/deepin/dde/Audio1",
"org.deepin.dde.Audio1",
"PortEnabledChanged",
this, SLOT(onAudioPortEnabledChanged(quint32, QString, bool)));
#else
disconnectedAudio = sessionBus.disconnect("com.deepin.daemon.Audio",
"/com/deepin/daemon/Audio",
"com.deepin.daemon.Audio",
"PortEnabledChanged",
this, SLOT(onAudioPortEnabledChanged(quint32, QString, bool)));
#endif
if (!disconnectedAudio) {
qWarning() << "Failed to disconnect Audio signal in TextEdit destructor.";
}
// 原有逻辑
if (m_scrollAnimation != nullptr) {
qDebug() << "m_scrollAnimation is not null";
if (m_scrollAnimation->state() != QAbstractAnimation::Stopped) {
// ...
}
}
}总结这段代码在安全性上的初衷是正确的，但在实现细节上存在 |
lzwind
approved these changes
Mar 3, 2026
Contributor
Author
|
/merge |
Contributor
|
This pr cannot be merged! (status: unstable) |
Contributor
Author
|
/forcemerge |
Contributor
|
This pr force merged! (status: unstable) |
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.
…nt callbacks to destroyed objects
Log: Prevent callbacks to destroyed TextEdit instances by disconnecting D-Bus signals.
bug: https://pms.uniontech.com/bug-view-350765.html
Summary by Sourcery
Bug Fixes: