From a21aadc2b59508f2a5014a14e9f310fdc2204a8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Andr=C3=A9=20Vadla=20Ravn=C3=A5s?= Date: Thu, 14 Mar 2024 12:24:45 +0100 Subject: [PATCH] base: Fix racy crash in custom GSource implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When thread A is in a GSource finalizer blocked on unregister_source(), meaning its ref count is zero, thread B would be walking the Gee.Map of sources to potentially update their ready time. Thread B would be doing this with the recursive lock held, but since the loop had an owned GSource variable, it would result in each source being reffed and subsequently unreffed. This meant that the ref count would once again go from one down to zero, triggering a second call to the finalizer. This would in turn call unregister_source(), instantly acquire the recursive lock a second time, and mutate the Map that is currently being iterated. So to avoid all of this we make sure that we use an unowned variable for the source while iterating. The alternative would have been to make a copy of the Map before walking it, but that's unnecessary overhead, along with the redundant ref count updates it would entail. Co-authored-by: Håvard Sørbø --- lib/base/p2p.vala | 4 ++-- lib/base/socket.vala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/base/p2p.vala b/lib/base/p2p.vala index fcf933714..a2ba647fc 100644 --- a/lib/base/p2p.vala +++ b/lib/base/p2p.vala @@ -447,7 +447,7 @@ namespace Frida { _pending_io = condition; foreach (var entry in sources.entries) { - Source source = entry.key; + unowned Source source = entry.key; IOCondition c = entry.value; if ((_pending_io & c) != 0) source.set_ready_time (0); @@ -825,7 +825,7 @@ namespace Frida { sctp_events = new_events; foreach (var entry in sources.entries) { - Source source = entry.key; + unowned Source source = entry.key; IOCondition c = entry.value; if ((new_events & c) != 0) source.set_ready_time (0); diff --git a/lib/base/socket.vala b/lib/base/socket.vala index 0730ce7c5..d2f4e8bd9 100644 --- a/lib/base/socket.vala +++ b/lib/base/socket.vala @@ -848,7 +848,7 @@ namespace Frida { _pending_io = new_io; foreach (var entry in sources.entries) { - Source source = entry.key; + unowned Source source = entry.key; IOCondition c = entry.value; if ((new_io & c) != 0) source.set_ready_time (0);