Skip to content

Commit bd77fe9

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 1fe247f commit bd77fe9

3 files changed

Lines changed: 275 additions & 17 deletions

File tree

panels/notification/server/notifyserverapplet.cpp

Lines changed: 44 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,25 @@ NotifyServerApplet::NotifyServerApplet(QObject *parent)
2527
NotifyServerApplet::~NotifyServerApplet()
2628
{
2729
qDebug(notifyLog) << "Exit notification server.";
28-
if (m_manager) {
29-
m_manager->deleteLater();
30-
}
30+
3131
if (m_worker) {
32-
m_worker->exit();
33-
m_worker->wait();
34-
m_worker->deleteLater();
32+
if (m_worker->isRunning()) {
33+
m_worker->quit();
34+
m_worker->wait();
35+
// finished deleteLater(manager/worker)
36+
} else {
37+
// worker miss finished
38+
if (m_manager) {
39+
delete m_manager;
40+
m_manager = nullptr;
41+
}
42+
43+
delete m_worker;
44+
m_worker = nullptr;
45+
}
46+
} else if (m_manager) {
47+
delete m_manager;
48+
m_manager = nullptr;
3549
}
3650
}
3751

@@ -42,12 +56,22 @@ bool NotifyServerApplet::load()
4256

4357
bool NotifyServerApplet::init()
4458
{
59+
// Reentrancy protection
60+
if (m_manager || m_worker) {
61+
qWarning(notifyLog) << "NotifyServerApplet is already initialized.";
62+
return true;
63+
}
64+
4565
DApplet::init();
4666

4767
m_manager = new NotificationManager();
4868

4969
if (!m_manager->registerDbusService()) {
5070
qWarning(notifyLog) << QString("Can't register Notifications to the D-Bus object.");
71+
72+
delete m_manager;
73+
m_manager = nullptr;
74+
5175
return false;
5276
}
5377

@@ -60,52 +84,66 @@ bool NotifyServerApplet::init()
6084

6185
m_worker = new QThread();
6286
m_manager->moveToThread(m_worker);
87+
88+
// Register Qt asynchronous cleanup
89+
connect(m_worker, &QThread::finished, m_manager, &QObject::deleteLater);
90+
connect(m_worker, &QThread::finished, m_worker, &QObject::deleteLater);
91+
6392
m_worker->start();
6493
return true;
6594
}
6695

6796
void NotifyServerApplet::actionInvoked(qint64 id, uint bubbleId, const QString &actionKey)
6897
{
98+
CHECK_MANAGER();
6999
QMetaObject::invokeMethod(m_manager, "actionInvoked", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(QString, actionKey));
70100
}
71101

72102
void NotifyServerApplet::actionInvoked(qint64 id, const QString &actionKey)
73103
{
104+
CHECK_MANAGER();
74105
QMetaObject::invokeMethod(m_manager, "actionInvoked", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(QString, actionKey));
75106
}
76107

77108
void NotifyServerApplet::notificationClosed(qint64 id, uint bubbleId, uint reason)
78109
{
110+
CHECK_MANAGER();
79111
QMetaObject::invokeMethod(m_manager, "notificationClosed", Qt::DirectConnection, Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(uint, reason));
80112
}
81113

82114
QVariant NotifyServerApplet::appValue(const QString &appId, int configItem)
83115
{
116+
CHECK_MANAGER_RET({});
84117
return m_manager->GetAppInfo(appId, configItem);
85118
}
86119

87120
void NotifyServerApplet::removeNotification(qint64 id)
88121
{
122+
CHECK_MANAGER();
89123
m_manager->removeNotification(id);
90124
}
91125

92126
void NotifyServerApplet::removeNotifications(const QString &appName)
93127
{
128+
CHECK_MANAGER();
94129
m_manager->removeNotifications(appName);
95130
}
96131

97132
void NotifyServerApplet::removeNotifications()
98133
{
134+
CHECK_MANAGER();
99135
m_manager->removeNotifications();
100136
}
101137

102138
void NotifyServerApplet::removeExpiredNotifications()
103139
{
140+
CHECK_MANAGER();
104141
m_manager->removeExpiredNotifications();
105142
}
106143

107144
void NotifyServerApplet::setBlockClosedId(qint64 id)
108145
{
146+
CHECK_MANAGER();
109147
m_manager->setBlockClosedId(id);
110148
}
111149

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)