Skip to content

Commit 6d69a62

Browse files
committed
Don't reuse the same list for _get_property_list()
1 parent d9eab9e commit 6d69a62

File tree

2 files changed

+17
-15
lines changed

2 files changed

+17
-15
lines changed

include/godot_cpp/classes/wrapped.hpp

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ class Wrapped {
105105
static GDExtensionBool validate_property_bind(GDExtensionClassInstancePtr p_instance, GDExtensionPropertyInfo *p_property) { return false; }
106106
static void to_string_bind(GDExtensionClassInstancePtr p_instance, GDExtensionBool *r_is_valid, GDExtensionStringPtr r_out) {}
107107

108-
// The only reason this has to be held here, is when we return results of `_get_property_list` to Godot, we pass
109-
// pointers to strings in this list. They have to remain valid to pass the bridge, until the list is freed by Godot...
110-
::godot::List<::godot::PropertyInfo> plist_owned;
111-
112108
void _postinitialize();
113109
virtual void _notificationv(int32_t p_what, bool p_reversed = false) {}
114110

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

157153
namespace internal {
158154

159-
GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size);
155+
GDExtensionPropertyInfo *create_c_property_list(::godot::List<::godot::PropertyInfo> *plist_cpp, uint32_t *r_size);
160156
void free_c_property_list(GDExtensionPropertyInfo *plist);
161157

162158
typedef void (*EngineClassRegistrationCallback)();
@@ -317,16 +313,14 @@ public:
317313
return nullptr; \
318314
} \
319315
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
320-
::godot::List<::godot::PropertyInfo> &plist_cpp = cls->plist_owned; \
321-
ERR_FAIL_COND_V_MSG(!plist_cpp.is_empty(), nullptr, "Internal error, property list was not freed by engine!"); \
322-
cls->_get_property_list(&plist_cpp); \
316+
::godot::List<::godot::PropertyInfo> *plist_cpp = memnew(::godot::List<::godot::PropertyInfo>); \
317+
cls->_get_property_list(plist_cpp); \
323318
return ::godot::internal::create_c_property_list(plist_cpp, r_count); \
324319
} \
325320
\
326321
static void free_property_list_bind(GDExtensionClassInstancePtr p_instance, const GDExtensionPropertyInfo *p_list, uint32_t /*p_count*/) { \
327322
if (p_instance) { \
328323
m_class *cls = reinterpret_cast<m_class *>(p_instance); \
329-
cls->plist_owned.clear(); \
330324
::godot::internal::free_c_property_list(const_cast<GDExtensionPropertyInfo *>(p_list)); \
331325
} \
332326
} \

src/classes/wrapped.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,21 @@ std::vector<EngineClassRegistrationCallback> &get_engine_class_registration_call
116116
return engine_class_registration_callbacks;
117117
}
118118

119-
GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size) {
120-
GDExtensionPropertyInfo *plist = nullptr;
119+
GDExtensionPropertyInfo *create_c_property_list(::godot::List<::godot::PropertyInfo> *plist_cpp, uint32_t *r_size) {
121120
// Linked list size can be expensive to get so we cache it
122-
const uint32_t plist_size = plist_cpp.size();
121+
const uint32_t plist_size = plist_cpp->size();
123122
if (r_size != nullptr) {
124123
*r_size = plist_size;
125124
}
126-
plist = reinterpret_cast<GDExtensionPropertyInfo *>(memalloc(sizeof(GDExtensionPropertyInfo) * plist_size));
125+
126+
// Use the padding to stash the plist_cpp pointer, so we can clean it up later.
127+
void *mem = ::godot::Memory::alloc_static(sizeof(GDExtensionPropertyInfo) * plist_size, true);
128+
GDExtensionPropertyInfo *plist = reinterpret_cast<GDExtensionPropertyInfo *>(mem);
129+
::godot::List<::godot::PropertyInfo> **plist_cpp_stash = reinterpret_cast<::godot::List<::godot::PropertyInfo> **>((uint8_t *)mem - ::godot::Memory::DATA_OFFSET + ::godot::Memory::ELEMENT_OFFSET);
130+
*plist_cpp_stash = plist_cpp;
131+
127132
unsigned int i = 0;
128-
for (const ::godot::PropertyInfo &E : plist_cpp) {
133+
for (const ::godot::PropertyInfo &E : *plist_cpp) {
129134
plist[i].type = static_cast<GDExtensionVariantType>(E.type);
130135
plist[i].name = E.name._native_ptr();
131136
plist[i].hint = E.hint;
@@ -138,7 +143,10 @@ GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::Pro
138143
}
139144

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

144152
void add_engine_class_registration_callback(EngineClassRegistrationCallback p_callback) {

0 commit comments

Comments
 (0)