Refactor ref-counting code and fix ref counted releasing before aquiring

1 files changed, 65 insertions, 67 deletions

diff --git a/core/variant/variant.cpp b/core/variant/variant.cpp
index 186643b024..e2865a06be 100644
--- a/core/variant/variant.cpp
+++ b/core/variant/variant.cpp
@@ -1072,17 +1072,69 @@ bool Variant::is_null() const {
 	}
 }
 
+void Variant::ObjData::ref(const ObjData &p_from) {
+	// Mirrors Ref::ref in refcounted.h
+	if (p_from.id == id) {
+		return;
+	}
+
+	ObjData cleanup_ref = *this;
+
+	*this = p_from;
+	if (id.is_ref_counted()) {
+		RefCounted *reference = static_cast<RefCounted *>(obj);
+		// Assuming reference is not null because id.is_ref_counted() was true.
+		if (!reference->reference()) {
+			*this = ObjData();
+		}
+	}
+
+	cleanup_ref.unref();
+}
+
+void Variant::ObjData::ref_pointer(Object *p_object) {
+	// Mirrors Ref::ref_pointer in refcounted.h
+	if (p_object == obj) {
+		return;
+	}
+
+	ObjData cleanup_ref = *this;
+
+	if (p_object) {
+		*this = ObjData{ p_object->get_instance_id(), p_object };
+		if (p_object->is_ref_counted()) {
+			RefCounted *reference = static_cast<RefCounted *>(p_object);
+			if (!reference->init_ref()) {
+				*this = ObjData();
+			}
+		}
+	} else {
+		*this = ObjData();
+	}
+
+	cleanup_ref.unref();
+}
+
+void Variant::ObjData::unref() {
+	// Mirrors Ref::unref in refcounted.h
+	if (id.is_ref_counted()) {
+		RefCounted *reference = static_cast<RefCounted *>(obj);
+		// Assuming reference is not null because id.is_ref_counted() was true.
+		if (reference->unreference()) {
+			memdelete(reference);
+		}
+	}
+	*this = ObjData();
+}
+
 void Variant::reference(const Variant &p_variant) {
-	switch (type) {
-		case NIL:
-		case BOOL:
-		case INT:
-		case FLOAT:
-			break;
-		default:
-			clear();
+	if (type == OBJECT && p_variant.type == OBJECT) {
+		_get_obj().ref(p_variant._get_obj());
+		return;
 	}
 
+	clear();
+
 	type = p_variant.type;
 
 	switch (p_variant.type) {
@@ -1165,18 +1217,7 @@ void Variant::reference(const Variant &p_variant) {
 		} break;
 		case OBJECT: {
 			memnew_placement(_data._mem, ObjData);
-
-			if (p_variant._get_obj().obj && p_variant._get_obj().id.is_ref_counted()) {
-				RefCounted *ref_counted = static_cast<RefCounted *>(p_variant._get_obj().obj);
-				if (!ref_counted->reference()) {
-					_get_obj().obj = nullptr;
-					_get_obj().id = ObjectID();
-					break;
-				}
-			}
-
-			_get_obj().obj = const_cast<Object *>(p_variant._get_obj().obj);
-			_get_obj().id = p_variant._get_obj().id;
+			_get_obj().ref(p_variant._get_obj());
 		} break;
 		case CALLABLE: {
 			memnew_placement(_data._mem, Callable(*reinterpret_cast<const Callable *>(p_variant._data._mem)));
@@ -1375,15 +1416,7 @@ void Variant::_clear_internal() {
 			reinterpret_cast<NodePath *>(_data._mem)->~NodePath();
 		} break;
 		case OBJECT: {
-			if (_get_obj().id.is_ref_counted()) {
-				// We are safe that there is a reference here.
-				RefCounted *ref_counted = static_cast<RefCounted *>(_get_obj().obj);
-				if (ref_counted->unreference()) {
-					memdelete(ref_counted);
-				}
-			}
-			_get_obj().obj = nullptr;
-			_get_obj().id = ObjectID();
+			_get_obj().unref();
 		} break;
 		case RID: {
 			// Not much need probably.
@@ -2589,24 +2622,8 @@ Variant::Variant(const ::RID &p_rid) :
 
 Variant::Variant(const Object *p_object) :
 		type(OBJECT) {
-	memnew_placement(_data._mem, ObjData);
-
-	if (p_object) {
-		if (p_object->is_ref_counted()) {
-			RefCounted *ref_counted = const_cast<RefCounted *>(static_cast<const RefCounted *>(p_object));
-			if (!ref_counted->init_ref()) {
-				_get_obj().obj = nullptr;
-				_get_obj().id = ObjectID();
-				return;
-			}
-		}
-
-		_get_obj().obj = const_cast<Object *>(p_object);
-		_get_obj().id = p_object->get_instance_id();
-	} else {
-		_get_obj().obj = nullptr;
-		_get_obj().id = ObjectID();
-	}
+	_get_obj() = ObjData();
+	_get_obj().ref_pointer(const_cast<Object *>(p_object));
 }
 
 Variant::Variant(const Callable &p_callable) :
@@ -2828,26 +2845,7 @@ void Variant::operator=(const Variant &p_variant) {
 			*reinterpret_cast<::RID *>(_data._mem) = *reinterpret_cast<const ::RID *>(p_variant._data._mem);
 		} break;
 		case OBJECT: {
-			if (_get_obj().id.is_ref_counted()) {
-				//we are safe that there is a reference here
-				RefCounted *ref_counted = static_cast<RefCounted *>(_get_obj().obj);
-				if (ref_counted->unreference()) {
-					memdelete(ref_counted);
-				}
-			}
-
-			if (p_variant._get_obj().obj && p_variant._get_obj().id.is_ref_counted()) {
-				RefCounted *ref_counted = static_cast<RefCounted *>(p_variant._get_obj().obj);
-				if (!ref_counted->reference()) {
-					_get_obj().obj = nullptr;
-					_get_obj().id = ObjectID();
-					break;
-				}
-			}
-
-			_get_obj().obj = const_cast<Object *>(p_variant._get_obj().obj);
-			_get_obj().id = p_variant._get_obj().id;
-
+			_get_obj().ref(p_variant._get_obj());
 		} break;
 		case CALLABLE: {
 			*reinterpret_cast<Callable *>(_data._mem) = *reinterpret_cast<const Callable *>(p_variant._data._mem);