godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix GDNative compat breakage due to dangling Variants fix 

This moves the instance id member from Variant to the ObjectRC so that Variant is still the same size as before the fix (and also regardless if debug or release build).
This commit is contained in:
Pedro J. Estébanez 2020-04-27 13:07:52 +02:00
parent 0233b7e51f
commit ac8b4708a3
5 changed files with 20 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
core/object_rc.h
View File

@ -46,6 +46,11 @@ class ObjectRC {
std::atomic<uint32_t> _users; std::atomic<uint32_t> _users;
public: public:
// This is for allowing debug builds 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.
const ObjectID instance_id;
_FORCE_INLINE_ void increment() { _FORCE_INLINE_ void increment() {
_users.fetch_add(1, std::memory_order_relaxed); _users.fetch_add(1, std::memory_order_relaxed);
} }
@ -63,7 +68,8 @@ public:
return _ptr.load(std::memory_order_acquire); return _ptr.load(std::memory_order_acquire);
} }
_FORCE_INLINE_ ObjectRC(Object *p_object) { _FORCE_INLINE_ ObjectRC(Object *p_object) :
instance_id(p_object->get_instance_id()) {
// 1 (the Object) + 1 (the first user) // 1 (the Object) + 1 (the first user)
_users.store(2, std::memory_order_relaxed); _users.store(2, std::memory_order_relaxed);
_ptr.store(p_object, std::memory_order_release); _ptr.store(p_object, std::memory_order_release);

7
core/variant.cpp
View File

@ -1125,7 +1125,6 @@ void Variant::clear() {
if (_get_obj().rc->decrement()) { if (_get_obj().rc->decrement()) {
memfree(_get_obj().rc); memfree(_get_obj().rc);
} }
_get_obj().instance_id = 0;
} else { } else {
_get_obj().ref.unref(); _get_obj().ref.unref();
} }
@ -1602,7 +1601,7 @@ String Variant::stringify(List<const void *> &stack) const {
return obj->to_string(); return obj->to_string();
} else { } else {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
return "[Deleted Object]"; return "[Deleted Object]";
} }
#endif #endif
@ -1765,7 +1764,7 @@ Variant::operator RID() const {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
Object *obj = _get_obj().rc->get_ptr(); Object *obj = _get_obj().rc->get_ptr();
if (unlikely(!obj)) { if (unlikely(!obj)) {
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted get RID on a deleted object."); WARN_PRINT("Attempted get RID on a deleted object.");
} }
} }
@ -2318,7 +2317,6 @@ Variant::Variant(const RefPtr &p_resource) {
memnew_placement(_data._mem, ObjData); memnew_placement(_data._mem, ObjData);
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
_get_obj().rc = NULL; _get_obj().rc = NULL;
_get_obj().instance_id = 0;
#else #else
REF *ref = reinterpret_cast<REF *>(p_resource.get_data()); REF *ref = reinterpret_cast<REF *>(p_resource.get_data());
_get_obj().obj = ref->ptr(); _get_obj().obj = ref->ptr();
@ -2339,7 +2337,6 @@ Variant::Variant(const Object *p_object) {
memnew_placement(_data._mem, ObjData); memnew_placement(_data._mem, ObjData);
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
_get_obj().rc = p_object ? const_cast<Object *>(p_object)->_use_rc() : NULL; _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 #else
_get_obj().obj = const_cast<Object *>(p_object); _get_obj().obj = const_cast<Object *>(p_object);
#endif #endif

6
core/variant.h
View File

@ -139,12 +139,6 @@ private:
// Will be null for every type deriving from Reference as they have their // Will be null for every type deriving from Reference as they have their
// own reference count mechanism // own reference count mechanism
ObjectRC *rc; 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 #else
Object *obj; Object *obj;
#endif #endif

4
core/variant_call.cpp
View File

@ -1098,7 +1098,7 @@ void Variant::call_ptr(const StringName &p_method, const Variant **p_args, int p
Object *obj = _OBJ_PTR(*this); Object *obj = _OBJ_PTR(*this);
if (!obj) { if (!obj) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted call on stray pointer object."); WARN_PRINT("Attempted call on stray pointer object.");
} }
#endif #endif
@ -1275,7 +1275,7 @@ bool Variant::has_method(const StringName &p_method) const {
Object *obj = _OBJ_PTR(*this); Object *obj = _OBJ_PTR(*this);
if (!obj) { if (!obj) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted method check on stray pointer object."); WARN_PRINT("Attempted method check on stray pointer object.");
} }
#endif #endif

18
core/variant_op.cpp
View File

@ -1516,7 +1516,7 @@ void Variant::set_named(const StringName &p_index, const Variant &p_value, bool
Object *obj = _OBJ_PTR(*this); Object *obj = _OBJ_PTR(*this);
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (unlikely(!obj)) { if (unlikely(!obj)) {
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted set on a deleted object."); WARN_PRINT("Attempted set on a deleted object.");
} }
break; break;
@ -1684,7 +1684,7 @@ Variant Variant::get_named(const StringName &p_index, bool *r_valid) const {
if (unlikely(!obj)) { if (unlikely(!obj)) {
if (r_valid) if (r_valid)
*r_valid = false; *r_valid = false;
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted get on a deleted object."); WARN_PRINT("Attempted get on a deleted object.");
} }
return Variant(); return Variant();
@ -2169,7 +2169,7 @@ void Variant::set(const Variant &p_index, const Variant &p_value, bool *r_valid)
if (unlikely(!obj)) { if (unlikely(!obj)) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
valid = false; valid = false;
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted set on a deleted object."); WARN_PRINT("Attempted set on a deleted object.");
} }
#endif #endif
@ -2539,7 +2539,7 @@ Variant Variant::get(const Variant &p_index, bool *r_valid) const {
if (unlikely(!obj)) { if (unlikely(!obj)) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
valid = false; valid = false;
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted get on a deleted object."); WARN_PRINT("Attempted get on a deleted object.");
} }
#endif #endif
@ -2602,7 +2602,7 @@ bool Variant::in(const Variant &p_index, bool *r_valid) const {
if (r_valid) { if (r_valid) {
*r_valid = false; *r_valid = false;
} }
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted 'in' on a deleted object."); WARN_PRINT("Attempted 'in' on a deleted object.");
} }
#endif #endif
@ -2865,7 +2865,7 @@ void Variant::get_property_list(List<PropertyInfo> *p_list) const {
Object *obj = _OBJ_PTR(*this); Object *obj = _OBJ_PTR(*this);
if (unlikely(!obj)) { if (unlikely(!obj)) {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted get property list on a deleted object."); WARN_PRINT("Attempted get property list on a deleted object.");
} }
#endif #endif
@ -2943,7 +2943,7 @@ bool Variant::iter_init(Variant &r_iter, bool &valid) const {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (unlikely(!obj)) { if (unlikely(!obj)) {
valid = false; valid = false;
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted iteration start on a deleted object."); WARN_PRINT("Attempted iteration start on a deleted object.");
} }
return false; return false;
@ -3110,7 +3110,7 @@ bool Variant::iter_next(Variant &r_iter, bool &valid) const {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (unlikely(!obj)) { if (unlikely(!obj)) {
valid = false; valid = false;
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted iteration check next on a deleted object."); WARN_PRINT("Attempted iteration check next on a deleted object.");
} }
return false; return false;
@ -3268,7 +3268,7 @@ Variant Variant::iter_get(const Variant &r_iter, bool &r_valid) const {
#ifdef DEBUG_ENABLED #ifdef DEBUG_ENABLED
if (unlikely(!obj)) { if (unlikely(!obj)) {
r_valid = false; r_valid = false;
if (ScriptDebugger::get_singleton() && _get_obj().instance_id != 0 && !ObjectDB::get_instance(_get_obj().instance_id)) { if (ScriptDebugger::get_singleton() && _get_obj().rc && !ObjectDB::get_instance(_get_obj().rc->instance_id)) {
WARN_PRINT("Attempted iteration get next on a deleted object."); WARN_PRINT("Attempted iteration get next on a deleted object.");
} }
return Variant(); return Variant();