[glib: 2/5] GMainContext - Fix memory leaks and memory corruption when freeing sources while freeing a context

[Philip Withnall <pwithnall src gnome org>]

commit aa20167d419c649f34fed06a9463890b41b1eba0
Author: Sebastian Dröge <sebastian centricular com>
Date:   Mon Feb 3 15:35:51 2020 +0200

    GMainContext - Fix memory leaks and memory corruption when freeing sources while freeing a context
    
    Instead of destroying sources directly while freeing the context, and
    potentially freeing them if this was the last reference to them, collect
    new references of all sources in a separate list before and at the same
    time invalidate their context so that they can't access it anymore. Only
    once all sources have their context invalidated, destroy them while
    still keeping a reference to them. Once all sources are destroyed we get
    rid of the additional references and free them if nothing else keeps a
    reference to them anymore.
    
    This fixes a regression introduced by 26056558be in 2012.
    
    The previous code that invalidated the context of each source and then
    destroyed it before going to the next source without keeping an
    additional reference caused memory leaks or memory corruption depending
    on the order of the sources in the sources lists.
    
    If a source was destroyed it might happen that this was the last
    reference to this source, and it would then be freed. This would cause
    the finalize function to be called, which might destroy and unref
    another source and potentially free it. This other source would then
    either
    - go through the normal free logic and change the intern linked list
      between the sources, while other sources that are unreffed as part of
      the main context freeing would not. As such the list would be in an
      inconsistent state and we might dereference freed memory.
    - go through the normal destroy and free logic but because the context
      pointer was already invalidated it would simply mark the source as
      destroyed without actually removing it from the context. This would
      then cause a memory leak because the reference owned by the context is
      not freed.
    
    Fixes https://github.com/gtk-rs/glib/issues/583 while still keeping
    https://bugzilla.gnome.org/show_bug.cgi?id=661767 fixes.

 glib/gmain.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)
---
diff --git a/glib/gmain.c b/glib/gmain.c
index 0e65aac14..41f1370d7 100644
--- a/glib/gmain.c
+++ b/glib/gmain.c
@@ -539,6 +539,7 @@ g_main_context_unref (GMainContext *context)
   GSourceIter iter;
   GSource *source;
   GList *sl_iter;
+  GSList *s_iter, *remaining_sources = NULL;
   GSourceList *list;
   guint i;
 
@@ -558,10 +559,30 @@ g_main_context_unref (GMainContext *context)
 
   /* g_source_iter_next() assumes the context is locked. */
   LOCK_CONTEXT (context);
-  g_source_iter_init (&iter, context, TRUE);
+
+  /* First collect all remaining sources from the sources lists and store a
+   * new reference in a separate list. Also set the context of the sources
+   * to NULL so that they can't access a partially destroyed context anymore.
+   *
+   * We have to do this first so that we have a strong reference to all
+   * sources and destroying them below does not also free them, and so that
+   * none of the sources can access the context from their finalize/dispose
+   * functions. */
+  g_source_iter_init (&iter, context, FALSE);
   while (g_source_iter_next (&iter, &source))
     {
       source->context = NULL;
+      remaining_sources = g_slist_prepend (remaining_sources, g_source_ref (source));
+    }
+  g_source_iter_clear (&iter);
+
+  /* Next destroy all sources. As we still hold a reference to all of them,
+   * this won't cause any of them to be freed yet and especially prevents any
+   * source that unrefs another source from its finalize function to be freed.
+   */
+  for (s_iter = remaining_sources; s_iter; s_iter = s_iter->next)
+    {
+      source = s_iter->data;
       g_source_destroy_internal (source, context, TRUE);
     }
   UNLOCK_CONTEXT (context);
@@ -586,6 +607,18 @@ g_main_context_unref (GMainContext *context)
   g_cond_clear (&context->cond);
 
   g_free (context);
+
+  /* And now finally get rid of our references to the sources. This will cause
+   * them to be freed unless something else still has a reference to them. Due
+   * to setting the context pointers in the sources to NULL above, this won't
+   * ever access the context or the internal linked list inside the GSource.
+   * We already removed the sources completely from the context above. */
+  for (s_iter = remaining_sources; s_iter; s_iter = s_iter->next)
+    {
+      source = s_iter->data;
+      g_source_unref_internal (source, NULL, FALSE);
+    }
+  g_slist_free (remaining_sources);
 }
 
 /* Helper function used by mainloop/overflow test.