Merge pull request #38119 from RandomShaper/imvu/new_dangling_variant_fix

Fix dangling Variants (3.2)

core/object.cpp

@@ -33,6 +33,7 @@
 #include "core/class_db.h"
 #include "core/core_string_names.h"
 #include "core/message_queue.h"
+#include "core/object_rc.h"
 #include "core/os/os.h"
 #include "core/print_string.h"
 #include "core/resource.h"
@@ -968,6 +969,37 @@ void Object::cancel_delete() {
 	_predelete_ok = true;
 }
 
+#ifdef DEBUG_ENABLED
+ObjectRC *Object::_use_rc() {
+
+	// The RC object is lazily created the first time it's requested;
+	// that way, there's no need to allocate and release it at all if this Object
+	// is not being referred by any Variant at all.
+
+	// Although when dealing with Objects from multiple threads some locking
+	// mechanism should be used, this at least makes safe the case of first
+	// assignment.
+
+	ObjectRC *rc = nullptr;
+	ObjectRC *const creating = reinterpret_cast<ObjectRC *>(1);
+	if (unlikely(_rc.compare_exchange_strong(rc, creating, std::memory_order_acq_rel))) {
+		// Not created yet
+		rc = memnew(ObjectRC(this));
+		_rc.store(rc, std::memory_order_release);
+		return rc;
+	}
+
+	// Spin-wait until we know it's created (or just return if it's already created)
+	for (;;) {
+		if (likely(rc != creating)) {
+			rc->increment();
+			return rc;
+		}
+		rc = _rc.load(std::memory_order_acquire);
+	}
+}
+#endif
+
 void Object::set_script_and_instance(const RefPtr &p_script, ScriptInstance *p_instance) {
 
 	//this function is not meant to be used in any of these ways
@@ -1944,6 +1976,9 @@ Object::Object() {
 	instance_binding_count = 0;
 	memset(_script_instance_bindings, 0, sizeof(void *) * MAX_SCRIPT_INSTANCE_BINDINGS);
 	script_instance = NULL;
+#ifdef DEBUG_ENABLED
+	_rc.store(nullptr, std::memory_order_release);
+#endif
 #ifdef TOOLS_ENABLED
 
 	_edited = false;
@@ -1957,6 +1992,15 @@ Object::Object() {
 
 Object::~Object() {
 
+#ifdef DEBUG_ENABLED
+	ObjectRC *rc = _rc.load(std::memory_order_acquire);
+	if (rc) {
+		if (rc->invalidate()) {
+			memfree(rc);
+		}
+	}
+#endif
+
 	if (script_instance)
 		memdelete(script_instance);
 	script_instance = NULL;

core/object.h

@@ -34,11 +34,16 @@
 #include "core/hash_map.h"
 #include "core/list.h"
 #include "core/map.h"
+#include "core/object_id.h"
 #include "core/os/rw_lock.h"
 #include "core/set.h"
 #include "core/variant.h"
 #include "core/vmap.h"
 
+#ifdef DEBUG_ENABLED
+#include <atomic> // For ObjectRC*
+#endif
+
 #define VARIANT_ARG_LIST const Variant &p_arg1 = Variant(), const Variant &p_arg2 = Variant(), const Variant &p_arg3 = Variant(), const Variant &p_arg4 = Variant(), const Variant &p_arg5 = Variant()
 #define VARIANT_ARG_PASS p_arg1, p_arg2, p_arg3, p_arg4, p_arg5
 #define VARIANT_ARG_DECLARE const Variant &p_arg1, const Variant &p_arg2, const Variant &p_arg3, const Variant &p_arg4, const Variant &p_arg5
@@ -397,7 +402,7 @@ public:                                                        \
 private:
 
 class ScriptInstance;
-typedef uint64_t ObjectID;
+class ObjectRC;
 
 class Object {
 public:
@@ -477,6 +482,9 @@ private:
 	int _predelete_ok;
 	Set<Object *> change_receptors;
 	ObjectID _instance_id;
+#ifdef DEBUG_ENABLED
+	std::atomic<ObjectRC *> _rc;
+#endif
 	bool _predelete();
 	void _postinitialize();
 	bool _can_translate;
@@ -587,6 +595,10 @@ public:
 		return &ptr;
 	}
 
+#ifdef DEBUG_ENABLED
+	ObjectRC *_use_rc();
+#endif
+
 	bool _is_gpl_reversed() const { return false; }
 
 	_FORCE_INLINE_ ObjectID get_instance_id() const { return _instance_id; }

core/object_id.h

@@ -0,0 +1,38 @@
+/*************************************************************************/
+/*  object_rc.h                                                          */
+/*************************************************************************/
+/*                       This file is part of:                           */
+/*                           GODOT ENGINE                                */
+/*                      https://godotengine.org                          */
+/*************************************************************************/
+/* Copyright (c) 2007-2020 Juan Linietsky, Ariel Manzur.                 */
+/* Copyright (c) 2014-2020 Godot Engine contributors (cf. AUTHORS.md).   */
+/*                                                                       */
+/* Permission is hereby granted, free of charge, to any person obtaining */
+/* a copy of this software and associated documentation files (the       */
+/* "Software"), to deal in the Software without restriction, including   */
+/* without limitation the rights to use, copy, modify, merge, publish,   */
+/* distribute, sublicense, and/or sell copies of the Software, and to    */
+/* permit persons to whom the Software is furnished to do so, subject to */
+/* the following conditions:                                             */
+/*                                                                       */
+/* The above copyright notice and this permission notice shall be        */
+/* included in all copies or substantial portions of the Software.       */
+/*                                                                       */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,       */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF    */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY  */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,  */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE     */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
+/*************************************************************************/
+
+#ifndef OBJECTID_H
+#define OBJECTID_H
+
+#include "core/int_types.h"
+
+typedef uint64_t ObjectID;
+
+#endif

core/object_rc.h

@@ -0,0 +1,75 @@
+/*************************************************************************/
+/*  object_rc.h                                                          */
+/*************************************************************************/
+/*                       This file is part of:                           */
+/*                           GODOT ENGINE                                */
+/*                      https://godotengine.org                          */
+/*************************************************************************/
+/* Copyright (c) 2007-2020 Juan Linietsky, Ariel Manzur.                 */
+/* Copyright (c) 2014-2020 Godot Engine contributors (cf. AUTHORS.md).   */
+/*                                                                       */
+/* Permission is hereby granted, free of charge, to any person obtaining */
+/* a copy of this software and associated documentation files (the       */
+/* "Software"), to deal in the Software without restriction, including   */
+/* without limitation the rights to use, copy, modify, merge, publish,   */
+/* distribute, sublicense, and/or sell copies of the Software, and to    */
+/* permit persons to whom the Software is furnished to do so, subject to */
+/* the following conditions:                                             */
+/*                                                                       */
+/* The above copyright notice and this permission notice shall be        */
+/* included in all copies or substantial portions of the Software.       */
+/*                                                                       */
+/* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,       */
+/* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF    */
+/* MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.*/
+/* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY  */
+/* CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,  */
+/* TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE     */
+/* SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.                */
+/*************************************************************************/
+
+#ifndef OBJECTRC_H
+#define OBJECTRC_H
+
+#ifdef DEBUG_ENABLED
+
+#include "core/os/memory.h"
+#include "core/typedefs.h"
+
+#include <atomic>
+
+class Object;
+
+// Used to track Variants pointing to a non-Reference Object
+class ObjectRC {
+	std::atomic<Object *> _ptr;
+	std::atomic<uint32_t> _users;
+
+public:
+	_FORCE_INLINE_ void increment() {
+		_users.fetch_add(1, std::memory_order_relaxed);
+	}
+
+	_FORCE_INLINE_ bool decrement() {
+		return _users.fetch_sub(1, std::memory_order_relaxed) == 1;
+	}
+
+	_FORCE_INLINE_ bool invalidate() {
+		_ptr.store(nullptr, std::memory_order_release);
+		return decrement();
+	}
+
+	_FORCE_INLINE_ Object *get_ptr() {
+		return _ptr.load(std::memory_order_acquire);
+	}
+
+	_FORCE_INLINE_ ObjectRC(Object *p_object) {
+		// 1 (the Object) + 1 (the first user)
+		_users.store(2, std::memory_order_relaxed);
+		_ptr.store(p_object, std::memory_order_release);
+	}
+};
+
+#endif
+
+#endif

core/variant.cpp

@@ -33,6 +33,7 @@
 #include "core/core_string_names.h"
 #include "core/io/marshalls.h"
 #include "core/math/math_funcs.h"
+#include "core/object_rc.h"
 #include "core/print_string.h"
 #include "core/resource.h"
 #include "core/variant_parser.h"
@@ -790,7 +791,7 @@ bool Variant::is_zero() const {
 		} break;
 		case OBJECT: {
 
-			return _get_obj().obj == NULL;
+			return _OBJ_PTR(*this) == NULL;
 		} break;
 		case NODE_PATH: {
 
@@ -1000,6 +1001,11 @@ void Variant::reference(const Variant &p_variant) {
 		case OBJECT: {
 
 			memnew_placement(_data._mem, ObjData(p_variant._get_obj()));
+#ifdef DEBUG_ENABLED
+			if (_get_obj().rc) {
+				_get_obj().rc->increment();
+			}
+#endif
 		} break;
 		case NODE_PATH: {
 
@@ -1114,8 +1120,19 @@ void Variant::clear() {
 		} break;
 		case OBJECT: {
 
+#ifdef DEBUG_ENABLED
+			if (_get_obj().rc) {
+				if (_get_obj().rc->decrement()) {
+					memfree(_get_obj().rc);
+				}
+				_get_obj().instance_id = 0;
+			} else {
+				_get_obj().ref.unref();
+			}
+#else
 			_get_obj().obj = NULL;
 			_get_obj().ref.unref();
+#endif
 		} break;
 		case _RID: {
 			// not much need probably
@@ -1580,19 +1597,17 @@ String Variant::stringify(List<const void *> &stack) const {
 		} break;
 		case OBJECT: {
 
-			if (_get_obj().obj) {
+			Object *obj = _OBJ_PTR(*this);
+			if (obj) {
+				return obj->to_string();
+			} else {
 #ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null()) {
-					//only if debugging!
-					if (!ObjectDB::instance_validate(_get_obj().obj)) {
-						return "[Deleted Object]";
-					};
-				};
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					return "[Deleted Object]";
+				}
 #endif
-				return _get_obj().obj->to_string();
-			} else
 				return "[Object:null]";
-
+			}
 		} break;
 		default: {
 			return "[" + get_type_name(type) + "]";
@@ -1741,22 +1756,33 @@ Variant::operator RefPtr() const {
 
 Variant::operator RID() const {
 
-	if (type == _RID)
+	if (type == _RID) {
 		return *reinterpret_cast<const RID *>(_data._mem);
-	else if (type == OBJECT && !_get_obj().ref.is_null()) {
-		return _get_obj().ref.get_rid();
-	} else if (type == OBJECT && _get_obj().obj) {
+	} else if (type == OBJECT) {
+		if (!_get_obj().ref.is_null()) {
+			return _get_obj().ref.get_rid();
+		} else {
 #ifdef DEBUG_ENABLED
-		if (ScriptDebugger::get_singleton()) {
-			ERR_FAIL_COND_V_MSG(!ObjectDB::instance_validate(_get_obj().obj), RID(), "Invalid pointer (object was deleted).");
-		};
+			Object *obj = _get_obj().rc->get_ptr();
+			if (unlikely(!obj)) {
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted get RID on a deleted object.");
+				}
+			}
+#else
+			Object *obj = _get_obj().obj;
+			if (unlikely(!obj)) {
+				return RID();
+			}
 #endif
-		Variant::CallError ce;
-		Variant ret = _get_obj().obj->call(CoreStringNames::get_singleton()->get_rid, NULL, 0, ce);
-		if (ce.error == Variant::CallError::CALL_OK && ret.get_type() == Variant::_RID) {
-			return ret;
+			Variant::CallError ce;
+			Variant ret = obj->call(CoreStringNames::get_singleton()->get_rid, NULL, 0, ce);
+			if (ce.error == Variant::CallError::CALL_OK && ret.get_type() == Variant::_RID) {
+				return ret;
+			} else {
+				return RID();
+			}
 		}
-		return RID();
 	} else {
 		return RID();
 	}
@@ -1765,23 +1791,33 @@ Variant::operator RID() const {
 Variant::operator Object *() const {
 
 	if (type == OBJECT)
-		return _get_obj().obj;
+		return _OBJ_PTR(*this);
 	else
 		return NULL;
 }
 Variant::operator Node *() const {
 
-	if (type == OBJECT)
-		return Object::cast_to<Node>(_get_obj().obj);
-	else
-		return NULL;
+	if (type == OBJECT) {
+#ifdef DEBUG_ENABLED
+		Object *obj = _get_obj().rc ? _get_obj().rc->get_ptr() : NULL;
+#else
+		Object *obj = _get_obj().obj;
+#endif
+		return Object::cast_to<Node>(obj);
+	}
+	return NULL;
 }
 Variant::operator Control *() const {
 
-	if (type == OBJECT)
-		return Object::cast_to<Control>(_get_obj().obj);
-	else
-		return NULL;
+	if (type == OBJECT) {
+#ifdef DEBUG_ENABLED
+		Object *obj = _get_obj().rc ? _get_obj().rc->get_ptr() : NULL;
+#else
+		Object *obj = _get_obj().obj;
+#endif
+		return Object::cast_to<Control>(obj);
+	}
+	return NULL;
 }
 
 Variant::operator Dictionary() const {
@@ -2280,8 +2316,13 @@ Variant::Variant(const RefPtr &p_resource) {
 
 	type = OBJECT;
 	memnew_placement(_data._mem, ObjData);
+#ifdef DEBUG_ENABLED
+	_get_obj().rc = NULL;
+	_get_obj().instance_id = 0;
+#else
 	REF *ref = reinterpret_cast<REF *>(p_resource.get_data());
 	_get_obj().obj = ref->ptr();
+#endif
 	_get_obj().ref = p_resource;
 }
 
@@ -2296,7 +2337,12 @@ Variant::Variant(const Object *p_object) {
 	type = OBJECT;
 
 	memnew_placement(_data._mem, ObjData);
+#ifdef DEBUG_ENABLED
+	_get_obj().rc = p_object ? const_cast<Object *>(p_object)->_use_rc() : NULL;
+	_get_obj().instance_id = p_object ? p_object->get_instance_id() : 0;
+#else
 	_get_obj().obj = const_cast<Object *>(p_object);
+#endif
 }
 
 Variant::Variant(const Dictionary &p_dictionary) {
@@ -2608,6 +2654,11 @@ void Variant::operator=(const Variant &p_variant) {
 		case OBJECT: {
 
 			*reinterpret_cast<ObjData *>(_data._mem) = p_variant._get_obj();
+#ifdef DEBUG_ENABLED
+			if (_get_obj().rc) {
+				_get_obj().rc->increment();
+			}
+#endif
 		} break;
 		case NODE_PATH: {
 
@@ -2805,7 +2856,7 @@ uint32_t Variant::hash() const {
 		} break;
 		case OBJECT: {
 
-			return hash_djb2_one_64(make_uint64_t(_get_obj().obj));
+			return hash_djb2_one_64(make_uint64_t(_OBJ_PTR(*this)));
 		} break;
 		case NODE_PATH: {
 

core/variant.h

@@ -44,13 +44,14 @@
 #include "core/math/transform_2d.h"
 #include "core/math/vector3.h"
 #include "core/node_path.h"
+#include "core/object_id.h"
 #include "core/pool_vector.h"
 #include "core/ref_ptr.h"
 #include "core/rid.h"
 #include "core/ustring.h"
 
-class RefPtr;
 class Object;
+class ObjectRC;
 class Node; // helper
 class Control; // helper
 
@@ -72,6 +73,13 @@ typedef PoolVector<Color> PoolColorArray;
 #define GCC_ALIGNED_8
 #endif
 
+#ifdef DEBUG_ENABLED
+// Ideally, an inline member of ObjectRC, but would cause circular includes
+#define _OBJ_PTR(m_variant) ((m_variant)._get_obj().rc ? (m_variant)._get_obj().rc->get_ptr() : reinterpret_cast<Ref<Reference> *>((m_variant)._get_obj().ref.get_data())->ptr())
+#else
+#define _OBJ_PTR(m_variant) ((m_variant)._get_obj().obj)
+#endif
+
 class Variant {
 public:
 	// If this changes the table in variant_op must be updated
@@ -127,7 +135,21 @@ private:
 
 	struct ObjData {
 
+#ifdef DEBUG_ENABLED
+		// Will be null for every type deriving from Reference as they have their
+		// own reference count mechanism
+		ObjectRC *rc;
+		// This is for allowing debug build to check for instance ID validity,
+		// so warnings are shown in debug builds when a stray Variant (one pointing
+		// to a released Object) would have happened.
+		// If it's zero, that means the Variant is has a legit null object value,
+		// thus not needing instance id validation.
+		ObjectID instance_id;
+#else
 		Object *obj;
+#endif
+		// Always initialized, but will be null if the Ref<> assigned was null
+		// or this Variant is not even holding a Reference-derived object
 		RefPtr ref;
 	};
 

core/variant_call.cpp

@@ -35,6 +35,7 @@
 #include "core/crypto/crypto_core.h"
 #include "core/io/compression.h"
 #include "core/object.h"
+#include "core/object_rc.h"
 #include "core/os/os.h"
 #include "core/script_language.h"
 
@@ -1094,22 +1095,18 @@ void Variant::call_ptr(const StringName &p_method, const Variant **p_args, int p
 
 	if (type == Variant::OBJECT) {
 		//call object
-		Object *obj = _get_obj().obj;
+		Object *obj = _OBJ_PTR(*this);
 		if (!obj) {
-			r_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL;
-			return;
-		}
 #ifdef DEBUG_ENABLED
-		if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null()) {
-			//only if debugging!
-			if (!ObjectDB::instance_validate(obj)) {
-				r_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL;
-				return;
+			if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+				WARN_PRINT("Attempted call on stray pointer object.");
 			}
+#endif
+			r_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL;
+			return;
 		}
 
-#endif
-		ret = _get_obj().obj->call(p_method, p_args, p_argcount, r_error);
+		ret = obj->call(p_method, p_args, p_argcount, r_error);
 
 		//else if (type==Variant::METHOD) {
 
@@ -1275,18 +1272,16 @@ Variant Variant::construct(const Variant::Type p_type, const Variant **p_args, i
 bool Variant::has_method(const StringName &p_method) const {
 
 	if (type == OBJECT) {
-		Object *obj = operator Object *();
-		if (!obj)
-			return false;
-#ifdef DEBUG_ENABLED
-		if (ScriptDebugger::get_singleton()) {
-			if (ObjectDB::instance_validate(obj)) {
-#endif
-				return obj->has_method(p_method);
+		Object *obj = _OBJ_PTR(*this);
+		if (!obj) {
 #ifdef DEBUG_ENABLED
+			if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+				WARN_PRINT("Attempted method check on stray pointer object.");
 			}
-		}
 #endif
+			return false;
+		}
+		return obj->has_method(p_method);
 	}
 
 	const _VariantCall::TypeFunc &tf = _VariantCall::type_funcs[type];

core/variant_op.cpp

@@ -32,6 +32,7 @@
 
 #include "core/core_string_names.h"
 #include "core/object.h"
+#include "core/object_rc.h"
 #include "core/script_language.h"
 
 #define CASE_TYPE_ALL(PREFIX, OP) \
@@ -399,7 +400,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_EQUAL, NIL) {
 				if (p_b.type == NIL) _RETURN(true);
 				if (p_b.type == OBJECT)
-					_RETURN(p_b._get_obj().obj == NULL);
+					_RETURN(_OBJ_PTR(p_b) == NULL);
 
 				_RETURN(false);
 			}
@@ -416,9 +417,9 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 
 			CASE_TYPE(math, OP_EQUAL, OBJECT) {
 				if (p_b.type == OBJECT)
-					_RETURN((p_a._get_obj().obj == p_b._get_obj().obj));
+					_RETURN(_OBJ_PTR(p_a) == _OBJ_PTR(p_b));
 				if (p_b.type == NIL)
-					_RETURN(p_a._get_obj().obj == NULL);
+					_RETURN(_OBJ_PTR(p_a) == NULL);
 
 				_RETURN_FAIL;
 			}
@@ -486,7 +487,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_NOT_EQUAL, NIL) {
 				if (p_b.type == NIL) _RETURN(false);
 				if (p_b.type == OBJECT)
-					_RETURN(p_b._get_obj().obj != NULL);
+					_RETURN(_OBJ_PTR(p_b) != NULL);
 
 				_RETURN(true);
 			}
@@ -504,9 +505,9 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 
 			CASE_TYPE(math, OP_NOT_EQUAL, OBJECT) {
 				if (p_b.type == OBJECT)
-					_RETURN((p_a._get_obj().obj != p_b._get_obj().obj));
+					_RETURN((_OBJ_PTR(p_a) != _OBJ_PTR(p_b)));
 				if (p_b.type == NIL)
-					_RETURN(p_a._get_obj().obj != NULL);
+					_RETURN(_OBJ_PTR(p_a) != NULL);
 
 				_RETURN_FAIL;
 			}
@@ -589,7 +590,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_LESS, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN((p_a._get_obj().obj < p_b._get_obj().obj));
+				_RETURN(_OBJ_PTR(p_a) < _OBJ_PTR(p_b));
 			}
 
 			CASE_TYPE(math, OP_LESS, ARRAY) {
@@ -643,7 +644,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_LESS_EQUAL, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN((p_a._get_obj().obj <= p_b._get_obj().obj));
+				_RETURN(_OBJ_PTR(p_a) <= _OBJ_PTR(p_b));
 			}
 
 			DEFAULT_OP_NUM(math, OP_LESS_EQUAL, INT, <=, _int);
@@ -693,7 +694,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_GREATER, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN((p_a._get_obj().obj > p_b._get_obj().obj));
+				_RETURN(_OBJ_PTR(p_a) > _OBJ_PTR(p_b));
 			}
 
 			CASE_TYPE(math, OP_GREATER, ARRAY) {
@@ -747,7 +748,7 @@ void Variant::evaluate(const Operator &p_op, const Variant &p_a,
 			CASE_TYPE(math, OP_GREATER_EQUAL, OBJECT) {
 				if (p_b.type != OBJECT)
 					_RETURN_FAIL;
-				_RETURN((p_a._get_obj().obj >= p_b._get_obj().obj));
+				_RETURN(_OBJ_PTR(p_a) >= _OBJ_PTR(p_b));
 			}
 
 			DEFAULT_OP_NUM(math, OP_GREATER_EQUAL, INT, >=, _int);
@@ -1512,15 +1513,16 @@ void Variant::set_named(const StringName &p_index, const Variant &p_value, bool
 		} break;
 		case OBJECT: {
 
+			Object *obj = _OBJ_PTR(*this);
 #ifdef DEBUG_ENABLED
-			if (!_get_obj().obj) {
-				break;
-			} else if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null() && !ObjectDB::instance_validate(_get_obj().obj)) {
+			if (unlikely(!obj)) {
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted set on a deleted object.");
+				}
 				break;
 			}
-
 #endif
-			_get_obj().obj->set(p_index, p_value, &valid);
+			obj->set(p_index, p_value, &valid);
 
 		} break;
 		default: {
@@ -1677,23 +1679,19 @@ Variant Variant::get_named(const StringName &p_index, bool *r_valid) const {
 		} break;
 		case OBJECT: {
 
+			Object *obj = _OBJ_PTR(*this);
 #ifdef DEBUG_ENABLED
-			if (!_get_obj().obj) {
+			if (unlikely(!obj)) {
 				if (r_valid)
 					*r_valid = false;
-				return "Instance base is null.";
-			} else {
-
-				if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null() && !ObjectDB::instance_validate(_get_obj().obj)) {
-					if (r_valid)
-						*r_valid = false;
-					return "Attempted use of stray pointer object.";
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted get on a deleted object.");
 				}
+				return Variant();
 			}
-
 #endif
 
-			return _get_obj().obj->get(p_index, r_valid);
+			return obj->get(p_index, r_valid);
 
 		} break;
 		default: {
@@ -2167,29 +2165,24 @@ void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid)
 		} break;
 		case OBJECT: {
 
-			Object *obj = _get_obj().obj;
-			//only if debugging!
-
-			if (obj) {
+			Object *obj = _OBJ_PTR(*this);
+			if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null()) {
-
-					if (!ObjectDB::instance_validate(obj)) {
-						WARN_PRINT("Attempted use of stray pointer object.");
-						valid = false;
-						return;
-					}
+				valid = false;
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted set on a deleted object.");
 				}
 #endif
+				return;
+			}
 
-				if (p_index.get_type() != Variant::STRING) {
-					obj->setvar(p_index, p_value, r_valid);
-					return;
-				}
-
-				obj->set(p_index, p_value, r_valid);
+			if (p_index.get_type() != Variant::STRING) {
+				obj->setvar(p_index, p_value, r_valid);
 				return;
 			}
+
+			obj->set(p_index, p_value, r_valid);
+			return;
 		} break;
 		case DICTIONARY: {
 
@@ -2542,23 +2535,20 @@ Variant Variant::get(const Variant &p_index, bool *r_valid) const {
 		case _RID: {
 		} break;
 		case OBJECT: {
-			Object *obj = _get_obj().obj;
-			if (obj) {
-
+			Object *obj = _OBJ_PTR(*this);
+			if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null()) {
-					//only if debugging!
-					if (!ObjectDB::instance_validate(obj)) {
-						valid = false;
-						return "Attempted get on stray pointer.";
-					}
+				valid = false;
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted get on a deleted object.");
 				}
 #endif
+				return Variant();
+			}
 
-				if (p_index.get_type() != Variant::STRING) {
-					return obj->getvar(p_index, r_valid);
-				}
-
+			if (p_index.get_type() != Variant::STRING) {
+				return obj->getvar(p_index, r_valid);
+			} else {
 				return obj->get(p_index, r_valid);
 			}
 
@@ -2606,34 +2596,26 @@ bool Variant::in(const Variant &p_index, bool *r_valid) const {
 
 		} break;
 		case OBJECT: {
-			Object *obj = _get_obj().obj;
-			if (obj) {
-
-				bool valid = false;
+			Object *obj = _OBJ_PTR(*this);
+			if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null()) {
-					//only if debugging!
-					if (!ObjectDB::instance_validate(obj)) {
-						if (r_valid) {
-							*r_valid = false;
-						}
-						return true; // Attempted get on stray pointer.
-					}
+				if (r_valid) {
+					*r_valid = false;
 				}
-#endif
-
-				if (p_index.get_type() != Variant::STRING) {
-					obj->getvar(p_index, &valid);
-				} else {
-					obj->get(p_index, &valid);
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted 'in' on a deleted object.");
 				}
+#endif
+				return false;
+			}
 
-				return valid;
+			bool result;
+			if (p_index.get_type() != Variant::STRING) {
+				obj->getvar(p_index, &result);
 			} else {
-				if (r_valid)
-					*r_valid = false;
+				obj->get(p_index, &result);
 			}
-			return false;
+			return result;
 		} break;
 		case DICTIONARY: {
 
@@ -2880,21 +2862,17 @@ void Variant::get_property_list(List<PropertyInfo> *p_list) const {
 		} break;
 		case OBJECT: {
 
-			Object *obj = _get_obj().obj;
-			if (obj) {
+			Object *obj = _OBJ_PTR(*this);
+			if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null()) {
-					//only if debugging!
-					if (!ObjectDB::instance_validate(obj)) {
-						WARN_PRINT("Attempted get_property list on stray pointer.");
-						return;
-					}
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted get property list on a deleted object.");
 				}
 #endif
-
-				obj->get_property_list(p_list);
+				return;
 			}
 
+			obj->get_property_list(p_list);
 		} break;
 		case DICTIONARY: {
 
@@ -2961,14 +2939,13 @@ bool Variant::iter_init(Variant &r_iter, bool &valid) const {
 		} break;
 		case OBJECT: {
 
+			Object *obj = _OBJ_PTR(*this);
 #ifdef DEBUG_ENABLED
-			if (!_get_obj().obj) {
-				valid = false;
-				return false;
-			}
-
-			if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null() && !ObjectDB::instance_validate(_get_obj().obj)) {
+			if (unlikely(!obj)) {
 				valid = false;
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted iteration start on a deleted object.");
+				}
 				return false;
 			}
 #endif
@@ -2978,7 +2955,7 @@ bool Variant::iter_init(Variant &r_iter, bool &valid) const {
 			ref.push_back(r_iter);
 			Variant vref = ref;
 			const Variant *refp[] = { &vref };
-			Variant ret = _get_obj().obj->call(CoreStringNames::get_singleton()->_iter_init, refp, 1, ce);
+			Variant ret = obj->call(CoreStringNames::get_singleton()->_iter_init, refp, 1, ce);
 
 			if (ref.size() != 1 || ce.error != Variant::CallError::CALL_OK) {
 				valid = false;
@@ -3129,14 +3106,13 @@ bool Variant::iter_next(Variant &r_iter, bool &valid) const {
 		} break;
 		case OBJECT: {
 
+			Object *obj = _OBJ_PTR(*this);
 #ifdef DEBUG_ENABLED
-			if (!_get_obj().obj) {
-				valid = false;
-				return false;
-			}
-
-			if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null() && !ObjectDB::instance_validate(_get_obj().obj)) {
+			if (unlikely(!obj)) {
 				valid = false;
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted iteration check next on a deleted object.");
+				}
 				return false;
 			}
 #endif
@@ -3146,7 +3122,7 @@ bool Variant::iter_next(Variant &r_iter, bool &valid) const {
 			ref.push_back(r_iter);
 			Variant vref = ref;
 			const Variant *refp[] = { &vref };
-			Variant ret = _get_obj().obj->call(CoreStringNames::get_singleton()->_iter_next, refp, 1, ce);
+			Variant ret = obj->call(CoreStringNames::get_singleton()->_iter_next, refp, 1, ce);
 
 			if (ref.size() != 1 || ce.error != Variant::CallError::CALL_OK) {
 				valid = false;
@@ -3288,21 +3264,20 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const {
 		} break;
 		case OBJECT: {
 
+			Object *obj = _OBJ_PTR(*this);
 #ifdef DEBUG_ENABLED
-			if (!_get_obj().obj) {
-				r_valid = false;
-				return Variant();
-			}
-
-			if (ScriptDebugger::get_singleton() && _get_obj().ref.is_null() && !ObjectDB::instance_validate(_get_obj().obj)) {
+			if (unlikely(!obj)) {
 				r_valid = false;
+				if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) {
+					WARN_PRINT("Attempted iteration get next on a deleted object.");
+				}
 				return Variant();
 			}
 #endif
 			Variant::CallError ce;
 			ce.error = Variant::CallError::CALL_OK;
 			const Variant *refp[] = { &r_iter };
-			Variant ret = _get_obj().obj->call(CoreStringNames::get_singleton()->_iter_get, refp, 1, ce);
+			Variant ret = obj->call(CoreStringNames::get_singleton()->_iter_get, refp, 1, ce);
 
 			if (ce.error != Variant::CallError::CALL_OK) {
 				r_valid = false;

scene/gui/rich_text_label.cpp

@@ -2884,15 +2884,15 @@ Dictionary RichTextLabel::parse_expressions_for_values(Vector<String> p_expressi
 
 		Vector<String> values = parts[1].split(",", false);
 
-		RegEx color = RegEx();
+		RegEx color;
 		color.compile("^#([A-Fa-f0-9]{6}|[A-Fa-f0-9]{3})$");
-		RegEx nodepath = RegEx();
+		RegEx nodepath;
 		nodepath.compile("^\\$");
-		RegEx boolean = RegEx();
+		RegEx boolean;
 		boolean.compile("^(true|false)$");
-		RegEx decimal = RegEx();
+		RegEx decimal;
 		decimal.compile("^-?^.?\\d+(\\.\\d+?)?$");
-		RegEx numerical = RegEx();
+		RegEx numerical;
 		numerical.compile("^\\d+$");
 
 		for (int j = 0; j < values.size(); j++) {