From 8f3e2c96eb91027ea050c1e2aacf2cd0e1bf35d9 Mon Sep 17 00:00:00 2001
From: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com>
Date: Tue, 2 Apr 2024 15:49:56 +0200
Subject: [PATCH] [Core] Fix `Variant::construct` of `Object`

Variant type was not updated correctly causing leaks in ref-counted
---
 core/variant/variant_construct.h                      |  8 +++++++-
 .../runtime/errors/invalid_property_assignment.gd     |  9 +++++++++
 .../runtime/errors/invalid_property_assignment.out    |  6 ++++++
 .../scripts/runtime/features/set_does_not_leak.gd     | 11 +++++++++++
 .../scripts/runtime/features/set_does_not_leak.out    |  1 +
 5 files changed, 34 insertions(+), 1 deletion(-)
 create mode 100644 modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.gd
 create mode 100644 modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.out
 create mode 100644 modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.gd
 create mode 100644 modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.out

diff --git a/core/variant/variant_construct.h b/core/variant/variant_construct.h
index b824044b82da..5afdb884f66d 100644
--- a/core/variant/variant_construct.h
+++ b/core/variant/variant_construct.h
@@ -153,11 +153,14 @@ class VariantConstructor {
 class VariantConstructorObject {
 public:
 	static void construct(Variant &r_ret, const Variant **p_args, Callable::CallError &r_error) {
-		VariantInternal::clear(&r_ret);
 		if (p_args[0]->get_type() == Variant::NIL) {
+			VariantInternal::clear(&r_ret);
+			VariantTypeChanger<Object *>::change(&r_ret);
 			VariantInternal::object_assign_null(&r_ret);
 			r_error.error = Callable::CallError::CALL_OK;
 		} else if (p_args[0]->get_type() == Variant::OBJECT) {
+			VariantInternal::clear(&r_ret);
+			VariantTypeChanger<Object *>::change(&r_ret);
 			VariantInternal::object_assign(&r_ret, p_args[0]);
 			r_error.error = Callable::CallError::CALL_OK;
 		} else {
@@ -169,6 +172,7 @@ class VariantConstructorObject {
 
 	static inline void validated_construct(Variant *r_ret, const Variant **p_args) {
 		VariantInternal::clear(r_ret);
+		VariantTypeChanger<Object *>::change(r_ret);
 		VariantInternal::object_assign(r_ret, p_args[0]);
 	}
 	static void ptr_construct(void *base, const void **p_args) {
@@ -198,11 +202,13 @@ class VariantConstructorNilObject {
 		}
 
 		VariantInternal::clear(&r_ret);
+		VariantTypeChanger<Object *>::change(&r_ret);
 		VariantInternal::object_assign_null(&r_ret);
 	}
 
 	static inline void validated_construct(Variant *r_ret, const Variant **p_args) {
 		VariantInternal::clear(r_ret);
+		VariantTypeChanger<Object *>::change(r_ret);
 		VariantInternal::object_assign_null(r_ret);
 	}
 	static void ptr_construct(void *base, const void **p_args) {
diff --git a/modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.gd b/modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.gd
new file mode 100644
index 000000000000..3724c8c71304
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.gd
@@ -0,0 +1,9 @@
+# https://github.com/godotengine/godot/issues/90086
+
+class MyObj:
+    var obj: WeakRef
+
+func test():
+    var obj_1 = MyObj.new()
+    var obj_2 = MyObj.new()
+    obj_1.obj = obj_2
diff --git a/modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.out b/modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.out
new file mode 100644
index 000000000000..dfca5b1eca38
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/errors/invalid_property_assignment.out
@@ -0,0 +1,6 @@
+GDTEST_RUNTIME_ERROR
+>> SCRIPT ERROR
+>> on function: test()
+>> runtime/errors/invalid_property_assignment.gd
+>> 9
+>> Invalid assignment of property or key 'obj' with value of type 'RefCounted (MyObj)' on a base object of type 'RefCounted (MyObj)'.
diff --git a/modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.gd b/modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.gd
new file mode 100644
index 000000000000..e1aba8350723
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.gd
@@ -0,0 +1,11 @@
+# https://github.com/godotengine/godot/issues/90086
+
+class MyObj:
+	var obj : WeakRef
+
+func test():
+	var obj_1 = MyObj.new()
+	var obj_2 = MyObj.new()
+	assert(obj_2.get_reference_count() == 1)
+	obj_1.set(&"obj", obj_2)
+	assert(obj_2.get_reference_count() == 1)
diff --git a/modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.out b/modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.out
new file mode 100644
index 000000000000..d73c5eb7cde3
--- /dev/null
+++ b/modules/gdscript/tests/scripts/runtime/features/set_does_not_leak.out
@@ -0,0 +1 @@
+GDTEST_OK