Construct Variants from Reference properly in GDNative

Previously godot_variant_new_object constructed Variant without
accounting for the fact that the Object can be a Reference, so refcount
was not increased and References were destructed prematurely.

Also, Reference::init_ref did not propagate refcount increment to the
script instance, which led to desync of refcount info on the script
side and Godot side.
This commit is contained in:
Ruslan Mustakov 2017-08-19 19:41:11 +07:00
parent 9ac940677c
commit f08bc0df7c
4 changed files with 27 additions and 9 deletions

View file

@ -33,7 +33,7 @@
bool Reference::init_ref() { bool Reference::init_ref() {
if (refcount.ref()) { if (reference()) {
// this may fail in the scenario of two threads assigning the pointer for the FIRST TIME // this may fail in the scenario of two threads assigning the pointer for the FIRST TIME
// at the same time, which is never likely to happen (would be crazy to do) // at the same time, which is never likely to happen (would be crazy to do)
@ -41,7 +41,7 @@ bool Reference::init_ref() {
if (refcount_init.get() > 0) { if (refcount_init.get() > 0) {
refcount_init.unref(); refcount_init.unref();
refcount.unref(); // first referencing is already 1, so compensate for the ref above unreference(); // first referencing is already 1, so compensate for the ref above
} }
return true; return true;
@ -62,13 +62,16 @@ int Reference::reference_get_count() const {
return refcount.get(); return refcount.get();
} }
void Reference::reference() { bool Reference::reference() {
bool success = refcount.ref();
refcount.ref(); if (success && get_script_instance()) {
if (get_script_instance()) {
get_script_instance()->refcount_incremented(); get_script_instance()->refcount_incremented();
} }
return success;
} }
bool Reference::unreference() { bool Reference::unreference() {
bool die = refcount.unref(); bool die = refcount.unref();

View file

@ -51,7 +51,7 @@ protected:
public: public:
_FORCE_INLINE_ bool is_referenced() const { return refcount_init.get() < 1; } _FORCE_INLINE_ bool is_referenced() const { return refcount_init.get() < 1; }
bool init_ref(); bool init_ref();
void reference(); bool reference(); // returns false if refcount is at zero and didn't get increased
bool unreference(); bool unreference();
int reference_get_count() const; int reference_get_count() const;

View file

@ -2259,8 +2259,8 @@ Variant::Variant(const RefPtr &p_resource) {
type = OBJECT; type = OBJECT;
memnew_placement(_data._mem, ObjData); memnew_placement(_data._mem, ObjData);
REF ref = p_resource; REF *ref = reinterpret_cast<REF *>(p_resource.get_data());
_get_obj().obj = ref.ptr(); _get_obj().obj = ref->ptr();
_get_obj().ref = p_resource; _get_obj().ref = p_resource;
} }

View file

@ -29,6 +29,7 @@
/*************************************************************************/ /*************************************************************************/
#include "gdnative/variant.h" #include "gdnative/variant.h"
#include "core/reference.h"
#include "core/variant.h" #include "core/variant.h"
#ifdef __cplusplus #ifdef __cplusplus
@ -158,7 +159,21 @@ void GDAPI godot_variant_new_rid(godot_variant *r_dest, const godot_rid *p_rid)
void GDAPI godot_variant_new_object(godot_variant *r_dest, const godot_object *p_obj) { void GDAPI godot_variant_new_object(godot_variant *r_dest, const godot_object *p_obj) {
Variant *dest = (Variant *)r_dest; Variant *dest = (Variant *)r_dest;
Object *obj = (Object *)p_obj; Object *obj = (Object *)p_obj;
Reference *reference = Object::cast_to<Reference>(obj);
REF ref;
if (reference) {
ref = REF(reference);
}
if (!ref.is_null()) {
memnew_placement_custom(dest, Variant, Variant(ref.get_ref_ptr()));
} else {
#if defined(DEBUG_METHODS_ENABLED)
if (reference) {
ERR_PRINT("Reference object has 0 refcount in godot_variant_new_object - you lost it somewhere.");
}
#endif
memnew_placement_custom(dest, Variant, Variant(obj)); memnew_placement_custom(dest, Variant, Variant(obj));
}
} }
void GDAPI godot_variant_new_dictionary(godot_variant *r_dest, const godot_dictionary *p_dict) { void GDAPI godot_variant_new_dictionary(godot_variant *r_dest, const godot_dictionary *p_dict) {