Skip to content

Commit 9be6287

Browse files
authored
[mono][gc] Remove reliance on regs/stack scanning in some cases (#101478)
* [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. * [mono][metadata] Allocate handle while operating with newly allocated RuntimeType * Enable test suite
1 parent 9926d60 commit 9be6287

File tree

3 files changed

+58
-28
lines changed

3 files changed

+58
-28
lines changed

src/libraries/tests.proj

-2
Original file line numberDiff line numberDiff line change
@@ -395,8 +395,6 @@
395395
<!-- Issue: https://github.com/dotnet/runtime/issues/95795 -->
396396
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.Globalization.Tests\Hybrid\System.Globalization.Hybrid.WASM.Tests.csproj" />
397397
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.Globalization.Calendars.Tests\Hybrid\System.Globalization.Calendars.Hybrid.WASM.Tests.csproj" />
398-
<!-- Issue: https://github.com/dotnet/runtime/issues/100932 -->
399-
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Runtime\tests\System.Reflection.Tests\System.Reflection.Tests.csproj" />
400398
</ItemGroup>
401399

402400
<!-- Aggressive Trimming related failures -->

src/mono/mono/metadata/reflection.c

+43-25
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,48 @@ mono_type_get_object (MonoDomain *domain, MonoType *type)
440440
return ret;
441441
}
442442

443+
/* LOCKING: assumes the loader lock is taken */
444+
static MonoReflectionType*
445+
mono_type_get_object_checked_alloc_helper (MonoType *type, MonoMemoryManager *memory_manager, MonoDomain *domain, MonoError *error)
446+
{
447+
HANDLE_FUNCTION_ENTER ();
448+
449+
MonoReflectionType *res, *cached;
450+
/* This is stored in vtables/JITted code so it has to be pinned */
451+
MonoReflectionTypeHandle res_handle = MONO_HANDLE_CAST (MonoReflectionType, mono_object_new_pinned_handle (mono_defaults.runtimetype_class, error));
452+
goto_if_nok (error, exit);
453+
454+
res = MONO_HANDLE_RAW (res_handle);
455+
456+
res->type = type;
457+
if (memory_manager->collectible) {
458+
MonoObject *loader_alloc = mono_gchandle_get_target_internal (mono_mem_manager_get_loader_alloc (memory_manager));
459+
g_assert (loader_alloc);
460+
MONO_OBJECT_SETREF_INTERNAL (res, m_keepalive, loader_alloc);
461+
}
462+
463+
mono_mem_manager_lock (memory_manager);
464+
if (memory_manager->collectible)
465+
cached = (MonoReflectionType *)mono_weak_hash_table_lookup (memory_manager->weak_type_hash, type);
466+
else
467+
cached = (MonoReflectionType *)mono_g_hash_table_lookup (memory_manager->type_hash, type);
468+
if (cached) {
469+
res = cached;
470+
MONO_HANDLE_ASSIGN_RAW (res_handle, res);
471+
} else {
472+
if (memory_manager->collectible)
473+
mono_weak_hash_table_insert (memory_manager->weak_type_hash, type, res);
474+
else
475+
mono_g_hash_table_insert_internal (memory_manager->type_hash, type, res);
476+
if (type->type == MONO_TYPE_VOID && !m_type_is_byref (type))
477+
domain->typeof_void = (MonoObject*)res;
478+
}
479+
mono_mem_manager_unlock (memory_manager);
480+
481+
exit:
482+
HANDLE_FUNCTION_RETURN_OBJ (res_handle);
483+
}
484+
443485
MonoReflectionType*
444486
mono_type_get_object_checked (MonoType *type, MonoError *error)
445487
{
@@ -554,33 +596,9 @@ mono_type_get_object_checked (MonoType *type, MonoError *error)
554596
res = &mono_class_get_ref_info_raw (klass)->type; /* FIXME use handles */
555597
goto leave;
556598
}
557-
/* This is stored in vtables/JITted code so it has to be pinned */
558-
res = (MonoReflectionType *)mono_object_new_pinned (mono_defaults.runtimetype_class, error);
559-
goto_if_nok (error, leave);
560599

561-
res->type = type;
562-
if (memory_manager->collectible) {
563-
MonoObject *loader_alloc = mono_gchandle_get_target_internal (mono_mem_manager_get_loader_alloc (memory_manager));
564-
g_assert (loader_alloc);
565-
MONO_OBJECT_SETREF_INTERNAL (res, m_keepalive, loader_alloc);
566-
}
567600

568-
mono_mem_manager_lock (memory_manager);
569-
if (memory_manager->collectible)
570-
cached = (MonoReflectionType *)mono_weak_hash_table_lookup (memory_manager->weak_type_hash, type);
571-
else
572-
cached = (MonoReflectionType *)mono_g_hash_table_lookup (memory_manager->type_hash, type);
573-
if (cached) {
574-
res = cached;
575-
} else {
576-
if (memory_manager->collectible)
577-
mono_weak_hash_table_insert (memory_manager->weak_type_hash, type, res);
578-
else
579-
mono_g_hash_table_insert_internal (memory_manager->type_hash, type, res);
580-
if (type->type == MONO_TYPE_VOID && !m_type_is_byref (type))
581-
domain->typeof_void = (MonoObject*)res;
582-
}
583-
mono_mem_manager_unlock (memory_manager);
601+
res = mono_type_get_object_checked_alloc_helper (type, memory_manager, domain, error);
584602

585603
leave:
586604
mono_loader_unlock ();

src/mono/mono/sgen/sgen-gc.c

+15-1
Original file line numberDiff line numberDiff line change
@@ -2848,6 +2848,9 @@ sgen_gc_invoke_finalizers (void)
28482848

28492849
g_assert (!pending_unqueued_finalizer);
28502850

2851+
gboolean gchandle_allocated = FALSE;
2852+
guint32 gchandle = 0;
2853+
28512854
/* FIXME: batch to reduce lock contention */
28522855
while (sgen_have_pending_finalizers ()) {
28532856
GCObject *obj;
@@ -2878,8 +2881,16 @@ sgen_gc_invoke_finalizers (void)
28782881
if (!obj)
28792882
break;
28802883

2884+
// We explicitly pin the object via a gchandle so we don't rely on the ref being
2885+
// present on stack/regs which is not scannable on WASM.
2886+
if (!gchandle_allocated) {
2887+
gchandle = sgen_gchandle_new (obj, TRUE);
2888+
gchandle_allocated = TRUE;
2889+
} else {
2890+
sgen_gchandle_set_target (gchandle, obj);
2891+
}
2892+
28812893
count++;
2882-
/* the object is on the stack so it is pinned */
28832894
/*g_print ("Calling finalizer for object: %p (%s)\n", obj, sgen_client_object_safe_name (obj));*/
28842895
sgen_client_run_finalize (obj);
28852896
}
@@ -2889,6 +2900,9 @@ sgen_gc_invoke_finalizers (void)
28892900
pending_unqueued_finalizer = FALSE;
28902901
}
28912902

2903+
if (gchandle_allocated)
2904+
sgen_gchandle_free (gchandle);
2905+
28922906
return count;
28932907
}
28942908

0 commit comments

Comments
 (0)