Don't reuse the same list for _get_property_list()

This commit is contained in:
David Snopek
2026-01-19 16:52:02 -06:00
parent d9eab9ee45
commit 6d69a6219a
2 changed files with 17 additions and 15 deletions

View File

@@ -105,10 +105,6 @@ protected:
static GDExtensionBool validate_property_bind(GDExtensionClassInstancePtr p_instance, GDExtensionPropertyInfo *p_property) { return false; } 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) {} 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(); void _postinitialize();
virtual void _notificationv(int32_t p_what, bool p_reversed = false) {} virtual void _notificationv(int32_t p_what, bool p_reversed = false) {}
@@ -156,7 +152,7 @@ _FORCE_INLINE_ Vector<StringName> snarray(P... p_args) {
namespace internal { 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); void free_c_property_list(GDExtensionPropertyInfo *plist);
typedef void (*EngineClassRegistrationCallback)(); typedef void (*EngineClassRegistrationCallback)();
@@ -317,16 +313,14 @@ public:
return nullptr; \ return nullptr; \
} \ } \
m_class *cls = reinterpret_cast<m_class *>(p_instance); \ m_class *cls = reinterpret_cast<m_class *>(p_instance); \
::godot::List<::godot::PropertyInfo> &plist_cpp = cls->plist_owned; \ ::godot::List<::godot::PropertyInfo> *plist_cpp = memnew(::godot::List<::godot::PropertyInfo>); \
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); \
cls->_get_property_list(&plist_cpp); \
return ::godot::internal::create_c_property_list(plist_cpp, r_count); \ 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*/) { \ static void free_property_list_bind(GDExtensionClassInstancePtr p_instance, const GDExtensionPropertyInfo *p_list, uint32_t /*p_count*/) { \
if (p_instance) { \ if (p_instance) { \
m_class *cls = reinterpret_cast<m_class *>(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)); \ ::godot::internal::free_c_property_list(const_cast<GDExtensionPropertyInfo *>(p_list)); \
} \ } \
} \ } \

View File

@@ -116,16 +116,21 @@ std::vector<EngineClassRegistrationCallback> &get_engine_class_registration_call
return engine_class_registration_callbacks; return engine_class_registration_callbacks;
} }
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) {
GDExtensionPropertyInfo *plist = nullptr;
// Linked list size can be expensive to get so we cache it // 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) { if (r_size != nullptr) {
*r_size = plist_size; *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; 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].type = static_cast<GDExtensionVariantType>(E.type);
plist[i].name = E.name._native_ptr(); plist[i].name = E.name._native_ptr();
plist[i].hint = E.hint; plist[i].hint = E.hint;
@@ -138,7 +143,10 @@ GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::Pro
} }
void free_c_property_list(GDExtensionPropertyInfo *plist) { 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) { void add_engine_class_registration_callback(EngineClassRegistrationCallback p_callback) {