Leak in method call handlers for calls that don't require a reply

Bug #1103050 reported by Chris Coulson
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
DBus Menu
Fix Released
Medium
Chris Coulson
libdbusmenu (Ubuntu)
Fix Released
High
Unassigned
Precise
Fix Released
High
Unassigned
Quantal
Won't Fix
High
Unassigned

Bug Description

[Impact]

 * Affected applications with a high number of menu updates reach the maximum value allowed for the ID of a menuitem, and rejects further menu changes. Because the application underneath the menu has already removed the underlying actual GtkMenuItem objects, it is impossible to activate the items -- to effect the actions linked to the menuitems.
 * Some indicators and applications have a relatively high and climbing memory usage due to the way they build menus to be displayed in the panel.

[Test Case]

 * Run nm-applet for a while (multiple days without shutdown, without killing the application), notice whether the menus are still usable.
 * Run indicators for a while, observe memory usage.

[Regression Potential]

Indicators with a very high amount of updates may be affected as circling back past the maximum value, if a new menu item is created with an ID still in use by a menuitem that has not been removed yet, neither or only one of the two menu items might be available to be clicked -- this could confuse users or cause error messages to be displayed.
Risk is low however since network-manager-gnome (nm-applet) is currently the application with the most menu updates.

[Other Info]
We get tonnes of these in Firefox:

==16593== 16,032 (1,152 direct, 14,880 indirect) bytes in 12 blocks are definitely lost in loss record 8,169 of 8,180
==16593== at 0x4C2CD7B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==16593== by 0xBC83410: g_malloc (gmem.c:159)
==16593== by 0xBC98522: g_slice_alloc (gslice.c:1003)
==16593== by 0xBC98A75: g_slice_alloc0 (gslice.c:1029)
==16593== by 0xBA164A4: g_type_create_instance (gtype.c:1892)
==16593== by 0xB9F9E37: g_object_constructor (gobject.c:1855)
==16593== by 0xB9FB920: g_object_newv (gobject.c:1638)
==16593== by 0xB9FBF6B: g_object_new (gobject.c:1548)
==16593== by 0xDC09C03: _g_dbus_method_invocation_new (gdbusmethodinvocation.c:326)
==16593== by 0xDBF129E: validate_and_maybe_schedule_method_call (gdbusconnection.c:4826)
==16593== by 0xDBF282A: on_worker_message_received (gdbusconnection.c:4890)
==16593== by 0xDC06257: _g_dbus_worker_do_read_cb (gdbusprivate.c:492)
==16593== by 0xDB9EA4D: g_simple_async_result_complete (gsimpleasyncresult.c:777)
==16593== by 0xDB9EB7B: complete_in_idle_cb (gsimpleasyncresult.c:789)
==16593== by 0xBC7D5C4: g_main_context_dispatch (gmain.c:2784)
==16593== by 0xBC7D907: g_main_context_iterate.isra.25 (gmain.c:3359)
==16593== by 0xBC7DD71: g_main_loop_run (gmain.c:3553)
==16593== by 0xDC03D95: gdbus_shared_thread_func (gdbusprivate.c:277)
==16593== by 0xBCA15F4: g_thread_proxy (gthread.c:798)
==16593== by 0x5643F9E: start_thread (pthread_create.c:311)
==16593== by 0x59500CC: clone (clone.S:114)

Basically, dbusmenu needs to unref the GDBusMethodInvocation in its handlers when not sending a reply (ie, not calling one of the g_dbus_method_invocation_return_* functions)

I've got a patch that fixes this already

Related branches

Charles Kerr (charlesk)
Changed in libdbusmenu:
status: New → In Progress
assignee: nobody → Chris Coulson (chrisccoulson)
importance: Undecided → Medium
Changed in libdbusmenu:
status: In Progress → Fix Committed
Changed in libdbusmenu (Ubuntu):
importance: Undecided → High
status: New → In Progress
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libdbusmenu - 12.10.2-0ubuntu2

---------------
libdbusmenu (12.10.2-0ubuntu2) raring; urgency=low

  * Backport trunk fixes from Mathieu Trudel-Lapierre and Chris Coulson
    - r438 "Remove uses of g_type_init" (lp: #1102471) for current glib
    - r439 "Fix a memory leak" (lp: #1103050)
    - r440 "Fix multiple leaks due to improper use of g_variant_parse()"
           (lp: #1104136)"
 -- Sebastien Bacher <email address hidden> Wed, 30 Jan 2013 12:10:31 +0100

Changed in libdbusmenu (Ubuntu):
status: In Progress → Fix Released
Changed in libdbusmenu:
status: Fix Committed → Fix Released
description: updated
Changed in libdbusmenu (Ubuntu Precise):
status: New → Triaged
Changed in libdbusmenu (Ubuntu Quantal):
status: New → Triaged
Changed in libdbusmenu (Ubuntu Precise):
importance: Undecided → High
Changed in libdbusmenu (Ubuntu Quantal):
importance: Undecided → High
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Chris, or anyone else affected,

Accepted libdbusmenu into precise-proposed. The package will build now and be available at http://launchpad.net/ubuntu/+source/libdbusmenu/0.6.2-0ubuntu0.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in libdbusmenu (Ubuntu Precise):
status: Triaged → Fix Committed
tags: added: verification-needed
Revision history for this message
Chris Coulson (chrisccoulson) wrote :

I tested this with a build of Firefox with valgrind enabled, and it looks like it works properly

tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package libdbusmenu - 0.6.2-0ubuntu0.2

---------------
libdbusmenu (0.6.2-0ubuntu0.2) precise-proposed; urgency=low

  * debian/patches/lp1103050_method_invocation_leaks.patch: Leak in method call
    handlers for calls that don't require a reply (LP: #1103050)
  * debian/patches/lp1104136_variant_builder_leaks.patch: Leaks GVariant's in
    multiple places. (LP: #1104136)
  * debian/patches/lp1011073_menuitem_id_max.patch: increase maximum value for
    menuitem ID. (LP: #1011073)
 -- Mathieu Trudel-Lapierre <email address hidden> Mon, 17 Jun 2013 15:06:03 -0400

Changed in libdbusmenu (Ubuntu Precise):
status: Fix Committed → Fix Released
Revision history for this message
Colin Watson (cjwatson) wrote : Update Released

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Revision history for this message
Rolf Leggewie (r0lf) wrote :

quantal has seen the end of its life and is no longer receiving any updates. Marking the quantal task for this ticket as "Won't Fix".

Changed in libdbusmenu (Ubuntu Quantal):
status: Triaged → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

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