Merge lp:~ted/ubuntu-app-launch/lp1495871-unref-context into lp:ubuntu-app-launch/15.10

Proposed by Ted Gould
Status: Superseded
Proposed branch: lp:~ted/ubuntu-app-launch/lp1495871-unref-context
Merge into: lp:ubuntu-app-launch/15.10
Prerequisite: lp:~ted/ubuntu-app-launch/pids-tools
Diff against target: 57 lines (+12/-3)
2 files modified
helpers-shared.c (+11/-2)
libubuntu-app-launch/ubuntu-app-launch.h (+1/-1)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/lp1495871-unref-context
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Indicator Applet Developers Pending
Review via email: mp+271322@code.launchpad.net

This proposal supersedes a proposal from 2015-09-16.

This proposal has been superseded by a proposal from 2015-12-02.

Commit message

Ensure all pending events on the context are complete before unref'ing it.

Description of the change

So while we create a context to ensure that we can capture all the events to the CGManager connection it seems that some of those events can get stuck on the context, and they need to get processed to complete. Otherwise they hold onto references of things, and that actually results in a g_spawn getting left around because it never gets free'd. (actually of the context itself, which is a bit confusing, but eh, refcounting is fun)

Also brought in the pid tools branch because it made testing this much, much easier. You can use the ubuntu-app-list-pids tool to cause the FD to be leaked and then see it fixed with something like (assuming you've done ubuntu-app-launch firefox):

valgrind --track-fds=yes ./ubuntu-app-list-pids firefox

Which on trunk will have 6 FDs left over, but with this branch only 5. (which come from globals that we don't control)

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
215. By Ted Gould

Merge FTBFS

Unmerged revisions

215. By Ted Gould

Merge FTBFS

214. By Ted Gould

Fix GDocs syntax for warning

213. By Ted Gould

Make sure get and set use the same value

212. By Ted Gould

Ensure that we process all the events on the context before unref'ing it. There may be references in the events.

211. By Ted Gould

Grabbing the pid tools branch

210. By Ted Gould

Easier to read an a cast that is needed

209. By Ted Gould

Plug leaking context

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'helpers-shared.c'
--- helpers-shared.c 2015-07-15 02:32:54 +0000
+++ helpers-shared.c 2015-09-16 14:36:19 +0000
@@ -145,6 +145,8 @@
145 cgroup_manager_connection_core_cb(g_dbus_connection_new_for_address_finish, res, (cgm_connection_t *)data);145 cgroup_manager_connection_core_cb(g_dbus_connection_new_for_address_finish, res, (cgm_connection_t *)data);
146}146}
147147
148G_DEFINE_QUARK(CGMANAGER_CONTEXT, cgmanager_context);
149
148/* Get the connection to the cgroup manager */150/* Get the connection to the cgroup manager */
149GDBusConnection *151GDBusConnection *
150cgroup_manager_connection (void)152cgroup_manager_connection (void)
@@ -191,7 +193,9 @@
191 g_main_context_pop_thread_default(context);193 g_main_context_pop_thread_default(context);
192194
193 if (!use_session_bus && connection.con != NULL) {195 if (!use_session_bus && connection.con != NULL) {
194 g_object_set_data(G_OBJECT(connection.con), "cgmanager-context", context);196 g_object_set_qdata(G_OBJECT(connection.con),
197 cgmanager_context_quark(),
198 context);
195 } else {199 } else {
196 g_main_context_unref(context);200 g_main_context_unref(context);
197 }201 }
@@ -213,7 +217,7 @@
213 if (cgmanager == NULL)217 if (cgmanager == NULL)
214 return;218 return;
215219
216 GMainContext * creationcontext = g_object_get_data(G_OBJECT(cgmanager), "cgmanager-context");220 GMainContext * creationcontext = g_object_get_qdata(G_OBJECT(cgmanager), cgmanager_context_quark());
217 if (creationcontext == NULL) {221 if (creationcontext == NULL) {
218 g_object_unref(cgmanager);222 g_object_unref(cgmanager);
219 return;223 return;
@@ -228,6 +232,11 @@
228 }232 }
229233
230 g_object_unref(cgmanager);234 g_object_unref(cgmanager);
235
236 while (g_main_context_pending(creationcontext)) {
237 g_main_context_iteration(creationcontext, TRUE /* may block */);
238 }
239
231 g_main_context_unref(creationcontext);240 g_main_context_unref(creationcontext);
232}241}
233242
234243
=== modified file 'libubuntu-app-launch/ubuntu-app-launch.h'
--- libubuntu-app-launch/ubuntu-app-launch.h 2015-09-16 14:36:19 +0000
+++ libubuntu-app-launch/ubuntu-app-launch.h 2015-09-16 14:36:19 +0000
@@ -394,7 +394,7 @@
394 * Checks to see if an application is running and returns394 * Checks to see if an application is running and returns
395 * the PIDs associated with it.395 * the PIDs associated with it.
396 *396 *
397 * Return Value: (transfer full) (element-type GLib.Pid) A list397 * Return Value: (transfer full) (element-type GLib.Pid): A list
398 * of PIDs associated with @appid, empty if not running.398 * of PIDs associated with @appid, empty if not running.
399 */399 */
400GList * ubuntu_app_launch_get_pids (const gchar * appid);400GList * ubuntu_app_launch_get_pids (const gchar * appid);

Subscribers

People subscribed via source and target branches