Skip to content

Commit

Permalink
base: Fix racy crash in custom GSource implementations
Browse files Browse the repository at this point in the history
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ø <[email protected]>
  • Loading branch information
oleavr and hsorbo committed Mar 14, 2024
1 parent 6c753d4 commit a21aadc
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 3 deletions.
4 changes: 2 additions & 2 deletions lib/base/p2p.vala
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion lib/base/socket.vala
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit a21aadc

Please sign in to comment.