Merge pull request #35208 from neikeq/mono-lazy-thread-attach

Mono/C#: Script interface calls now attach the current thread

modules/mono/csharp_script.cpp

@@ -559,6 +559,7 @@ Vector<ScriptLanguage::StackInfo> CSharpLanguage::debug_get_current_stack_info()
 
 #ifdef DEBUG_ENABLED
 	_TLS_RECURSION_GUARD_V_(Vector<StackInfo>());
+	GD_MONO_SCOPE_THREAD_ATTACH;
 
 	if (!gdmono->is_runtime_initialized() || !GDMono::get_singleton()->get_core_api_assembly() || !GDMonoCache::cached_data.corlib_cache_updated)
 		return Vector<StackInfo>();
@@ -583,6 +584,7 @@ Vector<ScriptLanguage::StackInfo> CSharpLanguage::debug_get_current_stack_info()
 Vector<ScriptLanguage::StackInfo> CSharpLanguage::stack_trace_get_info(MonoObject *p_stack_trace) {
 
 	_TLS_RECURSION_GUARD_V_(Vector<StackInfo>());
+	GD_MONO_SCOPE_THREAD_ATTACH;
 
 	MonoException *exc = NULL;
 
@@ -689,6 +691,7 @@ void CSharpLanguage::reload_all_scripts() {
 
 #ifdef GD_MONO_HOT_RELOAD
 	if (is_assembly_reloading_needed()) {
+		GD_MONO_SCOPE_THREAD_ATTACH;
 		reload_assemblies(false);
 	}
 #endif
@@ -706,6 +709,7 @@ void CSharpLanguage::reload_tool_script(const Ref<Script> &p_script, bool p_soft
 
 #ifdef GD_MONO_HOT_RELOAD
 	if (is_assembly_reloading_needed()) {
+		GD_MONO_SCOPE_THREAD_ATTACH;
 		reload_assemblies(p_soft_reload);
 	}
 #endif
@@ -1356,6 +1360,8 @@ void CSharpLanguage::free_instance_binding_data(void *p_data) {
 	if (finalizing)
 		return; // inside CSharpLanguage::finish(), all the gchandle bindings are released there
 
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	{
 		SCOPED_MUTEX_LOCK(language_bind_mutex);
 
@@ -1382,6 +1388,7 @@ void CSharpLanguage::refcount_incremented_instance_binding(Object *p_object) {
 
 #ifdef DEBUG_ENABLED
 	CRASH_COND(!ref_owner);
+	CRASH_COND(!p_object->has_script_instance_binding(get_language_index()));
 #endif
 
 	void *data = p_object->get_script_instance_binding(get_language_index());
@@ -1394,6 +1401,8 @@ void CSharpLanguage::refcount_incremented_instance_binding(Object *p_object) {
 		return;
 
 	if (ref_owner->reference_get_count() > 1 && gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0
+		GD_MONO_SCOPE_THREAD_ATTACH;
+
 		// The reference count was increased after the managed side was the only one referencing our owner.
 		// This means the owner is being referenced again by the unmanaged side,
 		// so the owner must hold the managed side alive again to avoid it from being GCed.
@@ -1415,6 +1424,7 @@ bool CSharpLanguage::refcount_decremented_instance_binding(Object *p_object) {
 
 #ifdef DEBUG_ENABLED
 	CRASH_COND(!ref_owner);
+	CRASH_COND(!p_object->has_script_instance_binding(get_language_index()));
 #endif
 
 	void *data = p_object->get_script_instance_binding(get_language_index());
@@ -1429,6 +1439,8 @@ bool CSharpLanguage::refcount_decremented_instance_binding(Object *p_object) {
 		return refcount == 0;
 
 	if (refcount == 1 && gchandle.is_valid() && !gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0
+		GD_MONO_SCOPE_THREAD_ATTACH;
+
 		// If owner owner is no longer referenced by the unmanaged side,
 		// the managed instance takes responsibility of deleting the owner when GCed.
 
@@ -1480,6 +1492,8 @@ bool CSharpInstance::set(const StringName &p_name, const Variant &p_value) {
 
 	ERR_FAIL_COND_V(!script.is_valid(), false);
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	MonoObject *mono_object = get_mono_object();
 	ERR_FAIL_NULL_V(mono_object, false);
 
@@ -1532,6 +1546,8 @@ bool CSharpInstance::get(const StringName &p_name, Variant &r_ret) const {
 
 	ERR_FAIL_COND_V(!script.is_valid(), false);
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	MonoObject *mono_object = get_mono_object();
 	ERR_FAIL_NULL_V(mono_object, false);
 
@@ -1625,6 +1641,8 @@ void CSharpInstance::get_property_list(List<PropertyInfo> *p_properties) const {
 
 	ERR_FAIL_COND(!script.is_valid());
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	MonoObject *mono_object = get_mono_object();
 	ERR_FAIL_NULL(mono_object);
 
@@ -1669,6 +1687,8 @@ bool CSharpInstance::has_method(const StringName &p_method) const {
 	if (!script.is_valid())
 		return false;
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoClass *top = script->script_class;
 
 	while (top && top != script->native) {
@@ -1684,6 +1704,11 @@ bool CSharpInstance::has_method(const StringName &p_method) const {
 
 Variant CSharpInstance::call(const StringName &p_method, const Variant **p_args, int p_argcount, Variant::CallError &r_error) {
 
+	if (!script.is_valid())
+		ERR_FAIL_V(Variant());
+
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	MonoObject *mono_object = get_mono_object();
 
 	if (!mono_object) {
@@ -1691,9 +1716,6 @@ Variant CSharpInstance::call(const StringName &p_method, const Variant **p_args,
 		ERR_FAIL_V(Variant());
 	}
 
-	if (!script.is_valid())
-		ERR_FAIL_V(Variant());
-
 	GDMonoClass *top = script->script_class;
 
 	while (top && top != script->native) {
@@ -1721,6 +1743,8 @@ Variant CSharpInstance::call(const StringName &p_method, const Variant **p_args,
 
 void CSharpInstance::call_multilevel(const StringName &p_method, const Variant **p_args, int p_argcount) {
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	if (script.is_valid()) {
 		MonoObject *mono_object = get_mono_object();
 
@@ -1732,6 +1756,8 @@ void CSharpInstance::call_multilevel(const StringName &p_method, const Variant *
 
 void CSharpInstance::_call_multilevel(MonoObject *p_mono_object, const StringName &p_method, const Variant **p_args, int p_argcount) {
 
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	GDMonoClass *top = script->script_class;
 
 	while (top && top != script->native) {
@@ -1894,6 +1920,8 @@ void CSharpInstance::refcount_incremented() {
 	Reference *ref_owner = Object::cast_to<Reference>(owner);
 
 	if (ref_owner->reference_get_count() > 1 && gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0
+		GD_MONO_SCOPE_THREAD_ATTACH;
+
 		// The reference count was increased after the managed side was the only one referencing our owner.
 		// This means the owner is being referenced again by the unmanaged side,
 		// so the owner must hold the managed side alive again to avoid it from being GCed.
@@ -1917,6 +1945,8 @@ bool CSharpInstance::refcount_decremented() {
 	int refcount = ref_owner->reference_get_count();
 
 	if (refcount == 1 && !gchandle->is_weak()) { // The managed side also holds a reference, hence 1 instead of 0
+		GD_MONO_SCOPE_THREAD_ATTACH;
+
 		// If owner owner is no longer referenced by the unmanaged side,
 		// the managed instance takes responsibility of deleting the owner when GCed.
 
@@ -1957,6 +1987,8 @@ MultiplayerAPI::RPCMode CSharpInstance::_member_get_rpc_mode(IMonoClassMember *p
 
 MultiplayerAPI::RPCMode CSharpInstance::get_rpc_mode(const StringName &p_method) const {
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoClass *top = script->script_class;
 
 	while (top && top != script->native) {
@@ -1973,6 +2005,8 @@ MultiplayerAPI::RPCMode CSharpInstance::get_rpc_mode(const StringName &p_method)
 
 MultiplayerAPI::RPCMode CSharpInstance::get_rset_mode(const StringName &p_variable) const {
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoClass *top = script->script_class;
 
 	while (top && top != script->native) {
@@ -1994,6 +2028,8 @@ MultiplayerAPI::RPCMode CSharpInstance::get_rset_mode(const StringName &p_variab
 
 void CSharpInstance::notification(int p_notification) {
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	if (p_notification == Object::NOTIFICATION_PREDELETE) {
 		// When NOTIFICATION_PREDELETE is sent, we also take the chance to call Dispose().
 		// It's safe to call Dispose() multiple times and NOTIFICATION_PREDELETE is guaranteed
@@ -2031,6 +2067,8 @@ void CSharpInstance::notification(int p_notification) {
 
 void CSharpInstance::_call_notification(int p_notification) {
 
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	MonoObject *mono_object = get_mono_object();
 	ERR_FAIL_NULL(mono_object);
 
@@ -2055,6 +2093,8 @@ void CSharpInstance::_call_notification(int p_notification) {
 }
 
 String CSharpInstance::to_string(bool *r_valid) {
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	MonoObject *mono_object = get_mono_object();
 
 	if (mono_object == NULL) {
@@ -2103,6 +2143,8 @@ CSharpInstance::CSharpInstance() :
 
 CSharpInstance::~CSharpInstance() {
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	destructing_script_instance = true;
 
 	if (gchandle.is_valid()) {
@@ -2206,6 +2248,8 @@ void CSharpScript::_update_exports_values(Map<StringName, Variant> &values, List
 void CSharpScript::_update_member_info_no_exports() {
 
 	if (exports_invalidated) {
+		GD_MONO_ASSERT_THREAD_ATTACHED;
+
 		exports_invalidated = false;
 
 		member_info.clear();
@@ -2264,6 +2308,8 @@ bool CSharpScript::_update_exports() {
 	bool changed = false;
 
 	if (exports_invalidated) {
+		GD_MONO_SCOPE_THREAD_ATTACH;
+
 		exports_invalidated = false;
 
 		changed = true;
@@ -2413,6 +2459,8 @@ void CSharpScript::load_script_signals(GDMonoClass *p_class, GDMonoClass *p_nati
 	// make sure this classes signals are empty when loading for the first time
 	_signals.clear();
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoClass *top = p_class;
 	while (top && top != p_native_class) {
 		const Vector<GDMonoClass *> &delegates = top->get_all_delegates();
@@ -2433,6 +2481,8 @@ void CSharpScript::load_script_signals(GDMonoClass *p_class, GDMonoClass *p_nati
 }
 
 bool CSharpScript::_get_signal(GDMonoClass *p_class, GDMonoClass *p_delegate, Vector<Argument> &params) {
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	if (p_delegate->has_attribute(CACHED_CLASS(SignalAttribute))) {
 		MonoType *raw_type = p_delegate->get_mono_type();
 
@@ -2474,6 +2524,8 @@ bool CSharpScript::_get_signal(GDMonoClass *p_class, GDMonoClass *p_delegate, Ve
  */
 bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect_export, PropertyInfo &r_prop_info, bool &r_exported) {
 
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	// Goddammit, C++. All I wanted was some nested functions.
 #define MEMBER_FULL_QUALIFIED_NAME(m_member) \
 	(m_member->get_enclosing_class()->get_full_name() + "." + (String)m_member->get_name())
@@ -2552,6 +2604,8 @@ bool CSharpScript::_get_member_export(IMonoClassMember *p_member, bool p_inspect
 
 int CSharpScript::_try_get_member_export_hint(IMonoClassMember *p_member, ManagedType p_type, Variant::Type p_variant_type, bool p_allow_generics, PropertyHint &r_hint, String &r_hint_string) {
 
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	if (p_variant_type == Variant::INT && p_type.type_encoding == MONO_TYPE_VALUETYPE && mono_class_is_enum(p_type.type_class->get_mono_ptr())) {
 		r_hint = PROPERTY_HINT_ENUM;
 
@@ -2661,6 +2715,8 @@ Variant CSharpScript::call(const StringName &p_method, const Variant **p_args, i
 		return Variant();
 	}
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoClass *top = script_class;
 
 	while (top && top != native) {
@@ -2853,6 +2909,8 @@ StringName CSharpScript::get_instance_base_type() const {
 
 CSharpInstance *CSharpScript::_create_instance(const Variant **p_args, int p_argcount, Object *p_owner, bool p_isref, Variant::CallError &r_error) {
 
+	GD_MONO_ASSERT_THREAD_ATTACHED;
+
 	/* STEP 1, CREATE */
 
 	// Search the constructor first, to fail with an error if it's not found before allocating anything else.
@@ -2947,12 +3005,14 @@ Variant CSharpScript::_new(const Variant **p_args, int p_argcount, Variant::Call
 	}
 
 	r_error.error = Variant::CallError::CALL_OK;
-	REF ref;
 
 	ERR_FAIL_NULL_V(native, Variant());
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	Object *owner = ClassDB::instance(NATIVE_GDMONOCLASS_NAME(native));
 
+	REF ref;
 	Reference *r = Object::cast_to<Reference>(owner);
 	if (r) {
 		ref = REF(r);
@@ -2990,6 +3050,8 @@ ScriptInstance *CSharpScript::instance_create(Object *p_this) {
 		}
 	}
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	Variant::CallError unchecked_error;
 	return _create_instance(NULL, 0, p_this, Object::cast_to<Reference>(p_this) != NULL, unchecked_error);
 }
@@ -3037,6 +3099,8 @@ void CSharpScript::get_script_method_list(List<MethodInfo> *p_list) const {
 	if (!script_class)
 		return;
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	// TODO: Filter out things unsuitable for explicit calls, like constructors.
 	const Vector<GDMonoMethod *> &methods = script_class->get_all_methods();
 	for (int i = 0; i < methods.size(); ++i) {
@@ -3049,6 +3113,8 @@ bool CSharpScript::has_method(const StringName &p_method) const {
 	if (!script_class)
 		return false;
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	return script_class->has_fetched_method_unknown_params(p_method);
 }
 
@@ -3057,6 +3123,8 @@ MethodInfo CSharpScript::get_method_info(const StringName &p_method) const {
 	if (!script_class)
 		return MethodInfo();
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoClass *top = script_class;
 
 	while (top && top != native) {
@@ -3081,6 +3149,8 @@ Error CSharpScript::reload(bool p_keep_state) {
 
 	ERR_FAIL_COND_V(!p_keep_state && has_instances, ERR_ALREADY_IN_USE);
 
+	GD_MONO_SCOPE_THREAD_ATTACH;
+
 	GDMonoAssembly *project_assembly = GDMono::get_singleton()->get_project_assembly();
 
 	if (project_assembly) {
@@ -3308,39 +3378,7 @@ RES ResourceFormatLoaderCSharpScript::load(const String &p_path, const String &p
 
 	script->set_path(p_original_path);
 
-#ifndef TOOLS_ENABLED
-
-#ifdef DEBUG_ENABLED
-	// User is responsible for thread attach/detach
-	CRASH_COND_MSG(mono_domain_get() == NULL, "Thread is not attached.");
-#endif
-
-#endif
-
-#ifdef TOOLS_ENABLED
-	MonoDomain *domain = mono_domain_get();
-	if (Engine::get_singleton()->is_editor_hint() && domain == NULL) {
-
-		CRASH_COND(Thread::get_caller_id() == Thread::get_main_id());
-
-		// Thread is not attached, but we will make an exception in this case
-		// because this may be called by one of the editor's worker threads.
-		// Attach this thread temporarily to reload the script.
-
-		if (domain) {
-			MonoThread *mono_thread = mono_thread_attach(domain);
-			CRASH_COND(mono_thread == NULL);
-			script->reload();
-			mono_thread_detach(mono_thread);
-		}
-
-	} else { // just reload it normally
-#endif
-		script->reload();
-
-#ifdef TOOLS_ENABLED
-	}
-#endif
+	script->reload();
 
 	if (r_error)
 		*r_error = OK;

modules/mono/mono_gd/gd_mono_utils.cpp

@@ -125,10 +125,12 @@ void set_main_thread(MonoThread *p_thread) {
 	mono_thread_set_main(p_thread);
 }
 
-void attach_current_thread() {
-	ERR_FAIL_COND(!GDMono::get_singleton()->is_runtime_initialized());
-	MonoThread *mono_thread = mono_thread_attach(mono_get_root_domain());
-	ERR_FAIL_NULL(mono_thread);
+MonoThread *attach_current_thread() {
+	ERR_FAIL_COND_V(!GDMono::get_singleton()->is_runtime_initialized(), NULL);
+	MonoDomain *scripts_domain = GDMono::get_singleton()->get_scripts_domain();
+	MonoThread *mono_thread = mono_thread_attach(scripts_domain ? scripts_domain : mono_get_root_domain());
+	ERR_FAIL_NULL_V(mono_thread, NULL);
+	return mono_thread;
 }
 
 void detach_current_thread() {
@@ -138,10 +140,20 @@ void detach_current_thread() {
 	mono_thread_detach(mono_thread);
 }
 
+void detach_current_thread(MonoThread *p_mono_thread) {
+	ERR_FAIL_COND(!GDMono::get_singleton()->is_runtime_initialized());
+	ERR_FAIL_NULL(p_mono_thread);
+	mono_thread_detach(p_mono_thread);
+}
+
 MonoThread *get_current_thread() {
 	return mono_thread_current();
 }
 
+bool is_thread_attached() {
+	return mono_domain_get() != NULL;
+}
+
 void runtime_object_init(MonoObject *p_this_obj, GDMonoClass *p_class, MonoException **r_exc) {
 	GDMonoMethod *ctor = p_class->get_method(".ctor", 0);
 	ERR_FAIL_NULL(ctor);
@@ -617,4 +629,19 @@ GDMonoClass *make_generic_dictionary_type(MonoReflectionType *p_key_reftype, Mon
 
 } // namespace Marshal
 
+ScopeThreadAttach::ScopeThreadAttach() :
+		mono_thread(NULL) {
+	if (likely(GDMono::get_singleton()->is_runtime_initialized()) && unlikely(!mono_domain_get())) {
+		mono_thread = GDMonoUtils::attach_current_thread();
+	}
+}
+
+ScopeThreadAttach::~ScopeThreadAttach() {
+	if (unlikely(mono_thread)) {
+		GDMonoUtils::detach_current_thread(mono_thread);
+	}
+}
+
+// namespace Marshal
+
 } // namespace GDMonoUtils

modules/mono/mono_gd/gd_mono_utils.h

@@ -83,9 +83,11 @@ _FORCE_INLINE_ void hash_combine(uint32_t &p_hash, const uint32_t &p_with_hash)
 MonoObject *unmanaged_get_managed(Object *unmanaged);
 
 void set_main_thread(MonoThread *p_thread);
-void attach_current_thread();
+MonoThread *attach_current_thread();
 void detach_current_thread();
+void detach_current_thread(MonoThread *p_mono_thread);
 MonoThread *get_current_thread();
+bool is_thread_attached();
 
 _FORCE_INLINE_ bool is_main_thread() {
 	return mono_domain_get() != NULL && mono_thread_get_main() == mono_thread_current();
@@ -142,6 +144,14 @@ uint64_t unbox_enum_value(MonoObject *p_boxed, MonoType *p_enum_basetype, bool &
 
 void dispose(MonoObject *p_mono_object, MonoException **r_exc);
 
+struct ScopeThreadAttach {
+	ScopeThreadAttach();
+	~ScopeThreadAttach();
+
+private:
+	MonoThread *mono_thread;
+};
+
 } // namespace GDMonoUtils
 
 #define NATIVE_GDMONOCLASS_NAME(m_class) (GDMonoMarshal::mono_string_to_godot((MonoString *)m_class->get_field(BINDINGS_NATIVE_NAME_FIELD)->get_value(NULL)))
@@ -153,4 +163,15 @@ void dispose(MonoObject *p_mono_object, MonoException **r_exc);
 #define GD_MONO_END_RUNTIME_INVOKE \
 	_runtime_invoke_count_ref -= 1;
 
+#define GD_MONO_SCOPE_THREAD_ATTACH                                   \
+	GDMonoUtils::ScopeThreadAttach __gdmono__scope__thread__attach__; \
+	(void)__gdmono__scope__thread__attach__;
+
+#ifdef DEBUG_ENABLED
+#define GD_MONO_ASSERT_THREAD_ATTACHED \
+	{ CRASH_COND(!GDMonoUtils::is_thread_attached()); }
+#else
+#define GD_MONO_ASSERT_THREAD_ATTACHED
+#endif
+
 #endif // GD_MONOUTILS_H