From 785af4bce86d10c25bcf67b455457c121eb42f36 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 18 Apr 2024 16:44:54 +0300 Subject: [PATCH 1/3] [mono][sgen] Allocated gchandle for this object when invoking finalizers We were assuming the object is kept alive from the stack/regs, which is not reliable on wasm. --- src/mono/mono/sgen/sgen-gc.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/mono/mono/sgen/sgen-gc.c b/src/mono/mono/sgen/sgen-gc.c index 07cf39ffd9dfc..705b8168ed36f 100644 --- a/src/mono/mono/sgen/sgen-gc.c +++ b/src/mono/mono/sgen/sgen-gc.c @@ -2848,6 +2848,9 @@ sgen_gc_invoke_finalizers (void) g_assert (!pending_unqueued_finalizer); + gboolean gchandle_allocated = FALSE; + guint32 gchandle = 0; + /* FIXME: batch to reduce lock contention */ while (sgen_have_pending_finalizers ()) { GCObject *obj; @@ -2878,8 +2881,16 @@ sgen_gc_invoke_finalizers (void) if (!obj) break; + // We explicitly pin the object via a gchandle so we don't rely on the ref being + // present on stack/regs which is not scannable on WASM. + if (!gchandle_allocated) { + gchandle = sgen_gchandle_new (obj, TRUE); + gchandle_allocated = TRUE; + } else { + sgen_gchandle_set_target (gchandle, obj); + } + count++; - /* the object is on the stack so it is pinned */ /*g_print ("Calling finalizer for object: %p (%s)\n", obj, sgen_client_object_safe_name (obj));*/ sgen_client_run_finalize (obj); } @@ -2889,6 +2900,9 @@ sgen_gc_invoke_finalizers (void) pending_unqueued_finalizer = FALSE; } + if (gchandle_allocated) + sgen_gchandle_free (gchandle); + return count; } From 545d784abc679aa93811514a209a7a604a01bb32 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Wed, 24 Apr 2024 11:15:20 +0300 Subject: [PATCH 2/3] [mono][metadata] Allocate handle while operating with newly allocated RuntimeType --- src/mono/mono/metadata/reflection.c | 68 ++++++++++++++++++----------- 1 file changed, 43 insertions(+), 25 deletions(-) diff --git a/src/mono/mono/metadata/reflection.c b/src/mono/mono/metadata/reflection.c index 4e450e027ac8c..1539b020a1b6b 100644 --- a/src/mono/mono/metadata/reflection.c +++ b/src/mono/mono/metadata/reflection.c @@ -440,6 +440,48 @@ mono_type_get_object (MonoDomain *domain, MonoType *type) return ret; } +/* LOCKING: assumes the loader lock is taken */ +static MonoReflectionType* +mono_type_get_object_checked_alloc_helper (MonoType *type, MonoMemoryManager *memory_manager, MonoDomain *domain, MonoError *error) +{ + HANDLE_FUNCTION_ENTER (); + + MonoReflectionType *res, *cached; + /* This is stored in vtables/JITted code so it has to be pinned */ + MonoReflectionTypeHandle res_handle = MONO_HANDLE_CAST (MonoReflectionType, mono_object_new_pinned_handle (mono_defaults.runtimetype_class, error)); + goto_if_nok (error, exit); + + res = MONO_HANDLE_RAW (res_handle); + + res->type = type; + if (memory_manager->collectible) { + MonoObject *loader_alloc = mono_gchandle_get_target_internal (mono_mem_manager_get_loader_alloc (memory_manager)); + g_assert (loader_alloc); + MONO_OBJECT_SETREF_INTERNAL (res, m_keepalive, loader_alloc); + } + + mono_mem_manager_lock (memory_manager); + if (memory_manager->collectible) + cached = (MonoReflectionType *)mono_weak_hash_table_lookup (memory_manager->weak_type_hash, type); + else + cached = (MonoReflectionType *)mono_g_hash_table_lookup (memory_manager->type_hash, type); + if (cached) { + res = cached; + MONO_HANDLE_ASSIGN_RAW (res_handle, res); + } else { + if (memory_manager->collectible) + mono_weak_hash_table_insert (memory_manager->weak_type_hash, type, res); + else + mono_g_hash_table_insert_internal (memory_manager->type_hash, type, res); + if (type->type == MONO_TYPE_VOID && !m_type_is_byref (type)) + domain->typeof_void = (MonoObject*)res; + } + mono_mem_manager_unlock (memory_manager); + +exit: + HANDLE_FUNCTION_RETURN_OBJ (res_handle); +} + MonoReflectionType* mono_type_get_object_checked (MonoType *type, MonoError *error) { @@ -554,33 +596,9 @@ mono_type_get_object_checked (MonoType *type, MonoError *error) res = &mono_class_get_ref_info_raw (klass)->type; /* FIXME use handles */ goto leave; } - /* This is stored in vtables/JITted code so it has to be pinned */ - res = (MonoReflectionType *)mono_object_new_pinned (mono_defaults.runtimetype_class, error); - goto_if_nok (error, leave); - res->type = type; - if (memory_manager->collectible) { - MonoObject *loader_alloc = mono_gchandle_get_target_internal (mono_mem_manager_get_loader_alloc (memory_manager)); - g_assert (loader_alloc); - MONO_OBJECT_SETREF_INTERNAL (res, m_keepalive, loader_alloc); - } - mono_mem_manager_lock (memory_manager); - if (memory_manager->collectible) - cached = (MonoReflectionType *)mono_weak_hash_table_lookup (memory_manager->weak_type_hash, type); - else - cached = (MonoReflectionType *)mono_g_hash_table_lookup (memory_manager->type_hash, type); - if (cached) { - res = cached; - } else { - if (memory_manager->collectible) - mono_weak_hash_table_insert (memory_manager->weak_type_hash, type, res); - else - mono_g_hash_table_insert_internal (memory_manager->type_hash, type, res); - if (type->type == MONO_TYPE_VOID && !m_type_is_byref (type)) - domain->typeof_void = (MonoObject*)res; - } - mono_mem_manager_unlock (memory_manager); + res = mono_type_get_object_checked_alloc_helper (type, memory_manager, domain, error); leave: mono_loader_unlock (); From 8d208b8b3be8a58af6849eece1de9a92ad309a34 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Wed, 24 Apr 2024 11:20:00 +0300 Subject: [PATCH 3/3] Enable test suite --- src/libraries/tests.proj | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libraries/tests.proj b/src/libraries/tests.proj index 7106a44c89e96..375a08cb76153 100644 --- a/src/libraries/tests.proj +++ b/src/libraries/tests.proj @@ -395,8 +395,6 @@ - -