Merge lp:~albaguirre/mir/fix-1480420 into lp:mir

[Alberto Aguirre]

Avoid calling g_source_get_context when the source is already destroyed.

If the main context has been deleted that marks the source as destroyed.
This may happen in Alarms that outlive the mainloop.

Avoid calling g_source_get_context when the source is already destroyed.

If the main context has been deleted that marks the source as destroyed.
This may happen in Alarms that outlive the mainloop which is the case in some test setups.

This can explain the failure in lp:1480422 as well, as before g_source_get_context could assert leading to FD leaks.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2801
http://jenkins.qa.ubuntu.com/job/mir-ci/4454/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3372
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/225
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/897
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3320
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/603
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/603/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3320
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3320/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/6111
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/22260

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4454/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Sounds reasonable.

review: Approve

[Alan Griffiths (alan-griffiths)]

Plausible

review: Approve

=== modified file 'src/server/glib_main_loop_sources.cpp'
--- src/server/glib_main_loop_sources.cpp	2015-06-17 05:20:42 +0000
+++ src/server/glib_main_loop_sources.cpp	2015-07-31 19:25:59 +0000
@@ -106,9 +106,9 @@
     if (gsource)
     {
         pre_destruction_hook(gsource);
+
+#ifdef GLIB_HAS_FIXED_LP_1401488
         g_source_destroy(gsource);
-
-#ifdef GLIB_HAS_FIXED_LP_1401488
         g_source_unref(gsource);
 #else
         /*
@@ -118,13 +118,22 @@
          * and so the main loop might be mid-iteration of the same source
          * making callbacks. And glib lacks protection to prevent sources
          * getting fully free()'d mid-callback (TODO: fix glib?). So we defer
-         * the final unref of the source to a point in the loop when it's safe:
+         * the final unref of the source to a point in the loop when it's safe.
          */
-        auto main_context = g_source_get_context(gsource);
-        auto idler = g_idle_source_new();
-        g_source_set_callback(idler, idle_callback, gsource, destroy_idler);
-        g_source_attach(idler, main_context);
-        g_source_unref(idler);
+        if (!g_source_is_destroyed(gsource))
+        {
+            auto main_context = g_source_get_context(gsource);
+            g_source_destroy(gsource);
+            auto idler = g_idle_source_new();
+            g_source_set_callback(idler, idle_callback, gsource, destroy_idler);
+            g_source_attach(idler, main_context);
+            g_source_unref(idler);
+        }
+        else
+        {
+            // The source is already destroyed so it's safe to unref now
+            g_source_unref(gsource);
+        }
 #endif
     }
 }