Skip to content

Commit 97e66aa

Browse files
1820278174318202781743
committed
refactor: improve D-Bus resource management with RAII
1. Added custom deleters for D-Bus resources (DBusError, DBusConnection, DBusMessage) 2. Replaced manual resource cleanup with std::unique_ptr RAII wrappers 3. Simplified error handling by removing repetitive cleanup code 4. Improved memory safety by ensuring resources are always released 5. Added detailed documentation for getId() function explaining the libdbus-1 usage The changes ensure proper resource management even in error cases and make the code more maintainable by eliminating manual cleanup. This is particularly important for D-Bus operations that may fail at multiple points. The RAII approach prevents resource leaks and makes the code more robust. refactor: 使用 RAII 改进 D-Bus 资源管理 1. 为 D-Bus 资源(DBusError, DBusConnection, DBusMessage)添加自定义删除器 2. 使用 std::unique_ptr RAII 包装器替代手动资源清理 3. 通过移除重复的清理代码简化错误处理 4. 通过确保资源总是被释放来提高内存安全性 5. 为 getId() 函数添加详细文档说明 libdbus-1 的使用 这些变更确保了即使在错误情况下也能正确处理资源，并通过消除手动清理使代码 更易于维护。这对于可能在多个点失败的 D-Bus 操作尤为重要。RAII 方法防止了 资源泄漏并使代码更加健壮。
1 parent 80a5f3e commit 97e66aa

File tree

1 file changed

+62
-67
lines changed

1 file changed

+62
-67
lines changed

‎src/dsgapplication.cpp‎

Lines changed: 62 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,36 @@ Q_LOGGING_CATEGORY(dsgApp, "dtk.core.dsg", QtInfoMsg)
2525

2626
DCORE_BEGIN_NAMESPACE
2727

28+
// D-Bus resource deleters for RAII management
29+
static void dbusErrorDeleter(DBusError *error) {
30+
if (error) { dbus_error_free(error); delete error; }
31+
}
32+
33+
static void dbusConnectionDeleter(DBusConnection *conn) {
34+
if (conn) dbus_connection_unref(conn);
35+
}
36+
37+
static void dbusMessageDeleter(DBusMessage *msg) {
38+
if (msg) dbus_message_unref(msg);
39+
}
40+
2841
// D-Bus utility functions using libdbus-1
2942
static bool checkDBusServiceActivatable(const QString &service)
3043
{
31-
DBusError error;
32-
dbus_error_init(&error);
44+
auto error = std::unique_ptr<DBusError, void(*)(DBusError*)>(new DBusError, dbusErrorDeleter);
45+
dbus_error_init(error.get());
3346

34-
DBusConnection *connection = dbus_bus_get(DBUS_BUS_SESSION, &error);
35-
if (dbus_error_is_set(&error)) {
36-
qCWarning(dsgApp) << "Failed to connect to session bus:" << error.message;
37-
dbus_error_free(&error);
47+
DBusConnection *connection = dbus_bus_get(DBUS_BUS_SESSION, error.get());
48+
if (dbus_error_is_set(error.get())) {
49+
qCWarning(dsgApp) << "Failed to connect to session bus:" << error->message;
3850
return false;
3951
}
4052

4153
if (!connection) {
4254
qCWarning(dsgApp) << "Failed to get session bus connection";
4355
return false;
4456
}
57+
auto connGuard = std::unique_ptr<DBusConnection, void(*)(DBusConnection*)>(connection, dbusConnectionDeleter);
4558

4659
// Create method call to check if service is registered
4760
DBusMessage *msg = dbus_message_new_method_call(
@@ -53,86 +66,71 @@ static bool checkDBusServiceActivatable(const QString &service)
5366

5467
if (!msg) {
5568
qCWarning(dsgApp) << "Failed to create D-Bus message";
56-
dbus_connection_unref(connection);
5769
return false;
5870
}
71+
auto msgGuard = std::unique_ptr<DBusMessage, void(*)(DBusMessage*)>(msg, dbusMessageDeleter);
5972

6073
const char *serviceName = service.toUtf8().constData();
6174
if (!dbus_message_append_args(msg, DBUS_TYPE_STRING, &serviceName, DBUS_TYPE_INVALID)) {
6275
qCWarning(dsgApp) << "Failed to append arguments to D-Bus message";
63-
dbus_message_unref(msg);
64-
dbus_connection_unref(connection);
6576
return false;
6677
}
6778

6879
// Send message and get reply
69-
DBusMessage *reply = dbus_connection_send_with_reply_and_block(connection, msg, 1000, &error);
70-
dbus_message_unref(msg);
80+
DBusMessage *reply = dbus_connection_send_with_reply_and_block(connection, msg, 1000, error.get());
81+
msgGuard.reset(); // msg is consumed by the call
7182

72-
if (dbus_error_is_set(&error)) {
73-
qCWarning(dsgApp) << "D-Bus call failed:" << error.message;
74-
dbus_error_free(&error);
75-
dbus_connection_unref(connection);
83+
if (dbus_error_is_set(error.get())) {
84+
qCWarning(dsgApp) << "D-Bus call failed:" << error->message;
7685
return false;
7786
}
7887

7988
if (!reply) {
8089
qCWarning(dsgApp) << "No reply received from D-Bus";
81-
dbus_connection_unref(connection);
8290
return false;
8391
}
92+
auto replyGuard = std::unique_ptr<DBusMessage, void(*)(DBusMessage*)>(reply, dbusMessageDeleter);
8493

8594
dbus_bool_t hasOwner = FALSE;
86-
if (!dbus_message_get_args(reply, &error, DBUS_TYPE_BOOLEAN, &hasOwner, DBUS_TYPE_INVALID)) {
87-
qCWarning(dsgApp) << "Failed to parse D-Bus reply:" << error.message;
88-
dbus_error_free(&error);
89-
dbus_message_unref(reply);
90-
dbus_connection_unref(connection);
95+
if (!dbus_message_get_args(reply, error.get(), DBUS_TYPE_BOOLEAN, &hasOwner, DBUS_TYPE_INVALID)) {
96+
qCWarning(dsgApp) << "Failed to parse D-Bus reply:" << error->message;
9197
return false;
9298
}
9399

94-
dbus_message_unref(reply);
95-
96100
if (!hasOwner) {
97-
dbus_connection_unref(connection);
98101
return false;
99102
}
100103

101104
// Check if service is activatable
102-
msg = dbus_message_new_method_call(
105+
DBusMessage *msg2 = dbus_message_new_method_call(
103106
"org.freedesktop.DBus",
104107
"/org/freedesktop/DBus",
105108
"org.freedesktop.DBus",
106109
"ListActivatableNames"
107110
);
108111

109-
if (!msg) {
112+
if (!msg2) {
110113
qCWarning(dsgApp) << "Failed to create ListActivatableNames message";
111-
dbus_connection_unref(connection);
112114
return false;
113115
}
116+
auto msg2Guard = std::unique_ptr<DBusMessage, void(*)(DBusMessage*)>(msg2, dbusMessageDeleter);
114117

115-
reply = dbus_connection_send_with_reply_and_block(connection, msg, 1000, &error);
116-
dbus_message_unref(msg);
118+
DBusMessage *reply2 = dbus_connection_send_with_reply_and_block(connection, msg2, 1000, error.get());
117119

118-
if (dbus_error_is_set(&error)) {
119-
qCWarning(dsgApp) << "ListActivatableNames call failed:" << error.message;
120-
dbus_error_free(&error);
121-
dbus_connection_unref(connection);
120+
if (dbus_error_is_set(error.get())) {
121+
qCWarning(dsgApp) << "ListActivatableNames call failed:" << error->message;
122122
return false;
123123
}
124124

125-
if (!reply) {
126-
dbus_connection_unref(connection);
125+
if (!reply2) {
127126
return false;
128127
}
128+
auto reply2Guard = std::unique_ptr<DBusMessage, void(*)(DBusMessage*)>(reply2, dbusMessageDeleter);
129129

130130
DBusMessageIter iter, array_iter;
131-
dbus_message_iter_init(reply, &iter);
131+
dbus_message_iter_init(reply2, &iter);
132132

133133
if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_ARRAY) {
134-
dbus_message_unref(reply);
135-
dbus_connection_unref(connection);
136134
return false;
137135
}
138136

@@ -149,29 +147,25 @@ static bool checkDBusServiceActivatable(const QString &service)
149147
dbus_message_iter_next(&array_iter);
150148
}
151149

152-
dbus_message_unref(reply);
153-
dbus_connection_unref(connection);
154-
dbus_error_free(&error);
155-
156150
return found;
157151
}
158152

159153
static QByteArray callDBusIdentifyMethod(const QString &service, const QString &path, const QString &interface, int pidfd)
160154
{
161-
DBusError error;
162-
dbus_error_init(&error);
155+
auto error = std::unique_ptr<DBusError, void(*)(DBusError*)>(new DBusError, dbusErrorDeleter);
156+
dbus_error_init(error.get());
163157

164-
DBusConnection *connection = dbus_bus_get(DBUS_BUS_SESSION, &error);
165-
if (dbus_error_is_set(&error)) {
166-
qCWarning(dsgApp) << "Failed to connect to session bus:" << error.message;
167-
dbus_error_free(&error);
158+
DBusConnection *connection = dbus_bus_get(DBUS_BUS_SESSION, error.get());
159+
if (dbus_error_is_set(error.get())) {
160+
qCWarning(dsgApp) << "Failed to connect to session bus:" << error->message;
168161
return QByteArray();
169162
}
170163

171164
if (!connection) {
172165
qCWarning(dsgApp) << "Failed to get session bus connection";
173166
return QByteArray();
174167
}
168+
auto connGuard = std::unique_ptr<DBusConnection, void(*)(DBusConnection*)>(connection, dbusConnectionDeleter);
175169

176170
// Create method call
177171
DBusMessage *msg = dbus_message_new_method_call(
@@ -183,51 +177,40 @@ static QByteArray callDBusIdentifyMethod(const QString &service, const QString &
183177

184178
if (!msg) {
185179
qCWarning(dsgApp) << "Failed to create D-Bus message";
186-
dbus_connection_unref(connection);
187180
return QByteArray();
188181
}
182+
auto msgGuard = std::unique_ptr<DBusMessage, void(*)(DBusMessage*)>(msg, dbusMessageDeleter);
189183

190184
// Append Unix file descriptor
191185
if (!dbus_message_append_args(msg, DBUS_TYPE_UNIX_FD, &pidfd, DBUS_TYPE_INVALID)) {
192186
qCWarning(dsgApp) << "Failed to append Unix FD to D-Bus message";
193-
dbus_message_unref(msg);
194-
dbus_connection_unref(connection);
195187
return QByteArray();
196188
}
197189

198190
// Send message and get reply
199-
DBusMessage *reply = dbus_connection_send_with_reply_and_block(connection, msg, 5000, &error);
200-
dbus_message_unref(msg);
191+
DBusMessage *reply = dbus_connection_send_with_reply_and_block(connection, msg, 5000, error.get());
192+
msgGuard.reset(); // msg is consumed by the call
201193

202-
if (dbus_error_is_set(&error)) {
203-
qCWarning(dsgApp) << "Identify from AM failed:" << error.message;
204-
dbus_error_free(&error);
205-
dbus_connection_unref(connection);
194+
if (dbus_error_is_set(error.get())) {
195+
qCWarning(dsgApp) << "Identify from AM failed:" << error->message;
206196
return QByteArray();
207197
}
208198

209199
if (!reply) {
210200
qCWarning(dsgApp) << "No reply received from Identify method";
211-
dbus_connection_unref(connection);
212201
return QByteArray();
213202
}
203+
auto replyGuard = std::unique_ptr<DBusMessage, void(*)(DBusMessage*)>(reply, dbusMessageDeleter);
214204

215205
const char *result;
216-
if (!dbus_message_get_args(reply, &error, DBUS_TYPE_STRING, &result, DBUS_TYPE_INVALID)) {
217-
qCWarning(dsgApp) << "Failed to parse Identify reply:" << error.message;
218-
dbus_error_free(&error);
219-
dbus_message_unref(reply);
220-
dbus_connection_unref(connection);
206+
if (!dbus_message_get_args(reply, error.get(), DBUS_TYPE_STRING, &result, DBUS_TYPE_INVALID)) {
207+
qCWarning(dsgApp) << "Failed to parse Identify reply:" << error->message;
221208
return QByteArray();
222209
}
223210

224211
QByteArray appId = QByteArray(result);
225212
qCInfo(dsgApp) << "AppId is fetched from AM, and value is " << appId;
226213

227-
dbus_message_unref(reply);
228-
dbus_connection_unref(connection);
229-
dbus_error_free(&error);
230-
231214
return appId;
232215
}
233216

@@ -286,6 +269,18 @@ QByteArray DSGApplication::id()
286269
return result;
287270
}
288271

272+
/**
273+
* Get application ID for a given process ID
274+
*
275+
* This function has been updated to use libdbus-1 instead of Qt D-Bus to fix
276+
* the bug(pms:BUG-278055) where calling this function before QCoreApplication
277+
* initialization would fail. This is particularly important when the service
278+
* is being started by D-Bus activation and DSGApplication::id() is called
279+
* during early startup.
280+
*
281+
* @param pid Process ID to get the application ID for
282+
* @return Application ID as QByteArray, or empty array on failure
283+
*/
289284
QByteArray DSGApplication::getId(qint64 pid)
290285
{
291286
if (!isServiceActivatable("org.desktopspec.ApplicationManager1")) {

0 commit comments

Comments
 (0)