|
|
2ca4f1 |
From fc051ec83d8894dd754bf364562ba9be9ff999fc Mon Sep 17 00:00:00 2001
|
|
|
2ca4f1 |
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
|
|
|
2ca4f1 |
Date: Mon, 3 Feb 2020 15:35:51 +0200
|
|
|
2ca4f1 |
Subject: [PATCH 3/5] GMainContext - Fix memory leaks and memory corruption
|
|
|
2ca4f1 |
when freeing sources while freeing a context
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
Instead of destroying sources directly while freeing the context, and
|
|
|
2ca4f1 |
potentially freeing them if this was the last reference to them, collect
|
|
|
2ca4f1 |
new references of all sources in a separate list before and at the same
|
|
|
2ca4f1 |
time invalidate their context so that they can't access it anymore. Only
|
|
|
2ca4f1 |
once all sources have their context invalidated, destroy them while
|
|
|
2ca4f1 |
still keeping a reference to them. Once all sources are destroyed we get
|
|
|
2ca4f1 |
rid of the additional references and free them if nothing else keeps a
|
|
|
2ca4f1 |
reference to them anymore.
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
This fixes a regression introduced by 26056558be in 2012.
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
The previous code that invalidated the context of each source and then
|
|
|
2ca4f1 |
destroyed it before going to the next source without keeping an
|
|
|
2ca4f1 |
additional reference caused memory leaks or memory corruption depending
|
|
|
2ca4f1 |
on the order of the sources in the sources lists.
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
If a source was destroyed it might happen that this was the last
|
|
|
2ca4f1 |
reference to this source, and it would then be freed. This would cause
|
|
|
2ca4f1 |
the finalize function to be called, which might destroy and unref
|
|
|
2ca4f1 |
another source and potentially free it. This other source would then
|
|
|
2ca4f1 |
either
|
|
|
2ca4f1 |
- go through the normal free logic and change the intern linked list
|
|
|
2ca4f1 |
between the sources, while other sources that are unreffed as part of
|
|
|
2ca4f1 |
the main context freeing would not. As such the list would be in an
|
|
|
2ca4f1 |
inconsistent state and we might dereference freed memory.
|
|
|
2ca4f1 |
- go through the normal destroy and free logic but because the context
|
|
|
2ca4f1 |
pointer was already invalidated it would simply mark the source as
|
|
|
2ca4f1 |
destroyed without actually removing it from the context. This would
|
|
|
2ca4f1 |
then cause a memory leak because the reference owned by the context is
|
|
|
2ca4f1 |
not freed.
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
Fixes https://github.com/gtk-rs/glib/issues/583 while still keeping
|
|
|
2ca4f1 |
https://bugzilla.gnome.org/show_bug.cgi?id=661767 fixes.
|
|
|
2ca4f1 |
---
|
|
|
2ca4f1 |
glib/gmain.c | 35 ++++++++++++++++++++++++++++++++++-
|
|
|
2ca4f1 |
1 file changed, 34 insertions(+), 1 deletion(-)
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
diff --git a/glib/gmain.c b/glib/gmain.c
|
|
|
2ca4f1 |
index a3ea1d36c..1c249ad02 100644
|
|
|
2ca4f1 |
--- a/glib/gmain.c
|
|
|
2ca4f1 |
+++ b/glib/gmain.c
|
|
|
2ca4f1 |
@@ -534,6 +534,7 @@ g_main_context_unref (GMainContext *context)
|
|
|
2ca4f1 |
GSourceIter iter;
|
|
|
2ca4f1 |
GSource *source;
|
|
|
2ca4f1 |
GList *sl_iter;
|
|
|
2ca4f1 |
+ GSList *s_iter, *remaining_sources = NULL;
|
|
|
2ca4f1 |
GSourceList *list;
|
|
|
2ca4f1 |
guint i;
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
@@ -553,10 +554,30 @@ g_main_context_unref (GMainContext *context)
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
/* g_source_iter_next() assumes the context is locked. */
|
|
|
2ca4f1 |
LOCK_CONTEXT (context);
|
|
|
2ca4f1 |
- g_source_iter_init (&iter, context, TRUE);
|
|
|
2ca4f1 |
+
|
|
|
2ca4f1 |
+ /* First collect all remaining sources from the sources lists and store a
|
|
|
2ca4f1 |
+ * new reference in a separate list. Also set the context of the sources
|
|
|
2ca4f1 |
+ * to NULL so that they can't access a partially destroyed context anymore.
|
|
|
2ca4f1 |
+ *
|
|
|
2ca4f1 |
+ * We have to do this first so that we have a strong reference to all
|
|
|
2ca4f1 |
+ * sources and destroying them below does not also free them, and so that
|
|
|
2ca4f1 |
+ * none of the sources can access the context from their finalize/dispose
|
|
|
2ca4f1 |
+ * functions. */
|
|
|
2ca4f1 |
+ g_source_iter_init (&iter, context, FALSE);
|
|
|
2ca4f1 |
while (g_source_iter_next (&iter, &source))
|
|
|
2ca4f1 |
{
|
|
|
2ca4f1 |
source->context = NULL;
|
|
|
2ca4f1 |
+ remaining_sources = g_slist_prepend (remaining_sources, g_source_ref (source));
|
|
|
2ca4f1 |
+ }
|
|
|
2ca4f1 |
+ g_source_iter_clear (&iter);
|
|
|
2ca4f1 |
+
|
|
|
2ca4f1 |
+ /* Next destroy all sources. As we still hold a reference to all of them,
|
|
|
2ca4f1 |
+ * this won't cause any of them to be freed yet and especially prevents any
|
|
|
2ca4f1 |
+ * source that unrefs another source from its finalize function to be freed.
|
|
|
2ca4f1 |
+ */
|
|
|
2ca4f1 |
+ for (s_iter = remaining_sources; s_iter; s_iter = s_iter->next)
|
|
|
2ca4f1 |
+ {
|
|
|
2ca4f1 |
+ source = s_iter->data;
|
|
|
2ca4f1 |
g_source_destroy_internal (source, context, TRUE);
|
|
|
2ca4f1 |
}
|
|
|
2ca4f1 |
UNLOCK_CONTEXT (context);
|
|
|
2ca4f1 |
@@ -581,6 +602,18 @@ g_main_context_unref (GMainContext *context)
|
|
|
2ca4f1 |
g_cond_clear (&context->cond);
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
g_free (context);
|
|
|
2ca4f1 |
+
|
|
|
2ca4f1 |
+ /* And now finally get rid of our references to the sources. This will cause
|
|
|
2ca4f1 |
+ * them to be freed unless something else still has a reference to them. Due
|
|
|
2ca4f1 |
+ * to setting the context pointers in the sources to NULL above, this won't
|
|
|
2ca4f1 |
+ * ever access the context or the internal linked list inside the GSource.
|
|
|
2ca4f1 |
+ * We already removed the sources completely from the context above. */
|
|
|
2ca4f1 |
+ for (s_iter = remaining_sources; s_iter; s_iter = s_iter->next)
|
|
|
2ca4f1 |
+ {
|
|
|
2ca4f1 |
+ source = s_iter->data;
|
|
|
2ca4f1 |
+ g_source_unref_internal (source, NULL, FALSE);
|
|
|
2ca4f1 |
+ }
|
|
|
2ca4f1 |
+ g_slist_free (remaining_sources);
|
|
|
2ca4f1 |
}
|
|
|
2ca4f1 |
|
|
|
2ca4f1 |
/* Helper function used by mainloop/overflow test.
|
|
|
2ca4f1 |
--
|
|
|
2ca4f1 |
2.31.1
|
|
|
2ca4f1 |
|