From 06da2ff39e65fce554bace8f5f596e957742741d Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 28 Aug 2023 08:53:01 +0300 Subject: [PATCH 1/2] [mono][metadata] Replace use of mem manager lock with loader lock Hash table operations under the mem manager lock could end up taking the loader lock when performing type comparison, in the case where custom modifiers needed to be loaded. Use the loader lock instead to prevent deadlocks. --- src/mono/mono/metadata/metadata.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index 7c7cd4433eecf6..c7ba4ef98fd43b 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -3434,7 +3434,8 @@ mono_metadata_get_canonical_generic_inst (MonoGenericInst *candidate) MonoMemoryManager *mm = mono_mem_manager_get_generic (data.images, data.nimages); collect_data_free (&data); - mono_mem_manager_lock (mm); + // Hashtable key equal func can take loader lock + mono_loader_lock (); if (!mm->ginst_cache) mm->ginst_cache = g_hash_table_new_full (mono_metadata_generic_inst_hash, mono_metadata_generic_inst_equal, NULL, (GDestroyNotify)free_generic_inst); @@ -3456,7 +3457,7 @@ mono_metadata_get_canonical_generic_inst (MonoGenericInst *candidate) g_hash_table_insert (mm->ginst_cache, ginst, ginst); } - mono_mem_manager_unlock (mm); + mono_loader_unlock (); return ginst; } @@ -3467,7 +3468,8 @@ mono_metadata_get_canonical_aggregate_modifiers (MonoAggregateModContainer *cand g_assert (candidate->count > 0); MonoMemoryManager *mm = mono_metadata_get_mem_manager_for_aggregate_modifiers (candidate); - mono_mem_manager_lock (mm); + // Hashtable key equal func can take loader lock + mono_loader_lock (); if (!mm->aggregate_modifiers_cache) mm->aggregate_modifiers_cache = g_hash_table_new_full (aggregate_modifiers_hash, aggregate_modifiers_equal, NULL, (GDestroyNotify)free_aggregate_modifiers); @@ -3484,7 +3486,7 @@ mono_metadata_get_canonical_aggregate_modifiers (MonoAggregateModContainer *cand g_hash_table_insert (mm->aggregate_modifiers_cache, amods, amods); } - mono_mem_manager_unlock (mm); + mono_loader_unlock (); return amods; } From e96fe14b910980d6e6f75c5464a16b7153a5f854 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Mon, 28 Aug 2023 23:01:55 +0300 Subject: [PATCH 2/2] [mono][metadata] Use loader lock during generic class hash table lookup --- src/mono/mono/metadata/metadata.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index c7ba4ef98fd43b..f9abe652b019a3 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -3545,7 +3545,8 @@ mono_metadata_lookup_generic_class (MonoClass *container_class, MonoGenericInst if (gclass) return gclass; - mono_mem_manager_lock (mm); + // Hashtable key equal func can take loader lock + mono_loader_lock (); gclass = mono_mem_manager_alloc0 (mm, sizeof (MonoGenericClass)); if (is_dynamic) @@ -3565,7 +3566,7 @@ mono_metadata_lookup_generic_class (MonoClass *container_class, MonoGenericInst // g_hash_table_insert (set->gclass_cache, gclass, gclass); - mono_mem_manager_unlock (mm); + mono_loader_unlock (); return gclass2; }