Skip to content

Commit

Permalink
Merge pull request #38119 from RandomShaper/imvu/new_dangling_variant…
Browse files Browse the repository at this point in the history
…_fix

Fix dangling Variants (3.2)
  • Loading branch information
akien-mga committed Apr 24, 2020
2 parents b04f092 + d904d05 commit 5d5848d
Show file tree
Hide file tree
Showing 9 changed files with 380 additions and 168 deletions.
44 changes: 44 additions & 0 deletions core/object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion core/object.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -397,7 +402,7 @@ public: \
private:

class ScriptInstance;
typedef uint64_t ObjectID;
class ObjectRC;

class Object {
public:
Expand Down Expand Up @@ -477,6 +482,9 @@ class Object {
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;
Expand Down Expand Up @@ -587,6 +595,10 @@ class Object {
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; }
Expand Down
38 changes: 38 additions & 0 deletions core/object_id.h
Original file line number Diff line number Diff line change
@@ -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
75 changes: 75 additions & 0 deletions core/object_rc.h
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit 5d5848d

Please sign in to comment.