Segmentation fault when assigning keys to a godot dictionary

Help
,
1

Godot Version

Godot4.5

Question

I am facing a segmentation fault when running the following GDScript code with my GDExtension loaded:

func _ready() -> void:
	var s = SmageSession.new();
	print(s.get_state());

The code for the method simply converts a cpp dictionary to a godot one:


Dictionary SmageSession::get_state(){
	Dictionary* result2 = memnew(Dictionary());
	
	vector<item_state> paia = {"under bench", "in pocket"};

	map<item_name, vector<item_state>> b = {};
	
	b["newspaper"] = paia;
	
	for(auto it=b.begin(); it!= b.end();++it){
		string item_name = it->first;
		vector<item_state> i_states = it->second;
		
		String gd_i_name = String(item_name.c_str());
		
		Array* pair = memnew(Array());
		for(item_state s: i_states){
			String gd_string = String(s.c_str());
			pair->append(gd_string);
		}
		
		Variant v1 = Variant(gd_i_name);
		Variant v2 = Variant(pair);
		
		result2->set(v1, v2);
	}
	return *result2;
};

From running the program in gdb, the segmentation fault occurs when result2->set(v1, v2); is run.

Any tips or pointers will be really helpful as I could not find much documentation of the godot-cpp layer. Most of my notes come from previous trial and error:

  • It is best to create new godot arrays using Array* gd_array = memnew(Array());. Likewise for Dictionaries.

  • Objects are never passed directly. A Reference to the godot object is created. Reference can be created with Ref<SmageSession> session_ref = Ref<SmageSession>(session) where session is a SmageSession*, and SmageSession inherits godot’s RefCounted.

  • To make an object reference null, call session_ref.unref()

I have uploaded the minimum reprroduction project to al2f/smage-gdextension-segfault - Codeberg.org.

2

Hi there,

The quick answer: you’re misusing reference types.

memnew() returns raw pointers, but Dictionary and Array are RefCounted derived, so you need to wrap those returned pointers up immediately, as they come with a refcount of 0.

So, as is, neither your Dictionary result2 nor any Array pair in the loop are referenced and they are getting cleaned up. To fix, wrap them up in Ref<>s:

	Ref<Dictionary> result2 = Ref<Dictionary>(memnew(Dictionary));

	// ...

		Ref<Array> pair = Ref<Array>(memnew(Array));

If you find yourself handling raw pointers while dealing with (most) Godot types, you have almost certainly gone astray :slight_smile:

Of your self intuited tips, only the second makes any sense to me; and you should be doing it on creation (see code snippets above) and then just dealing with Ref<>s, they will then manage themselves (incrementing refcount if you copy it, decrementing refcount when they go out of scope, and deleting when refcount == 0).

DO NOT call unref() manually!

NB: Or get rid of the pointers entirely, stack allocate and trust the compiler. Godot will wrap up a copy in a Ref<> for you if you pass it on to script.

3

Thanks for the tips. As I said, I was mostly finding my way around in the dark when it came to writing code from my limited knowledge of C++ and trying what worked. When dealing with the created Refs, how would use them to append a value to a dictionary/array? Just call Variant(array_ref) or something else?

4

You use a Ref<> largely the same way you use a pointer, -> is overloaded, and your existing code should work without change.

Note that after all that you are just making a copy to return at the end, return a Ref<> it you didn’t want that.

I would recommend not doing any of that though, just make the Dictionary on the stack, populate it and return it; the compiler will RVO it, the implied copy operations will not actually exist and your code will be much easier on the eye.

5

Ref<> is not supposed to be used with dictionaries and arrays. Just create on stack and return.

6

I have tried both Ref<Array> pair = Ref(memnew(Array));and Ref<Array> pair = Ref(memnew(Array()));(which I was using previously) , as @silnarm suggested which results in a build error:

godot-cpp/include/godot_cpp/classes/ref.hpp: In instantiation of 'void godot::Ref<T>::ref_pointer(T*) [with T = godot::Array]':
godot-cpp/include/godot_cpp/classes/ref.hpp:183:4:   required from 'godot::Ref<T>::Ref(T*) [with T = godot::Array]'
  183 |                         ref_pointer(p_reference);
      |                         ^~~~~~~~~~~
src/gdexample.cpp:283:45:   required from here
  283 |                 Ref<Array> pair = Ref<Array>(memnew(Array));
      |                                                           ^
godot-cpp/include/godot_cpp/classes/ref.hpp:67:28: error: 'class godot::Array' has no member named 'init_ref'
   67 |                 if (p_ref->init_ref()) {
      |                     ~~~~~~~^~~~~~~~
godot-cpp/include/godot_cpp/classes/ref.hpp: In instantiation of 'void godot::Ref<T>::unref() [with T = godot::Array]':
godot-cpp/include/godot_cpp/classes/ref.hpp:220:3:   required from 'godot::Ref<T>::~Ref() [with T = godot::Array]'
  220 |                 unref();
      |                 ^~~~~
src/gdexample.cpp:283:45:   required from here
  283 |                 Ref<Array> pair = Ref<Array>(memnew(Array));
      |                                                           ^
godot-cpp/include/godot_cpp/classes/ref.hpp:207:45: error: 'class godot::Array' has no member named 'unreference'
  207 |                 if (reference && reference->unreference()) {
      |                                  ~~~~~~~~~~~^~~~~~~~~~~

When you mentioned ‘create on stack and return’ what did you mean?

7

My bad sorry, I made some bad assumptions, as per @normalized’s post.

Dictionary is not RefCounted, in fact it is not even a godot::Object, so allocating dynamically with memnew() is wrong too.
Looks like the actual data is stored “elsewhere” (and is ref counted), so they are very lightweight to copy.

Anyway, create on stack and return means this:

	Dictionary result{};
	result["foo"] = bar;
	return result;

No pointer, no Ref, no dynamic allocation (by you, it is happening behind the scenes).

1 Like
8

Thanks! The code no longer gives a segmentation fault. The corrected code for any future forum wanderers(where SmageActionResponse extends RefCounted:

Dictionary SmageActionResponse::get_delta(){
	Dictionary result2;
	
	for(auto it=this->response.delta.begin(); it!= this->response.delta.end();++it){
		string item_name = it->first;
		vector<item_state> i_states = it->second;
		
		String gd_i_name = String(item_name.c_str());
		
		Array pair;
		for(item_state s: i_states){
			String gd_string = String(s.c_str());
			pair.append(gd_string);
		}
		
		Variant v1 = Variant(gd_i_name);
		Variant v2 = Variant(pair);
		
		result2.set(v1, v2);
	}
	return result2;
}