Skip to content

Commit bc35c3b

Browse files
committed
Cherry-pick godotengine#1904: fix _get_property_list failing in thread scenarios
1 parent da26c17 commit bc35c3b

2 files changed

Lines changed: 17 additions & 15 deletions

File tree

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

114110
Wrapped(const StringName &p_godot_class);
@@ -155,7 +151,7 @@ _FORCE_INLINE_ Vector<StringName> snarray(P... p_args) {
155151

156152
namespace internal {
157153

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

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

src/classes/wrapped.cpp

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -105,16 +105,21 @@ std::vector<EngineClassRegistrationCallback> &get_engine_class_registration_call
105105
return engine_class_registration_callbacks;
106106
}
107107

108-
GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::PropertyInfo> &plist_cpp, uint32_t *r_size) {
109-
GDExtensionPropertyInfo *plist = nullptr;
108+
GDExtensionPropertyInfo *create_c_property_list(::godot::List<::godot::PropertyInfo> *plist_cpp, uint32_t *r_size) {
110109
// Linked list size can be expensive to get so we cache it
111-
const uint32_t plist_size = plist_cpp.size();
110+
const uint32_t plist_size = plist_cpp->size();
112111
if (r_size != nullptr) {
113112
*r_size = plist_size;
114113
}
115-
plist = reinterpret_cast<GDExtensionPropertyInfo *>(memalloc(sizeof(GDExtensionPropertyInfo) * plist_size));
114+
115+
// Use the padding to stash the plist_cpp pointer, so we can clean it up later.
116+
void *mem = ::godot::Memory::alloc_static(sizeof(GDExtensionPropertyInfo) * plist_size, true);
117+
GDExtensionPropertyInfo *plist = reinterpret_cast<GDExtensionPropertyInfo *>(mem);
118+
::godot::List<::godot::PropertyInfo> **plist_cpp_stash = reinterpret_cast<::godot::List<::godot::PropertyInfo> **>((uint8_t *)mem - ::godot::Memory::DATA_OFFSET + ::godot::Memory::ELEMENT_OFFSET);
119+
*plist_cpp_stash = plist_cpp;
120+
116121
unsigned int i = 0;
117-
for (const ::godot::PropertyInfo &E : plist_cpp) {
122+
for (const ::godot::PropertyInfo &E : *plist_cpp) {
118123
plist[i].type = static_cast<GDExtensionVariantType>(E.type);
119124
plist[i].name = E.name._native_ptr();
120125
plist[i].hint = E.hint;
@@ -127,7 +132,10 @@ GDExtensionPropertyInfo *create_c_property_list(const ::godot::List<::godot::Pro
127132
}
128133

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

133141
void add_engine_class_registration_callback(EngineClassRegistrationCallback p_callback) {

0 commit comments

Comments
 (0)