RebuildData structs are leaked in rebuild() / do_rebuild()
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
AppMenu GTK+ |
Fix Released
|
Medium
|
Charles Kerr | ||
appmenu-gtk (Ubuntu) |
Fix Released
|
Medium
|
Unassigned | ||
Precise |
Won't Fix
|
Medium
|
Mathieu Trudel-Lapierre |
Bug Description
[Impact]
This issue affects most and any users of appgtk-menu, such as nm-applet, especially in environments (such as offices) where the detected wireless networks change a lot, and where roaming can occur frequently. See also: LP: #780602
[Test Case]
Run nm-applet for multiple hours:
- Observe that the VPN Connections submenu still opens and lists connections (if VPN connections are configured)
- Observe that the "More networks" submenu for additional detected wireless networks still opens and lists networks.
A common failure case is where such a submenu will open but show an empty list (a piece of menu a few milimeters long).
[Regression Potential]
Minimal, the fixes have been available for a while in the development release (and in other recent releases) with no regressions.
Possible issues could be introducing new memory leaks with the changes however, and although the risk is minimal, this could cause the same failure behavior as listed above under [Test Case].
--
Binary package hint: appmenu-gtk
The rebuild function creates a hash table mapping the toplevel to the g_timeout id responsible for doing the refresh.
The rebuild function allocates a RebuildData object to pass to the do_rebuild idle function. The do_rebuild callback function frees this object when it is called.
However, the do_rebuild function isn't guaranteed to be called. In fact the whole point of the hash table seems to be that the callback can be canceled. When that happens, the RebuildData object isn't freed, causing a leak.
Additionally, instead of storing a proper reference to the toplevel in the RebuldData object, a weak pointer is stored instead. But if the weak pointer gets set to null, the do_rebuld function will lose its only copy of that pointer, so it will have no way of cleaning up the associated entry in the hash table.
Related branches
- jenkins (community): Needs Fixing (continuous-integration)
- Lars Karlitski (community): Approve
- Indicator Applet Developers: Pending requested
-
Diff: 218 lines (+90/-79)1 file modifiedsrc/bridge.c (+90/-79)
- Charles Kerr (community): Approve
-
Diff: 211 lines (+83/-78)1 file modifiedsrc/bridge.c (+83/-78)
tags: | added: patch |
Changed in appmenu-gtk (Ubuntu): | |
status: | New → Triaged |
importance: | Undecided → Medium |
summary: |
- memory leak in rebuild + RebuildData structs are leaked in rebuild() / do_rebiuld() |
summary: |
- RebuildData structs are leaked in rebuild() / do_rebiuld() + RebuildData structs are leaked in rebuild() / do_rebuild() |
Changed in appmenu-gtk: | |
assignee: | nobody → Charles Kerr (charlesk) |
Changed in appmenu-gtk: | |
milestone: | none → 12.10.1 |
Changed in appmenu-gtk: | |
status: | In Progress → Fix Committed |
status: | Fix Committed → Fix Released |
Changed in appmenu-gtk (Ubuntu Precise): | |
assignee: | nobody → Mathieu Trudel-Lapierre (mathieu-tl) |
importance: | Undecided → Medium |
status: | New → In Progress |
Changed in appmenu-gtk (Ubuntu Precise): | |
status: | Fix Committed → In Progress |
One more thing -- app_menu_ bridge_ insert does not release the "attach-widget" reference it gets, causing another leak.
No patch. It's a one-liner.