From 6409ce6bb646e2c73362771b5e5e172de42a5a59 Mon Sep 17 00:00:00 2001 From: HP van Braam Date: Fri, 17 Apr 2026 11:57:22 +0200 Subject: [PATCH] Fix race in RefCounted::unreference() When two threads unreference() a RefCounted Object at the same time there's a potential race. Thread A sees rc_val == 1, enters the if() block. Thread B sees rc_val == 0, enters the if() block. Thread A gets scheduled out, or is simply scheduled on a slower core and Thread B finishes, returning die = true. Thread B then frees the Object. Some time during or after ~Object() Thread A wakes up and tries to call methods on the Object, which is either destroyed or in the process of being destroyed, leading to a use-after-free. We fix the problem by counting how many threads are currently inside the critical section, and blocking returning die = true as long as there are still other threads potentially alive. Tested by running with --test and by opening and playing various public and private Godot projects. This fixes #115173 --- core/object/ref_counted.cpp | 18 ++++++++++++++++++ core/object/ref_counted.h | 1 + 2 files changed, 19 insertions(+) diff --git a/core/object/ref_counted.cpp b/core/object/ref_counted.cpp index d63d8126dcc..374b0b381b1 100644 --- a/core/object/ref_counted.cpp +++ b/core/object/ref_counted.cpp @@ -90,6 +90,7 @@ bool RefCounted::reference() { } bool RefCounted::unreference() { + dereference_count.increment(); uint32_t rc_val = refcount.unrefval(); bool die = rc_val == 0; @@ -106,6 +107,22 @@ bool RefCounted::unreference() { die = die && binding_ret; } + dereference_count.decrement(); + + // If we are going to be destroyed we need to ensure that no other thread + // is still inside our critical section. If they are they might see + // a (partially) destroyed Object for get_script_instance, _get_extension, + // or _instance_binding_reference. + if (die) { + // It is unlikely that we will spin here for very long. + // Only threads that see die == true will spin, which should only + // ever be one. Only threads seeing rc_val == 1 and rc_val == 0 + // will do anything at all in the critical section. + while (dereference_count.get()) { + // Spin + } + } + return die; } @@ -114,6 +131,7 @@ RefCounted::RefCounted() : _define_ancestry(AncestralClass::REF_COUNTED); refcount.init(); refcount_init.init(); + dereference_count.set(0); } Variant WeakRef::get_ref() const { diff --git a/core/object/ref_counted.h b/core/object/ref_counted.h index 689a0f267c3..0eab2fafec3 100644 --- a/core/object/ref_counted.h +++ b/core/object/ref_counted.h @@ -37,6 +37,7 @@ class RefCounted : public Object { GDCLASS(RefCounted, Object); SafeRefCount refcount; SafeRefCount refcount_init; + SafeNumeric dereference_count; protected: static void _bind_methods();