Skip to content

Don't reuse the same list for _get_property_list() #1904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akien-mga merged 1 commit into from
Jan 20, 2026
Merged

Don't reuse the same list for _get_property_list() #1904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions include/godot_cpp/classes/wrapped.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ class Wrapped {
static GDExtensionBool validate_property_bind(GDExtensionClassInstancePtr p_instance, GDExtensionPropertyInfo *p_property) { return false; }
static void to_string_bind(GDExtensionClassInstancePtr p_instance, GDExtensionBool *r_is_valid, GDExtensionStringPtr r_out) {}

// The only reason this has to be held here, is when we return results of `_get_property_list` to Godot, we pass
// pointers to strings in this list. They have to remain valid to pass the bridge, until the list is freed by Godot...
::godot::List<::godot::PropertyInfo> plist_owned;

void _postinitialize();
virtual void _notificationv(int32_t p_what, bool p_reversed = false) {}

Expand Down Expand Up @@ -156,7 +152,7 @@ _FORCE_INLINE_ Vector<StringName> snarray(P... p_args) {

namespace internal {

GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size);
GDExtensionPropertyInfo *create_c_property_list(::godot::List<::godot::PropertyInfo> *plist_cpp, uint32_t *r_size);
void free_c_property_list(GDExtensionPropertyInfo *plist);

typedef void (*EngineClassRegistrationCallback)();
Expand Down Expand Up @@ -317,16 +313,14 @@ public:
return nullptr; \
} \
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
::godot::List<::godot::PropertyInfo> &plist_cpp = cls->plist_owned; \
ERR_FAIL_COND_V_MSG(!plist_cpp.is_empty(), nullptr, "Internal error, property list was not freed by engine!"); \
cls->_get_property_list(&plist_cpp); \
::godot::List<::godot::PropertyInfo> *plist_cpp = memnew(::godot::List<::godot::PropertyInfo>); \
cls->_get_property_list(plist_cpp); \
return ::godot::internal::create_c_property_list(plist_cpp, r_count); \
} \
\
static void free_property_list_bind(GDExtensionClassInstancePtr p_instance, const GDExtensionPropertyInfo *p_list, uint32_t /*p_count*/) { \
if (p_instance) { \
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
cls->plist_owned.clear(); \
::godot::internal::free_c_property_list(const_cast<GDExtensionPropertyInfo *>(p_list)); \
} \
} \
Expand Down
20 changes: 14 additions & 6 deletions src/classes/wrapped.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,21 @@ std::vector<EngineClassRegistrationCallback> &get_engine_class_registration_call
return engine_class_registration_callbacks;
}

GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size) {
GDExtensionPropertyInfo *plist = nullptr;
GDExtensionPropertyInfo *create_c_property_list(::godot::List<::godot::PropertyInfo> *plist_cpp, uint32_t *r_size) {
// Linked list size can be expensive to get so we cache it
const uint32_t plist_size = plist_cpp.size();
const uint32_t plist_size = plist_cpp->size();
if (r_size != nullptr) {
*r_size = plist_size;
}
plist = reinterpret_cast<GDExtensionPropertyInfo *>(memalloc(sizeof(GDExtensionPropertyInfo) * plist_size));

// Use the padding to stash the plist_cpp pointer, so we can clean it up later.
void *mem = ::godot::Memory::alloc_static(sizeof(GDExtensionPropertyInfo) * plist_size, true);
GDExtensionPropertyInfo *plist = reinterpret_cast<GDExtensionPropertyInfo *>(mem);
::godot::List<::godot::PropertyInfo> **plist_cpp_stash = reinterpret_cast<::godot::List<::godot::PropertyInfo> **>((uint8_t *)mem - ::godot::Memory::DATA_OFFSET + ::godot::Memory::ELEMENT_OFFSET);
*plist_cpp_stash = plist_cpp;

unsigned int i = 0;
for (const ::godot::PropertyInfo &E : plist_cpp) {
for (const ::godot::PropertyInfo &E : *plist_cpp) {
plist[i].type = static_cast<GDExtensionVariantType>(E.type);
plist[i].name = E.name._native_ptr();
plist[i].hint = E.hint;
Expand All @@ -138,7 +143,10 @@ GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::Pro
}

void free_c_property_list(GDExtensionPropertyInfo *plist) {
memfree(plist);
// Get the stashed plist_cpp pointer, before we free the memory that's holding it.
::godot::List<::godot::PropertyInfo> *plist_cpp = *reinterpret_cast<::godot::List<::godot::PropertyInfo> **>((uint8_t *)plist - ::godot::Memory::DATA_OFFSET + ::godot::Memory::ELEMENT_OFFSET);
::godot::Memory::free_static(plist, true);
memdelete(plist_cpp);
}

void add_engine_class_registration_callback(EngineClassRegistrationCallback p_callback) {
Expand Down
Loading