Browse Source

Merge pull request #51796 from RandomShaper/dangling_obj_release_3.x

[3.x] Promote object validity checks to release builds
Rémi Verschelde 3 years ago
parent
commit
22aab6be1c

+ 3 - 5
core/io/marshalls.cpp

@@ -786,10 +786,9 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo
 			}
 		} break;
 		case Variant::OBJECT: {
-#ifdef DEBUG_ENABLED
-			// Test for potential wrong values sent by the debugger when it breaks.
+			// Test for potential wrong values sent by the debugger when it breaks or freed objects.
 			Object *obj = p_variant;
-			if (!obj || !ObjectDB::instance_validate(obj)) {
+			if (!obj) {
 				// Object is invalid, send a NULL instead.
 				if (buf) {
 					encode_uint32(Variant::NIL, buf);
@@ -797,7 +796,6 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo
 				r_len += 4;
 				return OK;
 			}
-#endif // DEBUG_ENABLED
 			if (!p_full_objects) {
 				flags |= ENCODE_FLAG_OBJECT_AS_ID;
 			}
@@ -1090,7 +1088,7 @@ Error encode_variant(const Variant &p_variant, uint8_t *r_buffer, int &r_len, bo
 				if (buf) {
 					Object *obj = p_variant;
 					ObjectID id = 0;
-					if (obj && ObjectDB::instance_validate(obj)) {
+					if (obj) {
 						id = obj->get_instance_id();
 					}
 

+ 0 - 6
core/object.cpp

@@ -961,7 +961,6 @@ 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
@@ -989,7 +988,6 @@ ObjectRC *Object::_use_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
@@ -1927,9 +1925,7 @@ Object::Object() {
 	_emitting = false;
 	memset(_script_instance_bindings, 0, sizeof(void *) * MAX_SCRIPT_INSTANCE_BINDINGS);
 	script_instance = nullptr;
-#ifdef DEBUG_ENABLED
 	_rc.store(nullptr, std::memory_order_release);
-#endif
 #ifdef TOOLS_ENABLED
 
 	_edited = false;
@@ -1942,14 +1938,12 @@ Object::Object() {
 }
 
 Object::~Object() {
-#ifdef DEBUG_ENABLED
 	ObjectRC *rc = _rc.load(std::memory_order_acquire);
 	if (rc) {
 		if (rc->invalidate()) {
 			memdelete(rc);
 		}
 	}
-#endif
 
 	if (script_instance) {
 		memdelete(script_instance);

+ 2 - 7
core/object.h

@@ -41,9 +41,7 @@
 #include "core/variant.h"
 #include "core/vmap.h"
 
-#ifdef DEBUG_ENABLED
-#include <atomic> // For ObjectRC*
-#endif
+#include <atomic>
 
 #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
@@ -476,9 +474,7 @@ 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;
@@ -590,9 +586,7 @@ public:
 		return &ptr;
 	}
 
-#ifdef DEBUG_ENABLED
 	ObjectRC *_use_rc();
-#endif
 
 	bool _is_gpl_reversed() const { return false; }
 
@@ -798,6 +792,7 @@ public:
 	static void debug_objects(DebugFunc p_func);
 	static int get_object_count();
 
+	// This one may give false positives because a new object may be allocated at the same memory of a previously freed one
 	_FORCE_INLINE_ static bool instance_validate(Object *p_ptr) {
 		rw_lock.read_lock();
 

+ 0 - 4
core/object_rc.h

@@ -31,8 +31,6 @@
 #ifndef OBJECTRC_H
 #define OBJECTRC_H
 
-#ifdef DEBUG_ENABLED
-
 #include "core/os/memory.h"
 #include "core/typedefs.h"
 
@@ -77,5 +75,3 @@ public:
 };
 
 #endif
-
-#endif

+ 9 - 47
core/variant.cpp

@@ -817,11 +817,15 @@ ObjectID Variant::get_object_instance_id() const {
 	if (is_ref() && _get_obj().ref.is_null()) {
 		return 0;
 	} else {
-		return _get_obj().obj->get_instance_id();
+		return _get_obj().rc->get_ptr()->get_instance_id();
 	}
 #endif
 }
 
+bool Variant::is_invalid_object() const {
+	return type == OBJECT && _get_obj().rc && !_get_obj().rc->get_ptr();
+}
+
 void Variant::reference(const Variant &p_variant) {
 	switch (type) {
 		case NIL:
@@ -896,11 +900,9 @@ void Variant::reference(const Variant &p_variant) {
 		} break;
 		case OBJECT: {
 			memnew_placement(_data._mem, ObjData(p_variant._get_obj()));
-#ifdef DEBUG_ENABLED
-			if (_get_obj().rc) {
+			if (likely(_get_obj().rc)) {
 				_get_obj().rc->increment();
 			}
-#endif
 		} break;
 		case NODE_PATH: {
 			memnew_placement(_data._mem, NodePath(*reinterpret_cast<const NodePath *>(p_variant._data._mem)));
@@ -1018,7 +1020,6 @@ void Variant::clear() {
 			reinterpret_cast<NodePath *>(_data._mem)->~NodePath();
 		} break;
 		case OBJECT: {
-#ifdef DEBUG_ENABLED
 			if (likely(_get_obj().rc)) {
 				if (unlikely(_get_obj().rc->decrement())) {
 					memdelete(_get_obj().rc);
@@ -1026,10 +1027,6 @@ void Variant::clear() {
 			} else {
 				_get_obj().ref.unref();
 			}
-#else
-			_get_obj().obj = NULL;
-			_get_obj().ref.unref();
-#endif
 		} break;
 		case _RID: {
 			// not much need probably
@@ -1511,18 +1508,12 @@ String Variant::stringify(List<const void *> &stack) const {
 		} break;
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
-			if (obj) {
-				if (_get_obj().ref.is_null() && !ObjectDB::get_instance(obj->get_instance_id())) {
-					return "[Deleted Object]";
-				}
-
+			if (likely(obj)) {
 				return obj->to_string();
 			} else {
-#ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+				if (_get_obj().rc) {
 					return "[Deleted Object]";
 				}
-#endif
 				return "[Object:null]";
 			}
 		} break;
@@ -1678,20 +1669,13 @@ Variant::operator RID() const {
 		if (!_get_obj().ref.is_null()) {
 			return _get_obj().ref.get_rid();
 		} else {
-#ifdef DEBUG_ENABLED
 			Object *obj = likely(_get_obj().rc) ? _get_obj().rc->get_ptr() : nullptr;
 			if (unlikely(!obj)) {
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted get RID on a deleted object.");
 				}
 				return RID();
 			}
-#else
-			Object *obj = _get_obj().obj;
-			if (unlikely(!obj)) {
-				return RID();
-			}
-#endif
 			Variant::CallError ce;
 			Variant ret = obj->call(CoreStringNames::get_singleton()->get_rid, nullptr, 0, ce);
 			if (ce.error == Variant::CallError::CALL_OK && ret.get_type() == Variant::_RID) {
@@ -1714,22 +1698,14 @@ Variant::operator Object *() const {
 }
 Variant::operator Node *() const {
 	if (type == OBJECT) {
-#ifdef DEBUG_ENABLED
 		Object *obj = _get_obj().rc ? _get_obj().rc->get_ptr() : nullptr;
-#else
-		Object *obj = _get_obj().obj;
-#endif
 		return Object::cast_to<Node>(obj);
 	}
 	return nullptr;
 }
 Variant::operator Control *() const {
 	if (type == OBJECT) {
-#ifdef DEBUG_ENABLED
 		Object *obj = _get_obj().rc ? _get_obj().rc->get_ptr() : nullptr;
-#else
-		Object *obj = _get_obj().obj;
-#endif
 		return Object::cast_to<Control>(obj);
 	}
 	return nullptr;
@@ -2182,12 +2158,7 @@ Variant::Variant(const NodePath &p_node_path) {
 Variant::Variant(const RefPtr &p_resource) {
 	type = OBJECT;
 	memnew_placement(_data._mem, ObjData);
-#ifdef DEBUG_ENABLED
 	_get_obj().rc = nullptr;
-#else
-	REF *ref = reinterpret_cast<REF *>(p_resource.get_data());
-	_get_obj().obj = ref->ptr();
-#endif
 	_get_obj().ref = p_resource;
 }
 
@@ -2204,15 +2175,10 @@ Variant::Variant(const Object *p_object) {
 	Reference *ref = Object::cast_to<Reference>(obj);
 	if (unlikely(ref)) {
 		*reinterpret_cast<Ref<Reference> *>(_get_obj().ref.get_data()) = Ref<Reference>(ref);
-#ifdef DEBUG_ENABLED
 		_get_obj().rc = nullptr;
 	} else {
 		_get_obj().rc = likely(obj) ? obj->_use_rc() : nullptr;
-#endif
 	}
-#if !defined(DEBUG_ENABLED)
-	_get_obj().obj = obj;
-#endif
 }
 
 Variant::Variant(const Dictionary &p_dictionary) {
@@ -2490,19 +2456,15 @@ void Variant::operator=(const Variant &p_variant) {
 			*reinterpret_cast<RID *>(_data._mem) = *reinterpret_cast<const RID *>(p_variant._data._mem);
 		} break;
 		case OBJECT: {
-#ifdef DEBUG_ENABLED
 			if (likely(_get_obj().rc)) {
 				if (unlikely(_get_obj().rc->decrement())) {
 					memdelete(_get_obj().rc);
 				}
 			}
-#endif
 			*reinterpret_cast<ObjData *>(_data._mem) = p_variant._get_obj();
-#ifdef DEBUG_ENABLED
 			if (likely(_get_obj().rc)) {
 				_get_obj().rc->increment();
 			}
-#endif
 		} break;
 		case NODE_PATH: {
 			*reinterpret_cast<NodePath *>(_data._mem) = *reinterpret_cast<const NodePath *>(p_variant._data._mem);

+ 4 - 17
core/variant.h

@@ -73,22 +73,12 @@ typedef PoolVector<Color> PoolColorArray;
 #define GCC_ALIGNED_8
 #endif
 
-// With DEBUG_ENABLED, the pointer to a deleted object stored in ObjectRC is set to nullptr,
-// so _OBJ_PTR is not useful for checks in which we want to act as if we still believed the
-// object is alive; e.g., comparing a Variant that points to a deleted object with NIL,
-// should return false regardless DEBUG_ENABLED is defined or not.
-// So in cases like that we use _UNSAFE_OBJ_PROXY_PTR, which serves that purpose. With DEBUG_ENABLED
-// it won't be the real pointer to the object for non-Reference types, but that's fine.
-// We just need it to be unique for each object, to be comparable and not to be forced to NULL
-// when the object is freed.
-#ifdef DEBUG_ENABLED
 #define _REF_OBJ_PTR(m_variant) (reinterpret_cast<Ref<Reference> *>((m_variant)._get_obj().ref.get_data())->ptr())
 #define _OBJ_PTR(m_variant) ((m_variant)._get_obj().rc ? (m_variant)._get_obj().rc->get_ptr() : _REF_OBJ_PTR(m_variant))
+// _UNSAFE_OBJ_PROXY_PTR is needed for comparing an object Variant against NIL or compare two object Variants.
+// It's guaranteed to be unique per object, in contrast to the pointer stored in the RC structure,
+// which is set to null when the object is destroyed.
 #define _UNSAFE_OBJ_PROXY_PTR(m_variant) ((m_variant)._get_obj().rc ? reinterpret_cast<uint8_t *>((m_variant)._get_obj().rc) : reinterpret_cast<uint8_t *>(_REF_OBJ_PTR(m_variant)))
-#else
-#define _OBJ_PTR(m_variant) ((m_variant)._get_obj().obj)
-#define _UNSAFE_OBJ_PROXY_PTR(m_variant) _OBJ_PTR(m_variant)
-#endif
 
 class Variant {
 public:
@@ -149,13 +139,9 @@ private:
 	Type type;
 
 	struct ObjData {
-#ifdef DEBUG_ENABLED
 		// Will be null for every type deriving from Reference as they have their
 		// own reference count mechanism
 		ObjectRC *rc;
-#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;
@@ -193,6 +179,7 @@ public:
 	bool is_one() const;
 
 	ObjectID get_object_instance_id() const;
+	bool is_invalid_object() const;
 
 	operator bool() const;
 	operator signed int() const;

+ 4 - 4
core/variant_call.cpp

@@ -1162,9 +1162,9 @@ void Variant::call_ptr(const StringName &p_method, const Variant **p_args, int p
 	if (type == Variant::OBJECT) {
 		//call object
 		Object *obj = _OBJ_PTR(*this);
-		if (!obj) {
+		if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-			if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+			if (_get_obj().rc) {
 				ERR_PRINT("Attempted method call on a deleted object.");
 			}
 #endif
@@ -1374,9 +1374,9 @@ 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 = _OBJ_PTR(*this);
-		if (!obj) {
+		if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-			if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+			if (_get_obj().rc) {
 				ERR_PRINT("Attempted method check on a deleted object.");
 			}
 #endif

+ 22 - 22
core/variant_op.cpp

@@ -1529,14 +1529,14 @@ 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 (unlikely(!obj)) {
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted set on a deleted object.");
 				}
+#endif
 				break;
 			}
-#endif
 			obj->set(p_index, p_value, &valid);
 
 		} break;
@@ -1684,17 +1684,17 @@ Variant Variant::get_named(const StringName &p_index, bool *r_valid) const {
 		} break;
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
-#ifdef DEBUG_ENABLED
 			if (unlikely(!obj)) {
 				if (r_valid) {
 					*r_valid = false;
 				}
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted get on a deleted object.");
 				}
+#endif
 				return Variant();
 			}
-#endif
 
 			return obj->get(p_index, r_valid);
 
@@ -2169,9 +2169,9 @@ void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid)
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
 			if (unlikely(!obj)) {
-#ifdef DEBUG_ENABLED
 				valid = false;
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted set on a deleted object.");
 				}
 #endif
@@ -2520,9 +2520,9 @@ Variant Variant::get(const Variant &p_index, bool *r_valid) const {
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
 			if (unlikely(!obj)) {
-#ifdef DEBUG_ENABLED
 				valid = false;
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted get on a deleted object.");
 				}
 #endif
@@ -2578,11 +2578,11 @@ bool Variant::in(const Variant &p_index, bool *r_valid) const {
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
 			if (unlikely(!obj)) {
-#ifdef DEBUG_ENABLED
 				if (r_valid) {
 					*r_valid = false;
 				}
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted 'in' on a deleted object.");
 				}
 #endif
@@ -2832,7 +2832,7 @@ void Variant::get_property_list(List<PropertyInfo> *p_list) const {
 			Object *obj = _OBJ_PTR(*this);
 			if (unlikely(!obj)) {
 #ifdef DEBUG_ENABLED
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted get property list on a deleted object.");
 				}
 #endif
@@ -2903,15 +2903,15 @@ bool Variant::iter_init(Variant &r_iter, bool &valid) const {
 		} break;
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
-#ifdef DEBUG_ENABLED
 			if (unlikely(!obj)) {
 				valid = false;
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted iteration start on a deleted object.");
 				}
+#endif
 				return false;
 			}
-#endif
 			Variant::CallError ce;
 			ce.error = Variant::CallError::CALL_OK;
 			Array ref;
@@ -3077,15 +3077,15 @@ bool Variant::iter_next(Variant &r_iter, bool &valid) const {
 		} break;
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
-#ifdef DEBUG_ENABLED
 			if (unlikely(!obj)) {
 				valid = false;
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted iteration check next on a deleted object.");
 				}
+#endif
 				return false;
 			}
-#endif
 			Variant::CallError ce;
 			ce.error = Variant::CallError::CALL_OK;
 			Array ref;
@@ -3233,15 +3233,15 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const {
 		} break;
 		case OBJECT: {
 			Object *obj = _OBJ_PTR(*this);
-#ifdef DEBUG_ENABLED
 			if (unlikely(!obj)) {
 				r_valid = false;
-				if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
+#ifdef DEBUG_ENABLED
+				if (_get_obj().rc) {
 					ERR_PRINT("Attempted iteration get next on a deleted object.");
 				}
+#endif
 				return Variant();
 			}
-#endif
 			Variant::CallError ce;
 			ce.error = Variant::CallError::CALL_OK;
 			const Variant *refp[] = { &r_iter };

+ 22 - 24
modules/gdscript/gdscript_function.cpp

@@ -133,16 +133,16 @@ static String _get_var_type(const Variant *p_var) {
 	if (p_var->get_type() == Variant::OBJECT) {
 		Object *bobj = *p_var;
 		if (!bobj) {
-			basestr = "null instance";
+			if (p_var->is_invalid_object()) {
+				basestr = "previously freed instance";
+			} else {
+				basestr = "null instance";
+			}
 		} else {
-			if (ObjectDB::instance_validate(bobj)) {
-				if (bobj->get_script_instance()) {
-					basestr = bobj->get_class() + " (" + bobj->get_script_instance()->get_script()->get_path().get_file() + ")";
-				} else {
-					basestr = bobj->get_class();
-				}
+			if (bobj->get_script_instance()) {
+				basestr = bobj->get_class() + " (" + bobj->get_script_instance()->get_script()->get_path().get_file() + ")";
 			} else {
-				basestr = "previously freed instance";
+				basestr = bobj->get_class();
 			}
 		}
 
@@ -364,6 +364,9 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 		}
 	}
 
+#ifdef DEBUG_ENABLED
+	Variant instance = p_instance;
+#endif
 	static_ref = script;
 
 	String err_text;
@@ -466,7 +469,11 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 				GET_VARIANT_PTR(dst, 3);
 
 #ifdef DEBUG_ENABLED
-				if (b->get_type() != Variant::OBJECT || b->operator Object *() == nullptr) {
+				if (a->is_invalid_object()) {
+					err_text = "Left operand of 'is' was already freed.";
+					OPCODE_BREAK;
+				}
+				if (b->is_invalid_object()) {
 					err_text = "Right operand of 'is' is not a class.";
 					OPCODE_BREAK;
 				}
@@ -477,13 +484,6 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					Object *obj_A = *a;
 					Object *obj_B = *b;
 
-#ifdef DEBUG_ENABLED
-					if (!ObjectDB::instance_validate(obj_A)) {
-						err_text = "Left operand of 'is' was already freed.";
-						OPCODE_BREAK;
-					}
-#endif // DEBUG_ENABLED
-
 					GDScript *scr_B = Object::cast_to<GDScript>(obj_B);
 
 					if (scr_B) {
@@ -1264,17 +1264,15 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 					Object *obj = argobj->operator Object *();
 					String signal = argname->operator String();
 
+					if (argobj->is_invalid_object()) {
+						err_text = "First argument of yield() is a previously freed instance.";
+						OPCODE_BREAK;
+					}
 #ifdef DEBUG_ENABLED
 					if (!obj) {
 						err_text = "First argument of yield() is null.";
 						OPCODE_BREAK;
 					}
-					if (ScriptDebugger::get_singleton()) {
-						if (!ObjectDB::instance_validate(obj)) {
-							err_text = "First argument of yield() is a previously freed instance.";
-							OPCODE_BREAK;
-						}
-					}
 					if (signal.length() == 0) {
 						err_text = "Second argument of yield() is an empty string (for signal name).";
 						OPCODE_BREAK;
@@ -1525,7 +1523,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 		//error
 		// function, file, line, error, explanation
 		String err_file;
-		if (p_instance && ObjectDB::instance_validate(p_instance->owner) && p_instance->script->is_valid() && p_instance->script->path != "") {
+		if (p_instance && !instance.is_invalid_object() && p_instance->script->is_valid() && p_instance->script->path != "") {
 			err_file = p_instance->script->path;
 		} else if (script) {
 			err_file = script->path;
@@ -1534,7 +1532,7 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a
 			err_file = "<built-in>";
 		}
 		String err_func = name;
-		if (p_instance && ObjectDB::instance_validate(p_instance->owner) && p_instance->script->is_valid() && p_instance->script->name != "") {
+		if (p_instance && !instance.is_invalid_object() && p_instance->script->is_valid() && p_instance->script->name != "") {
 			err_func = p_instance->script->name + "." + err_func;
 		}
 		int err_line = line;

+ 2 - 2
modules/gdscript/gdscript_function.h

@@ -81,7 +81,7 @@ struct GDScriptDataType {
 				}
 
 				Object *obj = p_variant.operator Object *();
-				if (!obj || !ObjectDB::instance_validate(obj)) {
+				if (!obj) {
 					return false;
 				}
 
@@ -104,7 +104,7 @@ struct GDScriptDataType {
 				}
 
 				Object *obj = p_variant.operator Object *();
-				if (!obj || !ObjectDB::instance_validate(obj)) {
+				if (!obj) {
 					return false;
 				}
 

+ 1 - 1
modules/gdscript/gdscript_functions.cpp

@@ -1382,7 +1382,7 @@ void GDScriptFunctions::call(Function p_func, const Variant **p_args, int p_arg_
 				r_ret = false;
 			} else {
 				Object *obj = *p_args[0];
-				r_ret = ObjectDB::instance_validate(obj);
+				r_ret = obj != nullptr;
 			}
 
 		} break;

+ 0 - 11
scene/animation/tween.cpp

@@ -1321,7 +1321,6 @@ bool Tween::_build_interpolation(InterpolateType p_interpolation_type, Object *p
 
 	// Give it the object
 	ERR_FAIL_COND_V_MSG(p_object == nullptr, false, "Invalid object provided to Tween.");
-	ERR_FAIL_COND_V_MSG(!ObjectDB::instance_validate(p_object), false, "Invalid object provided to Tween.");
 	data.id = p_object->get_instance_id();
 
 	// Validate the initial and final values
@@ -1439,7 +1438,6 @@ bool Tween::interpolate_callback(Object *p_object, real_t p_duration, String p_c
 
 	// Check that the target object is valid
 	ERR_FAIL_COND_V(p_object == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false);
 
 	// Duration cannot be negative
 	ERR_FAIL_COND_V(p_duration < 0, false);
@@ -1499,7 +1497,6 @@ bool Tween::interpolate_deferred_callback(Object *p_object, real_t p_duration, S
 
 	// Check that the target object is valid
 	ERR_FAIL_COND_V(p_object == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false);
 
 	// No negative durations allowed
 	ERR_FAIL_COND_V(p_duration < 0, false);
@@ -1574,9 +1571,7 @@ bool Tween::follow_property(Object *p_object, NodePath p_property, Variant p_ini
 
 	// Confirm the source and target objects are valid
 	ERR_FAIL_COND_V(p_object == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false);
 	ERR_FAIL_COND_V(p_target == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_target), false);
 
 	// No negative durations
 	ERR_FAIL_COND_V(p_duration < 0, false);
@@ -1642,9 +1637,7 @@ bool Tween::follow_method(Object *p_object, StringName p_method, Variant p_initi
 
 	// Verify the source and target objects are valid
 	ERR_FAIL_COND_V(p_object == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false);
 	ERR_FAIL_COND_V(p_target == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_target), false);
 
 	// No negative durations
 	ERR_FAIL_COND_V(p_duration < 0, false);
@@ -1712,9 +1705,7 @@ bool Tween::targeting_property(Object *p_object, NodePath p_property, Object *p_
 
 	// Verify both objects are valid
 	ERR_FAIL_COND_V(p_object == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false);
 	ERR_FAIL_COND_V(p_initial == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_initial), false);
 
 	// No negative durations
 	ERR_FAIL_COND_V(p_duration < 0, false);
@@ -1785,9 +1776,7 @@ bool Tween::targeting_method(Object *p_object, StringName p_method, Object *p_in
 
 	// Make sure the given objects are valid
 	ERR_FAIL_COND_V(p_object == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_object), false);
 	ERR_FAIL_COND_V(p_initial == nullptr, false);
-	ERR_FAIL_COND_V(!ObjectDB::instance_validate(p_initial), false);
 
 	// No negative durations
 	ERR_FAIL_COND_V(p_duration < 0, false);

+ 1 - 1
scene/debugger/script_debugger_remote.cpp

@@ -101,7 +101,7 @@ void ScriptDebuggerRemote::_put_variable(const String &p_name, const Variant &p_
 	packet_peer_stream->put_var(p_name);
 
 	Variant var = p_variable;
-	if (p_variable.get_type() == Variant::OBJECT && !ObjectDB::instance_validate(p_variable)) {
+	if (p_variable.get_type() == Variant::OBJECT && p_variable.operator Object *() == nullptr) {
 		var = Variant();
 	}