From 4bbede9536bec2c6578ce372ec3206537615c6b4 Mon Sep 17 00:00:00 2001 From: reforia <11989463+reforia@users.noreply.github.com> Date: Sat, 30 May 2026 01:17:13 +0800 Subject: [PATCH] Guard `_create_instance_func` with `_constructing_mutex` on shared-global builds On macOS hot-reload builds (`_GODOT_CPP_AVOID_THREAD_LOCAL`) the `_constructing_*` state is a shared global guarded by `Wrapped::_constructing_mutex` rather than `thread_local`. `memnew` takes that lock via `_pre_initialize()` and releases it in `_postinitialize()`. Since #1568 the 4.4+ create path uses manual `_set_construct_info()` + placement-new instead of `memnew`, so it writes the shared globals without taking the lock while `_postinitialize()` still unlocks it: an unguarded write plus an unbalanced unlock on the `recursive_mutex`. The macOS shared-global workaround (#1594) landed between #1568's authoring and merge, so the create path silently lost the lock `memnew` had been providing. Under concurrent off-main-thread construction (e.g. threaded resource load) this races: `_constructing_class_binding_callbacks` can be clobbered before the `Wrapped` ctor consumes it, hitting `CRASH_NOW_MSG("...created without binding callbacks...")`, and the stray `unlock()` tears down a critical section held by another thread (downstream instance-binding / TypedArray corruption). It is intermittent (~50%), macOS-only, and only on threaded loads; release/export builds use real `thread_local` and are unaffected. Lock before `_set_construct_info()` on the 4.4+ path and release in the no-`p_notify_postinitialize` branch to keep the recursive mutex balanced, mirroring `_pre_initialize()`/`_postinitialize()`. Non-macOS and release builds compile out both blocks. `_recreate_instance_func` already uses a `lock_guard`. Fixes #1990. Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: reforia <11989463+reforia@users.noreply.github.com> --- include/godot_cpp/core/class_db.hpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/include/godot_cpp/core/class_db.hpp b/include/godot_cpp/core/class_db.hpp index c2fcb08cf..1d16d58db 100644 --- a/include/godot_cpp/core/class_db.hpp +++ b/include/godot_cpp/core/class_db.hpp @@ -117,13 +117,27 @@ class ClassDB { static GDExtensionObjectPtr _create_instance_func(void *data) { #endif // GODOT_VERSION_MINOR >= 4 if constexpr (!std::is_abstract_v) { - Wrapped::_set_construct_info(); #if GODOT_VERSION_MINOR >= 4 +#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL + // When the construction state is a shared global rather than + // thread_local (macOS + hot reload), it must be guarded for the + // whole construction, mirroring _pre_initialize() (the memnew path). + // _postinitialize() releases the lock; when post-init is deferred we + // release it here to keep the recursive_mutex balanced. + Wrapped::_constructing_mutex.lock(); +#endif + Wrapped::_set_construct_info(); T *new_object = new ("", "") T; if (p_notify_postinitialize) { new_object->_postinitialize(); } +#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL + else { + Wrapped::_constructing_mutex.unlock(); + } +#endif #else + Wrapped::_set_construct_info(); T *new_object = memnew(T); #endif // GODOT_VERSION_MINOR >= 4 return new_object->_owner;