diff --git a/src/mono/mono/metadata/appdomain.c b/src/mono/mono/metadata/appdomain.c index b602239e807700..73c8b7b1bbd34b 100644 --- a/src/mono/mono/metadata/appdomain.c +++ b/src/mono/mono/metadata/appdomain.c @@ -128,7 +128,7 @@ static gboolean mono_domain_asmctx_from_path (const char *fname, MonoAssembly *requesting_assembly, gpointer user_data, MonoAssemblyContextKind *out_asmctx); static void -add_assemblies_to_domain (MonoDomain *domain, MonoAssembly *ass, GHashTable *hash); +add_assemblies_to_domain (MonoDomain *domain, MonoAssembly *ass, GHashTable *ht); #if ENABLE_NETCORE @@ -1520,7 +1520,6 @@ mono_domain_assembly_postload_search (MonoAssemblyLoadContext *alc, MonoAssembly static void add_assemblies_to_domain (MonoDomain *domain, MonoAssembly *ass, GHashTable *ht) { - gint i; GSList *tmp; gboolean destroy_ht = FALSE; @@ -1533,37 +1532,40 @@ add_assemblies_to_domain (MonoDomain *domain, MonoAssembly *ass, GHashTable *ht) ht = g_hash_table_new (mono_aligned_addr_hash, NULL); destroy_ht = TRUE; for (tmp = domain->domain_assemblies; tmp; tmp = tmp->next) { - g_hash_table_insert (ht, tmp->data, tmp->data); + g_hash_table_add (ht, tmp->data); } } - /* FIXME: handle lazy loaded assemblies */ - if (!g_hash_table_lookup (ht, ass)) { mono_assembly_addref (ass); - g_hash_table_insert (ht, ass, ass); + g_hash_table_add (ht, ass); domain->domain_assemblies = g_slist_append (domain->domain_assemblies, ass); mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Assembly %s[%p] added to domain %s, ref_count=%d", ass->aname.name, ass, domain->friendly_name, ass->ref_count); } +#ifndef ENABLE_NETCORE if (ass->image->references) { - for (i = 0; i < ass->image->nreferences; i++) { - if (ass->image->references[i] && ass->image->references [i] != REFERENCE_MISSING) { - if (!g_hash_table_lookup (ht, ass->image->references [i])) { - add_assemblies_to_domain (domain, ass->image->references [i], ht); - } + for (int i = 0; i < ass->image->nreferences; i++) { + MonoAssembly *ref = ass->image->references [i]; + if (ref && ref != REFERENCE_MISSING) { + if (!g_hash_table_lookup (ht, ref)) + add_assemblies_to_domain (domain, ref, ht); } } } +#endif + if (destroy_ht) g_hash_table_destroy (ht); } +/* + * LOCKING: assumes the ALC's assemblies lock is taken + */ #ifdef ENABLE_NETCORE static void add_assembly_to_alc (MonoAssemblyLoadContext *alc, MonoAssembly *ass) { - gint i; GSList *tmp; g_assert (ass != NULL); @@ -1571,11 +1573,8 @@ add_assembly_to_alc (MonoAssemblyLoadContext *alc, MonoAssembly *ass) if (!ass->aname.name) return; - mono_alc_assemblies_lock (alc); - for (tmp = alc->loaded_assemblies; tmp; tmp = tmp->next) { if (tmp->data == ass) { - mono_alc_assemblies_unlock (alc); return; } } @@ -1585,14 +1584,6 @@ add_assembly_to_alc (MonoAssemblyLoadContext *alc, MonoAssembly *ass) alc->loaded_assemblies = g_slist_append (alc->loaded_assemblies, ass); mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Assembly %s[%p] added to ALC (%p), ref_count=%d", ass->aname.name, ass, (gpointer)alc, ass->ref_count); - if (ass->image->references) { - for (i = 0; i < ass->image->nreferences; i++) { - // TODO: remove all this after we're confident this assert isn't hit - g_assertf (!ass->image->references [i], "Did not expect reference %d of %s to be resolved", i, ass->image->name); - } - } - - mono_alc_assemblies_unlock (alc); } #endif @@ -1672,11 +1663,15 @@ mono_domain_fire_assembly_load (MonoAssemblyLoadContext *alc, MonoAssembly *asse mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Loading assembly %s (%p) into domain %s (%p) and ALC %p", assembly->aname.name, assembly, domain->friendly_name, domain, alc); mono_domain_assemblies_lock (domain); +#ifdef ENABLE_NETCORE + mono_alc_assemblies_lock (alc); +#endif add_assemblies_to_domain (domain, assembly, NULL); - mono_domain_assemblies_unlock (domain); #ifdef ENABLE_NETCORE add_assembly_to_alc (alc, assembly); + mono_alc_assemblies_unlock (alc); #endif + mono_domain_assemblies_unlock (domain); mono_domain_fire_assembly_load_event (domain, assembly, error_out); diff --git a/src/mono/mono/metadata/assembly.c b/src/mono/mono/metadata/assembly.c index e13eb5aa19bb80..9960747ab48a87 100644 --- a/src/mono/mono/metadata/assembly.c +++ b/src/mono/mono/metadata/assembly.c @@ -356,11 +356,21 @@ static MonoAssembly *corlib; static char* unquote (const char *str); -/* This protects loaded_assemblies and image->references */ -#define mono_assemblies_lock() mono_os_mutex_lock (&assemblies_mutex) -#define mono_assemblies_unlock() mono_os_mutex_unlock (&assemblies_mutex) +// This protects loaded_assemblies static mono_mutex_t assemblies_mutex; +static inline void +mono_assemblies_lock () +{ + mono_os_mutex_lock (&assemblies_mutex); +} + +static inline void +mono_assemblies_unlock () +{ + mono_os_mutex_unlock (&assemblies_mutex); +} + /* If defined, points to the bundled assembly information */ static const MonoBundledAssembly **bundles; @@ -1783,7 +1793,7 @@ mono_assembly_load_reference (MonoImage *image, int index) * image->references is shared between threads, so we need to access * it inside a critical section. */ - mono_assemblies_lock (); + mono_image_lock (image); if (!image->references) { MonoTableInfo *t = &image->tables [MONO_TABLE_ASSEMBLYREF]; @@ -1791,7 +1801,7 @@ mono_assembly_load_reference (MonoImage *image, int index) image->nreferences = t->rows; } reference = image->references [index]; - mono_assemblies_unlock (); + mono_image_unlock (image); if (reference) return; @@ -1874,7 +1884,7 @@ mono_assembly_load_reference (MonoImage *image, int index) } commit_reference: - mono_assemblies_lock (); + mono_image_lock (image); if (reference == NULL) { /* Flag as not found */ reference = (MonoAssembly *)REFERENCE_MISSING; @@ -1894,7 +1904,7 @@ mono_assembly_load_reference (MonoImage *image, int index) image->references [index] = reference; } - mono_assemblies_unlock (); + mono_image_unlock (image); if (image->references [index] != reference) { /* Somebody loaded it before us */ diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h index 55519c44331947..38033867d15c9a 100644 --- a/src/mono/mono/metadata/loader-internals.h +++ b/src/mono/mono/metadata/loader-internals.h @@ -33,6 +33,7 @@ struct _MonoAssemblyLoadContext { MonoDomain *domain; MonoLoadedImages *loaded_images; GSList *loaded_assemblies; + // If taking this with the domain assemblies_lock, always take this second MonoCoopMutex assemblies_lock; /* Handle of the corresponding managed object. If the ALC is * collectible, the handle is weak, otherwise it's strong. diff --git a/src/mono/mono/metadata/metadata-internals.h b/src/mono/mono/metadata/metadata-internals.h index 38a1fc84f0c2dd..181e9dec3ac95a 100644 --- a/src/mono/mono/metadata/metadata-internals.h +++ b/src/mono/mono/metadata/metadata-internals.h @@ -427,6 +427,8 @@ struct _MonoImage { * references is initialized only by using the mono_assembly_open * function, and not by using the lowlevel mono_image_open. * + * Protected by the image lock. + * * It is NULL terminated. */ MonoAssembly **references;