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

Merge pull request #38257 from RandomShaper/imvu/fix_variantrc_gdnative 

Fix GDNative compat breakage due to dangling Variants fix
This commit is contained in:
Rémi Verschelde 2020-04-27 18:43:10 +02:00 committed by GitHub
parent 4dda00461f 3bdc0913a5
commit 78f0cf40cb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 22 additions and 25 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

8
core/variant_call.cpp
View File

@ -1098,8 +1098,8 @@ 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 a deleted object.");
} }
#endif #endif
r_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL; r_error.error = CallError::CALL_ERROR_INSTANCE_IS_NULL;
@ -1275,8 +1275,8 @@ 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 a deleted object.");
} }
#endif #endif
return false; return false;

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();