GDExtension forces binding to keep property-related strings and lists allocated

[Bromeon]

Godot version

4.0.dev (8df8fff)

System information

Windows 10

Issue description

Possible issue with the callbacks in the extension API:

// e.g. this callback in GDNativeExtensionClassCreationInfo:
GDNativeExtensionClassGetPropertyList get_property_list_func;

// is defined as:
typedef const GDNativePropertyInfo *
    (*GDNativeExtensionClassGetPropertyList)(GDExtensionClassInstancePtr p_instance, uint32_t *r_count);
typedef struct {
	uint32_t type;
	const char *name;
	const char *class_name;
	uint32_t hint;
	const char *hint_string;
	uint32_t usage;
} GDNativePropertyInfo;

So, the extension binding returns a list of property infos. Each property info stores several char* strings.

The list and all those strings must be allocated somewhere, so they are still valid when the callback returns and Godot reads them.
The current design leaves only one option: the binding must keep that data around (statically).

It's not possible to generate it on-demand, because there's no way to move it "into the engine" (i.e. allocator function, and surrendering ownership when returning it from the callback).

Is this intended?
Are there certain guarantees that would allow to cache strings more short term (e.g. "if the callback is invoked next time, you can free the values from the last callback invocation"), which would allow to keep memory usage limited?

Steps to reproduce

Try to stack-allocate one of the strings to return, and UB will be invoked.

Minimal reproduction project

No response

[touilleMan]

@Bromeon now that #67750 is merged, can this be closed ?

[Bromeon]

Let me check how the new implementation would look in godot-rust, and come back with feedback 🙂

[reduz]

This issue is still open. I am still unsure why this was a problem originally nor whether this needed to be changed, because if you wanted to allocate all these strings on the fly and then remove them just for this function, it would have been fine too. There was no requirement to keep them in memory, nor this is performance critical.

[Bromeon]

As far as I can see, this has been fixed in #67750; I no longer need to keep char*s allocated on Rust side 🙂

@reduz The problem in this case was the binding had to allocate the objects/strings, and return them to the engine. Since the callstack was inverted (Godot invokes extension callbacks), the objects needed to outlive the callback defined by the extension user.

So even if the callback could have transferred ownership to Godot upon returning pointers, how was it supposed to allocate char*? malloc()? There was no way to know how the engine would deallocate them. So the current design which uses StringName, String and all these types with defined function pointers for constructors and destructors is much cleaner.