multiple reference leaks in server.c

Bug #784890 reported by JKL
22
This bug affects 2 people
Affects Status Importance Assigned to Milestone
DBus Menu
Fix Released
Undecided
Unassigned
0.4
Fix Released
Undecided
Unassigned
0.5
Fix Released
Undecided
Unassigned
libdbusmenu (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

This bug discusses the segment of code beginning at line 1192:

http://bazaar.launchpad.net/~dbusmenu-team/dbusmenu/trunk/view/head:/libdbusmenu-glib/server.c

At 1199, we have this line:

const gchar ** props = g_variant_get_strv(g_variant_get_child_value(params, 2), NULL);

According to the documentation, g_variant_get_strv returns a shallow copy of a string array, and the array itself must be freed using g_free, although the underlying strings must not be freed.

http://developer.gnome.org/glib/stable/glib-GVariant.html#g-variant-get-strv

The 'props' array is not freed properly, creating a memory leak. Perhaps the correct place to call g_free is at line 1212, after the code block which creates the items variant using the props data.

Below is a valgrind log illustrating the problem.

==10301== 1,873 (1,824 direct, 49 indirect) bytes in 38 blocks are definitely lost in loss record 9,093 of 9,326
==10301== at 0x4C28FAC: malloc (vg_replace_malloc.c:236)
==10301== by 0x8F62A62: g_malloc (gmem.c:164)
==10301== by 0x8F932A2: g_variant_get_strv (gvariant.c:1391)
==10301== by 0x5046045: bus_get_layout (server.c:1199)
==10301== by 0x6D1834F: call_in_idle_cb (gdbusconnection.c:4434)
==10301== by 0x8F5BBCC: g_main_context_dispatch (gmain.c:2440)
==10301== by 0x8F5C3A7: g_main_context_iterate.clone.6 (gmain.c:3091)
==10301== by 0x8F5C9F1: g_main_loop_run (gmain.c:3299)
==10301== by 0x416D77: main (main.c:101)

Tags: patch

Related branches

Revision history for this message
JKL (jkl102001) wrote :

In addition, there are 3 GVariant reference leaks at the beginning of bus_get_layout.

JKL (jkl102001)
summary: - memory leak in bus_get_layout
+ multiple reference leaks in server.c
Revision history for this message
JKL (jkl102001) wrote :

I marked two other bugs as duplicates. They aren't strictly speaking duplicates, but since everything is in the same file it will be difficult to make separate patches.

Revision history for this message
JKL (jkl102001) wrote :

There are so many reference leaks in server.c it is difficult to list them all individually. This patch generally tries to use g_variant_get to unpack parameters, which avoids many problems with reference counting. It makes the earlier one obsolete.

Revision history for this message
JKL (jkl102001) wrote :

That patch is broken. For one thing, it looks like the calling convention for the server methods doesn't match what it says in the protocol XML, or maybe I have just misunderstood something, so the patch gets the parameter types wrong. There are probably other problems as well.

I believe I have reached the limit of what I can accomplish here. Good luck.

tags: added: patch
Revision history for this message
JKL (jkl102001) wrote :

Before throwing in the towel here, I'll post the current patch I'm using on my own system. It fixes this bug, the two that are marked as duplicates (but which are actually separate bugs in the same file), and also #785828. It also fixes several other problems that don't have separate bugs filed. These unreported problems are:

* crash in menuitem.c if variant parameter is NULL

* reference leaks caused by g_variant_get_child_value

* reference leaks from g_variant_type_new

* typo bug in calling g_variant_parse without error parameter

* variant wrapped inside another variant wrapper before being passed to event callback, when it should be just passed as-is

Ted Gould (ted)
Changed in dbusmenu:
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libdbusmenu - 0.4.90-0ubuntu1

---------------
libdbusmenu (0.4.90-0ubuntu1) oneiric; urgency=low

  [ Ted Gould ]
  * debian/control, debian/*: Renaming packages for library version
    bump from 3 to 4.
  * New upstream release.
    * Fixing visibility for Eclipse (LP: #770263 and LP: #618587)
    * Unseting a GValue properly (LP: #785828)
    * Memory leaks for GVariant usage (LP: #784890)
    * Making GTK 3 default build
    * Removing the SerializableMenuitem object
  * debian/rules: Making GTK2 the special build

  [ Ken VanDine ]
  * +debian/libdbusmenu-jsonloader4.symbols
  * debian/*.symbols
    - Fixed sonames and removed all the duplicate symbols
  * debian/*.install, debian/rules
    - Use cdbs to do the dual builds for gtk2/gtk3
  * debian/control
    - Updated standards version to 3.9.2
    - Set version on the json-glib build depends to >= 0.13.4
    - Make gir1.2-dbusmenu-glib-0.4 break gir1.2-unity-3.0 and
      gir1.2-indicate-0.5 built against older versions of dbusmenu to
      prevent breakage in python apps that use gir loading multiple versions
      of dbusmenu-glib
 -- Ken VanDine <email address hidden> Fri, 24 Jun 2011 14:55:59 -0400

Changed in libdbusmenu (Ubuntu):
status: New → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.