Skip to content

Commit d89d53f

Browse files
Daniel Gomezkraj
authored andcommitted
glib-2.0: Backport GMainContext fixes
Backport fixes introduced in 2.63.6 for memory leaks and memory corruption in GMainContext Upstream merge: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1353 Fixes SIGSEGV in GStreamer: Thread 2 "multihandlesink" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7ffff6bb9700 (LWP 18045)] 0x00007ffff7d65992 in g_source_unref_internal (source=0x7ffff00047d0, context=0x55555561c800, have_lock=1) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c:2146 2146 ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c: No such file or directory. (gdb) bt #0 0x00007ffff7d65992 in g_source_unref_internal (source=0x7ffff00047d0, context=0x55555561c800, have_lock=1) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c:2146 #1 0x00007ffff7d65bb6 in g_source_iter_next (iter=iter@entry=0x7ffff6bb8db0, source=source@entry=0x7ffff6bb8da8) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c:980 #2 0x00007ffff7d67ef3 in g_main_context_prepare (context=context@entry=0x55555561c800, priority=priority@entry=0x7ffff6bb8e30) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c:944 #3 0x00007ffff7d6896b in g_main_context_iterate (context=context@entry=0x55555561c800, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c:3900 #4 0x00007ffff7d68b4c in g_main_context_iteration (context=0x55555561c800, may_block=may_block@entry=1) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gmain.c:3981 #5 0x00007ffff6be4482 in gst_multi_socket_sink_thread (mhsink=0x555555679ab0 [GstMultiSocketSink]) at ../../../gst-plugins-base-1.14.4/gst/tcp/gstmultisocketsink.c:1164 #6 0x00007ffff7d8fb35 in g_thread_proxy (data=0x55555565c770) at ../../../../../../../repo/workspace/sources/glib-2.0/glib/gthread.c:784 #7 0x00007ffff7841ebd in start_thread (arg=<optimized out>) at pthread_create.c:486 #8 0x00007ffff7aa12bf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 #8 0x00007ffff7aa12bf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Signed-off-by: Daniel Gomez <daniel@qtec.com> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
1 parent a9d2af8 commit d89d53f

4 files changed

Lines changed: 191 additions & 0 deletions
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
From ef2be42998e3fc10299055a5a01f7c791538174c Mon Sep 17 00:00:00 2001
2+
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
3+
Date: Mon, 3 Feb 2020 15:38:28 +0200
4+
Subject: [PATCH] GMainContext - Fix GSource iterator if iteration can modify
5+
the list
6+
7+
We first have to ref the next source and then unref the previous one.
8+
This might be the last reference to the previous source, and freeing the
9+
previous source might unref and free the next one which would then leave
10+
use with a dangling pointer here.
11+
12+
Fixes https://gitlab.gnome.org/GNOME/glib/issues/2031
13+
14+
Upstream-Status: Backport [https://gitlab.gnome.org/GNOME/glib/-/commit/b06c48de7554607ff3fb58d6c0510cfa5088e909]
15+
16+
---
17+
glib/gmain.c | 8 ++++++--
18+
1 file changed, 6 insertions(+), 2 deletions(-)
19+
20+
diff --git a/glib/gmain.c b/glib/gmain.c
21+
index af979c8..a9a287d 100644
22+
--- a/glib/gmain.c
23+
+++ b/glib/gmain.c
24+
@@ -969,13 +969,17 @@ g_source_iter_next (GSourceIter *iter, GSource **source)
25+
* GSourceList to be removed from source_lists (if iter->source is
26+
* the only source in its list, and it is destroyed), so we have to
27+
* keep it reffed until after we advance iter->current_list, above.
28+
+ *
29+
+ * Also we first have to ref the next source before unreffing the
30+
+ * previous one as unreffing the previous source can potentially
31+
+ * free the next one.
32+
*/
33+
+ if (next_source && iter->may_modify)
34+
+ g_source_ref (next_source);
35+
36+
if (iter->source && iter->may_modify)
37+
g_source_unref_internal (iter->source, iter->context, TRUE);
38+
iter->source = next_source;
39+
- if (iter->source && iter->may_modify)
40+
- g_source_ref (iter->source);
41+
42+
*source = iter->source;
43+
return *source != NULL;
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
From 611430a32a46d0dc806a829161e2dccf9c0196a8 Mon Sep 17 00:00:00 2001
2+
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
3+
Date: Mon, 3 Feb 2020 15:35:51 +0200
4+
Subject: [PATCH] GMainContext - Fix memory leaks and memory corruption when
5+
freeing sources while freeing a context
6+
7+
Instead of destroying sources directly while freeing the context, and
8+
potentially freeing them if this was the last reference to them, collect
9+
new references of all sources in a separate list before and at the same
10+
time invalidate their context so that they can't access it anymore. Only
11+
once all sources have their context invalidated, destroy them while
12+
still keeping a reference to them. Once all sources are destroyed we get
13+
rid of the additional references and free them if nothing else keeps a
14+
reference to them anymore.
15+
16+
This fixes a regression introduced by 26056558be in 2012.
17+
18+
The previous code that invalidated the context of each source and then
19+
destroyed it before going to the next source without keeping an
20+
additional reference caused memory leaks or memory corruption depending
21+
on the order of the sources in the sources lists.
22+
23+
If a source was destroyed it might happen that this was the last
24+
reference to this source, and it would then be freed. This would cause
25+
the finalize function to be called, which might destroy and unref
26+
another source and potentially free it. This other source would then
27+
either
28+
- go through the normal free logic and change the intern linked list
29+
between the sources, while other sources that are unreffed as part of
30+
the main context freeing would not. As such the list would be in an
31+
inconsistent state and we might dereference freed memory.
32+
- go through the normal destroy and free logic but because the context
33+
pointer was already invalidated it would simply mark the source as
34+
destroyed without actually removing it from the context. This would
35+
then cause a memory leak because the reference owned by the context is
36+
not freed.
37+
38+
Fixes https://github.com/gtk-rs/glib/issues/583 while still keeping
39+
https://bugzilla.gnome.org/show_bug.cgi?id=661767 fixes.
40+
41+
Upstream-Status: Backport [https://gitlab.gnome.org/GNOME/glib/-/commit/aa20167d419c649f34fed06a9463890b41b1eba0]
42+
43+
---
44+
glib/gmain.c | 35 ++++++++++++++++++++++++++++++++++-
45+
1 file changed, 34 insertions(+), 1 deletion(-)
46+
47+
diff --git a/glib/gmain.c b/glib/gmain.c
48+
index a9a287d..10ba2f8 100644
49+
--- a/glib/gmain.c
50+
+++ b/glib/gmain.c
51+
@@ -538,6 +538,7 @@ g_main_context_unref (GMainContext *context)
52+
GSourceIter iter;
53+
GSource *source;
54+
GList *sl_iter;
55+
+ GSList *s_iter, *remaining_sources = NULL;
56+
GSourceList *list;
57+
guint i;
58+
59+
@@ -557,10 +558,30 @@ g_main_context_unref (GMainContext *context)
60+
61+
/* g_source_iter_next() assumes the context is locked. */
62+
LOCK_CONTEXT (context);
63+
- g_source_iter_init (&iter, context, TRUE);
64+
+
65+
+ /* First collect all remaining sources from the sources lists and store a
66+
+ * new reference in a separate list. Also set the context of the sources
67+
+ * to NULL so that they can't access a partially destroyed context anymore.
68+
+ *
69+
+ * We have to do this first so that we have a strong reference to all
70+
+ * sources and destroying them below does not also free them, and so that
71+
+ * none of the sources can access the context from their finalize/dispose
72+
+ * functions. */
73+
+ g_source_iter_init (&iter, context, FALSE);
74+
while (g_source_iter_next (&iter, &source))
75+
{
76+
source->context = NULL;
77+
+ remaining_sources = g_slist_prepend (remaining_sources, g_source_ref (source));
78+
+ }
79+
+ g_source_iter_clear (&iter);
80+
+
81+
+ /* Next destroy all sources. As we still hold a reference to all of them,
82+
+ * this won't cause any of them to be freed yet and especially prevents any
83+
+ * source that unrefs another source from its finalize function to be freed.
84+
+ */
85+
+ for (s_iter = remaining_sources; s_iter; s_iter = s_iter->next)
86+
+ {
87+
+ source = s_iter->data;
88+
g_source_destroy_internal (source, context, TRUE);
89+
}
90+
UNLOCK_CONTEXT (context);
91+
@@ -585,6 +606,18 @@ g_main_context_unref (GMainContext *context)
92+
g_cond_clear (&context->cond);
93+
94+
g_free (context);
95+
+
96+
+ /* And now finally get rid of our references to the sources. This will cause
97+
+ * them to be freed unless something else still has a reference to them. Due
98+
+ * to setting the context pointers in the sources to NULL above, this won't
99+
+ * ever access the context or the internal linked list inside the GSource.
100+
+ * We already removed the sources completely from the context above. */
101+
+ for (s_iter = remaining_sources; s_iter; s_iter = s_iter->next)
102+
+ {
103+
+ source = s_iter->data;
104+
+ g_source_unref_internal (source, NULL, FALSE);
105+
+ }
106+
+ g_slist_free (remaining_sources);
107+
}
108+
109+
/* Helper function used by mainloop/overflow test.
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
From 3e9d85f1b75e2b1096d9643563d7d17380752fc7 Mon Sep 17 00:00:00 2001
2+
From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= <sebastian@centricular.com>
3+
Date: Tue, 11 Feb 2020 09:34:38 +0200
4+
Subject: [PATCH] GMainContext - Move mutex unlocking in destructor right
5+
before freeing the mutex
6+
7+
This does not have any behaviour changes but is cleaner. The mutex is
8+
only unlocked now after all operations on the context are done and right
9+
before freeing the mutex and the context itself.
10+
11+
Upstream-Status: Backport [https://gitlab.gnome.org/GNOME/glib/-/commit/730a75fc8e8271c38fbd5363d1f77a00876b9ddc]
12+
13+
---
14+
glib/gmain.c | 2 +-
15+
1 file changed, 1 insertion(+), 1 deletion(-)
16+
17+
diff --git a/glib/gmain.c b/glib/gmain.c
18+
index 10ba2f8..b1df470 100644
19+
--- a/glib/gmain.c
20+
+++ b/glib/gmain.c
21+
@@ -584,7 +584,6 @@ g_main_context_unref (GMainContext *context)
22+
source = s_iter->data;
23+
g_source_destroy_internal (source, context, TRUE);
24+
}
25+
- UNLOCK_CONTEXT (context);
26+
27+
for (sl_iter = context->source_lists; sl_iter; sl_iter = sl_iter->next)
28+
{
29+
@@ -595,6 +594,7 @@ g_main_context_unref (GMainContext *context)
30+
31+
g_hash_table_destroy (context->sources);
32+
33+
+ UNLOCK_CONTEXT (context);
34+
g_mutex_clear (&context->mutex);
35+
36+
g_ptr_array_free (context->pending_dispatches, TRUE);

meta/recipes-core/glib-2.0/glib-2.0_2.62.4.bb

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ SRC_URI = "${GNOME_MIRROR}/glib/${SHRT_VER}/glib-${PV}.tar.xz \
1616
file://0001-Do-not-write-bindir-into-pkg-config-files.patch \
1717
file://0001-meson-Run-atomics-test-on-clang-as-well.patch \
1818
file://0001-gio-tests-resources.c-comment-out-a-build-host-only-.patch \
19+
file://0011-GMainContext-Fix-GSource-iterator-if-iteration-can-m.patch \
20+
file://0012-GMainContext-Fix-memory-leaks-and-memory-corruption-.patch \
21+
file://0013-GMainContext-Move-mutex-unlocking-in-destructor-righ.patch \
1922
"
2023

2124
SRC_URI_append_class-native = " file://relocate-modules.patch"

0 commit comments

Comments
 (0)