doczkal / rpms / abrt

Forked from rpms/abrt 4 years ago
Clone

Blame SOURCES/0228-Fix-memory-leaks-in-abrt-dbus.patch

a60cd7
From 1902735613a3cc4a1c87e8cbae83a7452bfd8327 Mon Sep 17 00:00:00 2001
a60cd7
From: Jakub Filak <jfilak@redhat.com>
a60cd7
Date: Sun, 1 May 2016 07:13:56 +0200
a60cd7
Subject: [PATCH] Fix memory leaks in abrt-dbus
a60cd7
a60cd7
Fix several repeated leaks that were causing abrt-dbus to waste system
a60cd7
memory.
a60cd7
a60cd7
I used this valgrind command:
a60cd7
    valgrind --tool=memcheck --leak-check=full --show-leak-kinds=all \
a60cd7
             --track-origins=yes --suppressions=glib.supp \
a60cd7
             --log-file=/tmp/leaks-$(date +%s).txt abrt-dbus -vvv -t 10
a60cd7
a60cd7
With suppressions from libsecret and NetworkManager:
a60cd7
  * https://raw.githubusercontent.com/GNOME/libsecret/master/build/glib.supp
a60cd7
  * https://raw.githubusercontent.com/NetworkManager/NetworkManager/master/valgrind.suppressions
a60cd7
a60cd7
The suppressions were needed because Glib allocates a lot of static
a60cd7
stuff and does not free it at exit because it is useless.
a60cd7
a60cd7
Resolves: #1319704
a60cd7
a60cd7
Signed-off-by: Jakub Filak <jfilak@redhat.com>
a60cd7
---
a60cd7
 src/dbus/abrt-dbus.c   | 39 ++++++++++++++++++++++-----------------
a60cd7
 src/dbus/abrt-polkit.c |  3 +++
a60cd7
 src/lib/abrt_conf.c    |  3 +++
a60cd7
 src/lib/abrt_glib.c    |  7 +++----
a60cd7
 src/lib/problem_api.c  |  1 +
a60cd7
 5 files changed, 32 insertions(+), 21 deletions(-)
a60cd7
a60cd7
diff --git a/src/dbus/abrt-dbus.c b/src/dbus/abrt-dbus.c
a60cd7
index 173cec4..0a459cd 100644
a60cd7
--- a/src/dbus/abrt-dbus.c
a60cd7
+++ b/src/dbus/abrt-dbus.c
a60cd7
@@ -97,22 +97,21 @@ static uid_t get_caller_uid(GDBusConnection *connection, GDBusMethodInvocation *
a60cd7
     GError *error = NULL;
a60cd7
     guint caller_uid;
a60cd7
 
a60cd7
-    GDBusProxy * proxy = g_dbus_proxy_new_sync(connection,
a60cd7
-                                     G_DBUS_PROXY_FLAGS_NONE,
a60cd7
-                                     NULL,
a60cd7
-                                     "org.freedesktop.DBus",
a60cd7
-                                     "/org/freedesktop/DBus",
a60cd7
-                                     "org.freedesktop.DBus",
a60cd7
-                                     NULL,
a60cd7
-                                     &error);
a60cd7
-
a60cd7
-    GVariant *result = g_dbus_proxy_call_sync(proxy,
a60cd7
-                                     "GetConnectionUnixUser",
a60cd7
-                                     g_variant_new ("(s)", caller),
a60cd7
-                                     G_DBUS_CALL_FLAGS_NONE,
a60cd7
-                                     -1,
a60cd7
-                                     NULL,
a60cd7
-                                     &error);
a60cd7
+    /* Proxy isn't necessary if only need to call a single method.  By default
a60cd7
+     * GDBusProxy connects to signals and downloads property values. It
a60cd7
+     * suppressed by passing flags argument, but not-creating proxy at all is
a60cd7
+     * much faster and safer. */
a60cd7
+    GVariant *result = g_dbus_connection_call_sync(connection,
a60cd7
+                                                   "org.freedesktop.DBus",
a60cd7
+                                                   "/org/freedesktop/DBus",
a60cd7
+                                                   "org.freedesktop.DBus",
a60cd7
+                                                   "GetConnectionUnixUser",
a60cd7
+                                                   g_variant_new ("(s)", caller),
a60cd7
+                                                   /* reply_type */  NULL,
a60cd7
+                                                   G_DBUS_CALL_FLAGS_NONE,
a60cd7
+                                                   /* timeout */     -1,
a60cd7
+                                                   /* cancellable */ NULL,
a60cd7
+                                                   &error);
a60cd7
 
a60cd7
     if (result == NULL)
a60cd7
     {
a60cd7
@@ -940,7 +939,11 @@ static void handle_method_call(GDBusConnection *connection,
a60cd7
 static gboolean on_timeout_cb(gpointer user_data)
a60cd7
 {
a60cd7
     g_main_loop_quit(loop);
a60cd7
-    return TRUE;
a60cd7
+
a60cd7
+    /* FALSE -> remove and destroy this source. Without it, the timeout source
a60cd7
+     * will be leaked at exit - that isn't a problem but it makes valgrind out
a60cd7
+     * less readable. */
a60cd7
+    return FALSE;
a60cd7
 }
a60cd7
 
a60cd7
 static const GDBusInterfaceVTable interface_vtable =
a60cd7
@@ -1059,6 +1062,8 @@ int main(int argc, char *argv[])
a60cd7
 
a60cd7
     g_dbus_node_info_unref(introspection_data);
a60cd7
 
a60cd7
+    g_main_loop_unref(loop);
a60cd7
+
a60cd7
     free_abrt_conf_data();
a60cd7
 
a60cd7
     return 0;
a60cd7
diff --git a/src/dbus/abrt-polkit.c b/src/dbus/abrt-polkit.c
a60cd7
index 39880e5..34af8a4 100644
a60cd7
--- a/src/dbus/abrt-polkit.c
a60cd7
+++ b/src/dbus/abrt-polkit.c
a60cd7
@@ -59,8 +59,11 @@ static PolkitResult do_check(PolkitSubject *subject, const char *action_id)
a60cd7
                 POLKIT_CHECK_AUTHORIZATION_FLAGS_ALLOW_USER_INTERACTION,
a60cd7
                 cancellable,
a60cd7
                 &error);
a60cd7
+
a60cd7
+    g_object_unref(cancellable);
a60cd7
     g_object_unref(authority);
a60cd7
     g_source_remove(cancel_timeout);
a60cd7
+    g_object_unref(subject);
a60cd7
     if (error)
a60cd7
     {
a60cd7
         g_error_free(error);
a60cd7
diff --git a/src/lib/abrt_conf.c b/src/lib/abrt_conf.c
a60cd7
index 4a49032..5ae64c5 100644
a60cd7
--- a/src/lib/abrt_conf.c
a60cd7
+++ b/src/lib/abrt_conf.c
a60cd7
@@ -37,6 +37,9 @@ void free_abrt_conf_data()
a60cd7
 
a60cd7
     free(g_settings_dump_location);
a60cd7
     g_settings_dump_location = NULL;
a60cd7
+
a60cd7
+    free(g_settings_autoreporting_event);
a60cd7
+    g_settings_autoreporting_event = NULL;
a60cd7
 }
a60cd7
 
a60cd7
 static void ParseCommon(map_string_t *settings, const char *conf_filename)
a60cd7
diff --git a/src/lib/abrt_glib.c b/src/lib/abrt_glib.c
a60cd7
index f7c128e..60e104f 100644
a60cd7
--- a/src/lib/abrt_glib.c
a60cd7
+++ b/src/lib/abrt_glib.c
a60cd7
@@ -22,15 +22,14 @@
a60cd7
 GList *string_list_from_variant(GVariant *variant)
a60cd7
 {
a60cd7
     GList *list = NULL;
a60cd7
-    GVariantIter *iter;
a60cd7
+    GVariantIter iter;
a60cd7
+    g_variant_iter_init(&iter, variant);
a60cd7
     gchar *str;
a60cd7
-    g_variant_get(variant, "as", &iter);
a60cd7
-    while (g_variant_iter_loop(iter, "s", &str))
a60cd7
+    while (g_variant_iter_loop(&iter, "s", &str))
a60cd7
     {
a60cd7
         log_notice("adding: %s", str);
a60cd7
         list = g_list_prepend(list, xstrdup(str));
a60cd7
     }
a60cd7
-    g_variant_unref(variant);
a60cd7
 
a60cd7
     /* we were prepending items, so we should reverse the list to not confuse people
a60cd7
      * by returning items in reversed order than it's in the variant
a60cd7
diff --git a/src/lib/problem_api.c b/src/lib/problem_api.c
a60cd7
index b343882..9fedb3d 100644
a60cd7
--- a/src/lib/problem_api.c
a60cd7
+++ b/src/lib/problem_api.c
a60cd7
@@ -51,6 +51,7 @@ int for_each_problem_in_dir(const char *path,
a60cd7
         if (dir_fd < 0)
a60cd7
         {
a60cd7
             VERB2 perror_msg("can't open problem directory '%s'", full_name);
a60cd7
+            free(full_name);
a60cd7
             continue;
a60cd7
         }
a60cd7
 
a60cd7
-- 
a60cd7
1.8.3.1
a60cd7