Merge pull request #118678 from hpvb/fix-115173

Fix race in `RefCounted::unreference()`

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 {

core/object/ref_counted.h

@@ -37,6 +37,7 @@ class RefCounted : public Object {
 	GDCLASS(RefCounted, Object);
 	SafeRefCount refcount;
 	SafeRefCount refcount_init;
+	SafeNumeric<uint32_t> dereference_count;
 
 protected:
 	static void _bind_methods();