godot-cpp: Incorrect _Wrapped::_owner initialization

I have a GDNative class extending Translation. When I call _new I get pointer to it where _owner field initialized to incorrect memory address:

    GodotI18n *i18n = GodotI18n::_new();

(gdb) p i18n
$17 = (godot::GodotI18n *) 0xe4414b0
(gdb) p i18n->_owner
$18 = (godot_object *) 0xe441

and next it fails in reference counter

Thread 1 "godot.x11.tools" received signal SIGSEGV, Segmentation fault.
0x0000000003a0f398 in atomic_conditional_increment<unsigned int> (pw=0xe539) at ./core/safe_refcount.h:107
107			T tmp = static_cast<T const volatile &>(*pw);
(gdb) bt
#0  0x0000000003a0f398 in atomic_conditional_increment<unsigned int> (pw=0xe539) at ./core/safe_refcount.h:107
#1  SafeRefCount::refval (this=0xe539) at ./core/safe_refcount.h:187
#2  Reference::reference (this=0xe441) at core/reference.cpp:63
#3  0x000000000191c360 in MethodBind0R<bool>::ptrcall (this=0x5663570, p_object=0xe441, p_args=0x7fffffffc3b0, r_ret=0x7fffffffc3af) at ./core/method_bind.gen.inc:244
#4  0x0000000001942d2b in godot_method_bind_ptrcall (p_method_bind=0x5663570, p_instance=0xe441, p_args=0x7fffffffc3b0, p_ret=0x7fffffffc3af) at modules/gdnative/gdnative/gdnative.cpp:69
#5  0x00007fffe2dfb2e8 in godot::___godot_icall_bool (inst=0xe4414b0, mb=<optimized out>) at /mnt/another/srcs/GIT/freeorion/godot/godot-cpp/include/gen/__icalls.hpp:7378
#6  godot::Reference::reference (this=this@entry=0xe4414b0) at /mnt/another/srcs/GIT/freeorion/godot/godot-cpp/src/gen/Reference.cpp:34
#7  0x00007fffe2dd422d in godot::Ref<godot::Translation>::ref (p_from=..., this=0x7fffffffc4a0) at /mnt/another/srcs/GIT/freeorion/godot/godot-cpp/include/core/Ref.hpp:123
#8  godot::Ref<godot::Translation>::Ref (p_from=..., this=0x7fffffffc4a0) at /mnt/another/srcs/GIT/freeorion/godot/godot-cpp/include/core/Ref.hpp:126
#9  godot::GDFreeOrion::_init (this=this@entry=0xecc5050) at /mnt/another/srcs/GIT/freeorion/godot/gdfreeorion.cpp:200
#10 0x00007fffe2dd177b in godot::_godot_class_instance_func<godot::GDFreeOrion> (p=0xdf60a30, method_data=<optimized out>) at /usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/include/g++-v9/typeinfo:143
#11 0x0000000001947826 in NativeScript::instance_create (this=0xe4c08f0, p_this=0xdf60a30) at modules/gdnative/nativescript/nativescript.cpp:220
#12 0x00000000039e5794 in Object::set_script (this=0xdf60a30, p_script=...) at core/object.cpp:1030
#13 0x00000000039e258b in Object::set (this=0xdf60a30, p_name=..., p_value=..., r_valid=0x7fffffffc9e8) at core/object.cpp:433
#14 0x0000000003386c77 in SceneState::instance (this=0xeb645d0, p_edit_state=SceneState::GEN_EDIT_STATE_DISABLED) at scene/resources/packed_scene.cpp:212
#15 0x0000000003391e5b in PackedScene::instance (this=0xe25f370, p_edit_state=PackedScene::GEN_EDIT_STATE_DISABLED) at scene/resources/packed_scene.cpp:1698
#16 0x000000000145c135 in Main::start () at main/main.cpp:1942
#17 0x000000000141a4d0 in main (argc=1, argv=0x7fffffffd5c8) at platform/x11/godot_x11.cpp:55

OS: Gentoo Linux AMD64 Version: 3.2

About this issue

Original URL
State: open
Created 4 years ago
Comments: 19 (10 by maintainers)

Most upvoted comments

FYI, I confirm this even reproduces directly in Godot’s code:

Passing a naked pointer to ref to a Godot function which doesn’t take ownership of it is a no-go if you didn’t take ownership yourself. And you can’t do that from a C++ constructor because it would always prevent refcount initialization to properly finish before something else takes ownership. Even if Godot raised an error about the set being invalid, it would carry on as usual and your crash would still happen. The result is a dangling pointer to a deleted object. Depending on your debugger the contents of the object might be filled with a pattern like 0xfefefefefefefe or 0xbaadf00d.

In GDScript this doesn’t happen because this is what it gets inerpreted into instead:

Observe the difference of order in the printouts. Due to how GDScript works, when _init is called, something else already holds an initialized reference count. While in C++ this can’t happen without moving the set logic outside the constructor.

If you desperately need it, you might be able to do this horror:

	void _init()
	{
		// Initialize refcount
		Ref<Thing> test(this);
		// Add an artificial reference
		this->reference();

		// Pass it to whatever Godot things...
	}

	// and then somewhere outside:
	// remove the artificial reference
	thing->unreference()

Otherwise the behavior is as intented.

Zylann on Dec 18, 2020

I expanded my reasoning about that part in the message above. I didnt run anything yet, but I think this is your issue.

You might be able to reproduce it like that:

	void _init()
	{
		Ref<Thing> test(this);
	}

Zylann on Dec 18, 2020