Skip to content

Commit 67fa2f8

Browse files
committed
fix(notification): fix memory leak in NotifyServerApplet destructor
Fixed the issue where NotificationManager was not properly destroyed when the worker thread exited. The deleteLater() call was scheduled but never executed because the thread's event loop had already stopped. Added comprehensive unit tests to verify the fix covers all destruction scenarios including worker-only, manager-only, and both present cases. Log: Fix memory leak in NotifyServerApplet destructor fix(notification): 修复 NotifyServerApplet 析构函数中的内存泄漏 修复了当工作线程退出时 NotificationManager 未被正确销毁的问题。 deleteLater() 被调度但从未执行,因为线程的事件循环已经停止。 添加了全面的单元测试来验证修复涵盖了所有销毁场景, 包括仅 worker、仅 manager 和两者都存在的情况。 Log: 修复 NotifyServerApplet 析构函数中的内存泄漏
1 parent dfa6319 commit 67fa2f8

3 files changed

Lines changed: 287 additions & 17 deletions

File tree

panels/notification/server/notifyserverapplet.cpp

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
#include "dbusadaptor.h"
88
#include "pluginfactory.h"
99

10+
#include <QCoreApplication>
11+
#include <QEvent>
1012
#include <QThread>
1113
#include <QLoggingCategory>
1214

@@ -25,13 +27,37 @@ NotifyServerApplet::NotifyServerApplet(QObject *parent)
2527
NotifyServerApplet::~NotifyServerApplet()
2628
{
2729
qDebug(notifyLog) << "Exit notification server.";
28-
if (m_manager) {
29-
m_manager->deleteLater();
30-
}
30+
31+
constexpr int kWaitTimeoutMs = 3000;
32+
constexpr int kTerminateWaitMs = 1000;
33+
3134
if (m_worker) {
32-
m_worker->exit();
33-
m_worker->wait();
34-
m_worker->deleteLater();
35+
if (m_worker->isRunning()) {
36+
m_worker->quit();
37+
38+
if (!m_worker->wait(kWaitTimeoutMs)) {
39+
qWarning(notifyLog)
40+
<< "Worker thread did not exit in time, terminating.";
41+
42+
m_worker->terminate();
43+
44+
if (!m_worker->wait(kTerminateWaitMs)) {
45+
qCritical(notifyLog)
46+
<< "Worker thread terminate timeout.";
47+
}
48+
}
49+
}
50+
51+
if (m_manager) {
52+
delete m_manager;
53+
m_manager = nullptr;
54+
}
55+
56+
delete m_worker;
57+
m_worker = nullptr;
58+
} else if (m_manager) {
59+
delete m_manager;
60+
m_manager = nullptr;
3561
}
3662
}
3763

@@ -42,12 +68,22 @@ bool NotifyServerApplet::load()
4268

4369
bool NotifyServerApplet::init()
4470
{
71+
// Reentrancy protection
72+
if (m_manager || m_worker) {
73+
qWarning(notifyLog) << "NotifyServerApplet is already initialized.";
74+
return true;
75+
}
76+
4577
DApplet::init();
4678

4779
m_manager = new NotificationManager();
4880

4981
if (!m_manager->registerDbusService()) {
5082
qWarning(notifyLog) << QString("Can't register Notifications to the D-Bus object.");
83+
84+
delete m_manager;
85+
m_manager = nullptr;
86+
5187
return false;
5288
}
5389

@@ -60,52 +96,66 @@ bool NotifyServerApplet::init()
6096

6197
m_worker = new QThread();
6298
m_manager->moveToThread(m_worker);
99+
100+
// Register Qt asynchronous cleanup
101+
// connect(m_worker, &QThread::finished, m_manager, &QObject::deleteLater);
102+
// connect(m_worker, &QThread::finished, m_worker, &QObject::deleteLater);
103+
63104
m_worker->start();
64105
return true;
65106
}
66107

67108
void NotifyServerApplet::actionInvoked(qint64 id, uint bubbleId, const QString &actionKey)
68109
{
110+
CHECK_MANAGER();
69111
QMetaObject::invokeMethod(m_manager, "actionInvoked", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(QString, actionKey));
70112
}
71113

72114
void NotifyServerApplet::actionInvoked(qint64 id, const QString &actionKey)
73115
{
116+
CHECK_MANAGER();
74117
QMetaObject::invokeMethod(m_manager, "actionInvoked", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(QString, actionKey));
75118
}
76119

77120
void NotifyServerApplet::notificationClosed(qint64 id, uint bubbleId, uint reason)
78121
{
122+
CHECK_MANAGER();
79123
QMetaObject::invokeMethod(m_manager, "notificationClosed", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, reason));
80124
}
81125

82126
QVariant NotifyServerApplet::appValue(const QString &appId, int configItem)
83127
{
128+
CHECK_MANAGER_RET({});
84129
return m_manager->GetAppInfo(appId, configItem);
85130
}
86131

87132
void NotifyServerApplet::removeNotification(qint64 id)
88133
{
134+
CHECK_MANAGER();
89135
m_manager->removeNotification(id);
90136
}
91137

92138
void NotifyServerApplet::removeNotifications(const QString &appName)
93139
{
140+
CHECK_MANAGER();
94141
m_manager->removeNotifications(appName);
95142
}
96143

97144
void NotifyServerApplet::removeNotifications()
98145
{
146+
CHECK_MANAGER();
99147
m_manager->removeNotifications();
100148
}
101149

102150
void NotifyServerApplet::removeExpiredNotifications()
103151
{
152+
CHECK_MANAGER();
104153
m_manager->removeExpiredNotifications();
105154
}
106155

107156
void NotifyServerApplet::setBlockClosedId(qint64 id)
108157
{
158+
CHECK_MANAGER();
109159
m_manager->setBlockClosedId(id);
110160
}
111161

panels/notification/server/notifyserverapplet.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,24 @@
44

55
#pragma once
66

7+
#include <QPointer>
78
#include "applet.h"
89

10+
#define CHECK_MANAGER() \
11+
do { \
12+
if (!m_manager) { \
13+
qWarning(notifyLog) << "NotificationManager is null."; \
14+
return; \
15+
} \
16+
} while(0)
17+
18+
#define CHECK_MANAGER_RET(val) \
19+
do { \
20+
if (!m_manager) { \
21+
qWarning(notifyLog) << "NotificationManager is null."; \
22+
return val; \
23+
} \
24+
} while(0)
925
namespace notification {
1026

1127
class NotificationManager;
@@ -34,8 +50,8 @@ public Q_SLOTS:
3450
void setBlockClosedId(qint64 id);
3551

3652
private:
37-
NotificationManager *m_manager = nullptr;
38-
QThread *m_worker = nullptr;
53+
QPointer<NotificationManager> m_manager;
54+
QPointer<QThread> m_worker;
3955
};
4056

4157
}

0 commit comments

Comments
 (0)