Merge pull request #1992 from dsnopek/better-macos-thread-local-hack-maybe

Use a different method to avoid `thread_local` on MacOS

Author: dsnopek

include/godot_cpp/classes/wrapped.hpp

@@ -40,11 +40,9 @@
 #include <godot_cpp/godot.hpp>
 
 #if defined(MACOS_ENABLED) && defined(HOT_RELOAD_ENABLED)
+#include <map>
 #include <mutex>
-#define _GODOT_CPP_AVOID_THREAD_LOCAL
-#define _GODOT_CPP_THREAD_LOCAL
-#else
-#define _GODOT_CPP_THREAD_LOCAL thread_local
+#include <thread>
 #endif
 
 namespace godot {
@@ -65,21 +63,45 @@ class Wrapped {
 	template <typename T, std::enable_if_t<std::is_base_of<::godot::Wrapped, T>::value, bool>>
 	friend _ALWAYS_INLINE_ void _pre_initialize();
 
-#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL
-	static std::recursive_mutex _constructing_mutex;
+	struct ConstructInfo {
+		const StringName *extension_class_name = nullptr;
+		const GDExtensionInstanceBindingCallbacks *class_binding_callbacks = nullptr;
+#ifdef HOT_RELOAD_ENABLED
+		GDExtensionObjectPtr recreate_owner = nullptr;
 #endif
+	};
 
-	_GODOT_CPP_THREAD_LOCAL static const StringName *_constructing_extension_class_name;
-	_GODOT_CPP_THREAD_LOCAL static const GDExtensionInstanceBindingCallbacks *_constructing_class_binding_callbacks;
-
-#ifdef HOT_RELOAD_ENABLED
-	_GODOT_CPP_THREAD_LOCAL static GDExtensionObjectPtr _constructing_recreate_owner;
+#if defined(MACOS_ENABLED) && defined(HOT_RELOAD_ENABLED)
+	// On macOS, `thread_local` storage keeps the library from being unloaded,
+	// which breaks hot-reload. Instead we keep each thread's `ConstructInfo` in a
+	// map keyed by thread id.
+	class ConstructInfoStore {
+		std::mutex mutex;
+		std::map<std::thread::id, ConstructInfo> infos;
+
+	public:
+		ConstructInfo &get() {
+			std::lock_guard<std::mutex> lock(mutex);
+			return infos[std::this_thread::get_id()];
+		}
+	};
+
+	static ConstructInfo &_get_construct_info() {
+		static ConstructInfoStore store;
+		return store.get();
+	}
+#else
+	static ConstructInfo &_get_construct_info() {
+		static thread_local ConstructInfo info;
+		return info;
+	}
 #endif
 
 	template <typename T>
 	_ALWAYS_INLINE_ static void _set_construct_info() {
-		_constructing_extension_class_name = T::_get_extension_class_name();
-		_constructing_class_binding_callbacks = &T::_gde_binding_callbacks;
+		ConstructInfo &info = _get_construct_info();
+		info.extension_class_name = T::_get_extension_class_name();
+		info.class_binding_callbacks = &T::_gde_binding_callbacks;
 	}
 
 protected:
@@ -124,9 +146,6 @@ class Wrapped {
 
 template <typename T, std::enable_if_t<std::is_base_of<::godot::Wrapped, T>::value, bool>>
 _ALWAYS_INLINE_ void _pre_initialize() {
-#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL
-	Wrapped::_constructing_mutex.lock();
-#endif
 	Wrapped::_set_construct_info<T>();
 }
 

include/godot_cpp/core/class_db.hpp

@@ -136,10 +136,7 @@ class ClassDB {
 	static GDExtensionClassInstancePtr _recreate_instance_func(void *data, GDExtensionObjectPtr obj) {
 		if constexpr (!std::is_abstract_v<T>) {
 #ifdef HOT_RELOAD_ENABLED
-#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL
-			std::lock_guard<std::recursive_mutex> lk(Wrapped::_constructing_mutex);
-#endif
-			Wrapped::_constructing_recreate_owner = obj;
+			Wrapped::_get_construct_info().recreate_owner = obj;
 			T *new_instance = (T *)memalloc(sizeof(T));
 			memnew_placement(new_instance, T);
 			return new_instance;

src/classes/wrapped.cpp

@@ -40,26 +40,11 @@
 
 namespace godot {
 
-#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL
-std::recursive_mutex Wrapped::_constructing_mutex;
-#endif
-
-_GODOT_CPP_THREAD_LOCAL const StringName *Wrapped::_constructing_extension_class_name = nullptr;
-_GODOT_CPP_THREAD_LOCAL const GDExtensionInstanceBindingCallbacks *Wrapped::_constructing_class_binding_callbacks = nullptr;
-
-#ifdef HOT_RELOAD_ENABLED
-_GODOT_CPP_THREAD_LOCAL GDExtensionObjectPtr Wrapped::_constructing_recreate_owner = nullptr;
-#endif
-
 const StringName *Wrapped::_get_extension_class_name() {
 	return nullptr;
 }
 
 void Wrapped::_postinitialize() {
-#ifdef _GODOT_CPP_AVOID_THREAD_LOCAL
-	Wrapped::_constructing_mutex.unlock();
-#endif
-
 #if GODOT_VERSION_MINOR >= 4
 	Object *obj = dynamic_cast<Object *>(this);
 	if (obj) {
@@ -74,10 +59,12 @@ void Wrapped::_postinitialize() {
 }
 
 Wrapped::Wrapped(const StringName &p_godot_class) {
+	ConstructInfo &info = _get_construct_info();
+
 #ifdef HOT_RELOAD_ENABLED
-	if (unlikely(Wrapped::_constructing_recreate_owner)) {
-		_owner = Wrapped::_constructing_recreate_owner;
-		Wrapped::_constructing_recreate_owner = nullptr;
+	if (unlikely(info.recreate_owner)) {
+		_owner = info.recreate_owner;
+		info.recreate_owner = nullptr;
 	} else
 #endif
 	{
@@ -90,14 +77,14 @@ Wrapped::Wrapped(const StringName &p_godot_class) {
 #endif
 	}
 
-	if (_constructing_extension_class_name) {
-		::godot::gdextension_interface::object_set_instance(_owner, reinterpret_cast<GDExtensionConstStringNamePtr>(_constructing_extension_class_name), this);
-		_constructing_extension_class_name = nullptr;
+	if (info.extension_class_name) {
+		::godot::gdextension_interface::object_set_instance(_owner, reinterpret_cast<GDExtensionConstStringNamePtr>(info.extension_class_name), this);
+		info.extension_class_name = nullptr;
 	}
 
-	if (likely(_constructing_class_binding_callbacks)) {
-		::godot::gdextension_interface::object_set_instance_binding(_owner, ::godot::gdextension_interface::token, this, _constructing_class_binding_callbacks);
-		_constructing_class_binding_callbacks = nullptr;
+	if (likely(info.class_binding_callbacks)) {
+		::godot::gdextension_interface::object_set_instance_binding(_owner, ::godot::gdextension_interface::token, this, info.class_binding_callbacks);
+		info.class_binding_callbacks = nullptr;
 	} else {
 		CRASH_NOW_MSG("BUG: Godot Object created without binding callbacks. Did you forget to use memnew()?");
 	}