[mono] ALC function restructuring for unloadability (#41344)

We previously had a weird split between the domain and loader headers/files. This makes fewer things public, moves the useful ALC functions to loader-internals.h with the implementation in assembly-load-context.c, and cleans up the flow for some unloading behavior.

diff --git a/src/mono/mono/metadata/assembly-load-context.c b/src/mono/mono/metadata/assembly-load-context.c
index 105609d..ec46fbc 100644 (file)
--- a/src/mono/mono/metadata/assembly-load-context.c
+++ b/src/mono/mono/metadata/assembly-load-context.c
@@ -15,7 +15,7 @@
 
 GENERATE_GET_CLASS_WITH_CACHE (assembly_load_context, "System.Runtime.Loader", "AssemblyLoadContext");
 
-void
+static void
 mono_alc_init (MonoAssemblyLoadContext *alc, MonoDomain *domain, gboolean collectible)
 {
        MonoLoadedImages *li = g_new0 (MonoLoadedImages, 1);
@@ -30,36 +30,63 @@ mono_alc_init (MonoAssemblyLoadContext *alc, MonoDomain *domain, gboolean collec
        mono_coop_mutex_init (&alc->pinvoke_lock);
 }
 
+static MonoAssemblyLoadContext *
+mono_alc_create (MonoDomain *domain, gboolean is_default, gboolean collectible)
+{
+       MonoAssemblyLoadContext *alc = NULL;
+
+       mono_domain_alcs_lock (domain);
+       if (is_default && domain->default_alc)
+               goto leave;
+
+       alc = g_new0 (MonoAssemblyLoadContext, 1);
+       mono_alc_init (alc, domain, collectible);
+
+       domain->alcs = g_slist_prepend (domain->alcs, alc);
+       if (is_default)
+               domain->default_alc = alc;
+
+leave:
+       mono_domain_alcs_unlock (domain);
+       return alc;
+}
+
 void
-mono_alc_cleanup (MonoAssemblyLoadContext *alc)
+mono_alc_create_default (MonoDomain *domain)
 {
-       /*
-        * This is still very much WIP. It needs to be split up into various other functions and adjusted to work with the 
-        * managed LoaderAllocator design. For now, I've put it all in this function, but don't look at it too closely.
-        * 
-        * Of particular note: the minimum refcount on assemblies is 2: one for the domain and one for the ALC. 
-        * The domain refcount might be less than optimal on netcore, but its removal is too likely to cause issues for now.
-        */
-       GSList *tmp;
-       MonoDomain *domain = alc->domain;
+       if (domain->default_alc)
+               return;
+       mono_alc_create (domain, TRUE, FALSE);
+}
 
-       g_assert (alc != mono_domain_default_alc (domain));
-       g_assert (alc->collectible == TRUE);
+MonoAssemblyLoadContext *
+mono_alc_create_individual (MonoDomain *domain, MonoGCHandle this_gchandle, gboolean collectible, MonoError *error)
+{
+       MonoAssemblyLoadContext *alc = mono_alc_create (domain, FALSE, collectible);
 
-       // FIXME: alc unloading profiler event
+       alc->gchandle = this_gchandle;
+
+       return alc;
+}
+
+static void
+mono_alc_cleanup_assemblies (MonoAssemblyLoadContext *alc)
+{
+       // The minimum refcount on assemblies is 2: one for the domain and one for the ALC. 
+       // The domain refcount might be less than optimal on netcore, but its removal is too likely to cause issues for now.
+       GSList *tmp;
+       MonoDomain *domain = alc->domain;
 
        // Remove the assemblies from domain_assemblies
        mono_domain_assemblies_lock (domain);
        for (tmp = alc->loaded_assemblies; tmp; tmp = tmp->next) {
                MonoAssembly *assembly = (MonoAssembly *)tmp->data;
-               g_slist_remove (domain->domain_assemblies, assembly);
+               domain->domain_assemblies = g_slist_remove (domain->domain_assemblies, assembly);
                mono_assembly_decref (assembly);
                mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_ASSEMBLY, "Unloading ALC [%p], removing assembly %s[%p] from domain_assemblies, ref_count=%d\n", alc, assembly->aname.name, assembly, assembly->ref_count);
        }
        mono_domain_assemblies_unlock (domain);
 
-       // Some equivalent to mono_gc_clear_domain? I guess in our case we just have to assert that we have no lingering references?
-
        // Release the GC roots
        for (tmp = alc->loaded_assemblies; tmp; tmp = tmp->next) {
                MonoAssembly *assembly = (MonoAssembly *)tmp->data;
@@ -99,17 +126,48 @@ mono_alc_cleanup (MonoAssemblyLoadContext *alc)
        g_slist_free (alc->loaded_assemblies);
        alc->loaded_assemblies = NULL;
 
-       // FIXME: alc unloaded profiler event
-
-       g_hash_table_destroy (alc->pinvoke_scopes);
        mono_coop_mutex_destroy (&alc->assemblies_lock);
-       mono_coop_mutex_destroy (&alc->pinvoke_lock);
 
        mono_loaded_images_free (alc->loaded_images);
+       alc->loaded_images = NULL;
 
        // TODO: free mempool stuff/jit info tables, see domain freeing for an example
 }
 
+static void
+mono_alc_cleanup (MonoAssemblyLoadContext *alc)
+{
+       MonoDomain *domain = alc->domain;
+
+       g_assert (alc != mono_domain_default_alc (domain));
+       g_assert (alc->collectible == TRUE);
+
+       // TODO: alc unloading profiler event
+
+       // Remove from domain list
+       mono_domain_alcs_lock (domain);
+       domain->alcs = g_slist_remove (domain->alcs, alc);
+       mono_domain_alcs_unlock (domain);
+
+       mono_alc_cleanup_assemblies (alc);
+
+       mono_gchandle_free_internal (alc->gchandle);
+       alc->gchandle = NULL;
+
+       g_hash_table_destroy (alc->pinvoke_scopes);
+       alc->pinvoke_scopes = NULL;
+       mono_coop_mutex_destroy (&alc->pinvoke_lock);
+
+       // TODO: alc unloaded profiler event
+}
+
+static void
+mono_alc_free (MonoAssemblyLoadContext *alc)
+{
+       mono_alc_cleanup (alc);
+       g_free (alc);
+}
+
 void
 mono_alc_assemblies_lock (MonoAssemblyLoadContext *alc)
 {
@@ -136,10 +194,9 @@ ves_icall_System_Runtime_Loader_AssemblyLoadContext_InternalInitializeNativeALC
                g_assert (alc);
                if (!alc->gchandle)
                        alc->gchandle = this_gchandle;
-       } else {
-               /* create it */
-               alc = mono_domain_create_individual_alc (domain, this_gchandle, collectible, error);
-       }
+       } else
+               alc = mono_alc_create_individual (domain, this_gchandle, collectible, error);
+
        return alc;
 }
 
@@ -149,10 +206,13 @@ ves_icall_System_Runtime_Loader_AssemblyLoadContext_PrepareForAssemblyLoadContex
        MonoGCHandle strong_gchandle = (MonoGCHandle)strong_gchandle_ptr;
        MonoAssemblyLoadContext *alc = (MonoAssemblyLoadContext *)alc_pointer;
 
-       g_assert (alc->collectible == TRUE);
-       g_assert (alc->unloading == FALSE);
+       g_assert (alc->collectible);
+       g_assert (!alc->unloading);
+       g_assert (alc->gchandle);
+
        alc->unloading = TRUE;
 
+       // Replace the weak gchandle with the new strong one to keep the managed ALC alive
        MonoGCHandle weak_gchandle = alc->gchandle;
        alc->gchandle = strong_gchandle;
        mono_gchandle_free_internal (weak_gchandle);

diff --git a/src/mono/mono/metadata/domain-internals.h b/src/mono/mono/metadata/domain-internals.h
index 0985b67..3968a77 100644 (file)
--- a/src/mono/mono/metadata/domain-internals.h
+++ b/src/mono/mono/metadata/domain-internals.h
@@ -696,8 +696,17 @@ MonoAssemblyLoadContext *
 mono_domain_default_alc (MonoDomain *domain);
 
 #ifdef ENABLE_NETCORE
-MonoAssemblyLoadContext *
-mono_domain_create_individual_alc (MonoDomain *domain, MonoGCHandle this_gchandle, gboolean collectible, MonoError *error);
+static inline void
+mono_domain_alcs_lock (MonoDomain *domain)
+{
+       mono_coop_mutex_lock (&domain->alcs_lock);
+}
+
+static inline void
+mono_domain_alcs_unlock (MonoDomain *domain)
+{
+       mono_coop_mutex_unlock (&domain->alcs_lock);
+}
 #endif
 
 static inline

diff --git a/src/mono/mono/metadata/domain.c b/src/mono/mono/metadata/domain.c
index 4f4294a..36271f8 100644 (file)
--- a/src/mono/mono/metadata/domain.c
+++ b/src/mono/mono/metadata/domain.c
@@ -128,17 +128,6 @@ get_runtimes_from_exe (const char *exe_file, MonoImage **exe_image);
 static const MonoRuntimeInfo*
 get_runtime_by_version (const char *version);
 
-#ifdef ENABLE_NETCORE
-static void
-mono_domain_alcs_lock (MonoDomain *domain);
-
-static void
-mono_domain_alcs_unlock (MonoDomain *domain);
-
-static void
-mono_domain_create_default_alc (MonoDomain *domain);
-#endif
-
 static LockFreeMempool*
 lock_free_mempool_new (void)
 {
@@ -482,7 +471,7 @@ mono_domain_create (void)
        mono_debug_domain_create (domain);
 
 #ifdef ENABLE_NETCORE
-       mono_domain_create_default_alc (domain);
+       mono_alc_create_default (domain);
 #endif
 
        if (create_domain_hook)
@@ -2075,60 +2064,3 @@ mono_domain_default_alc (MonoDomain *domain)
        return domain->default_alc;
 #endif
 }
-
-#ifdef ENABLE_NETCORE
-static inline void
-mono_domain_alcs_lock (MonoDomain *domain)
-{
-       mono_coop_mutex_lock (&domain->alcs_lock);
-}
-
-static inline void
-mono_domain_alcs_unlock (MonoDomain *domain)
-{
-       mono_coop_mutex_unlock (&domain->alcs_lock);
-}
-
-static MonoAssemblyLoadContext *
-create_alc (MonoDomain *domain, gboolean is_default, gboolean collectible)
-{
-       MonoAssemblyLoadContext *alc = NULL;
-
-       mono_domain_alcs_lock (domain);
-       if (is_default && domain->default_alc)
-               goto leave;
-
-       alc = g_new0 (MonoAssemblyLoadContext, 1);
-       mono_alc_init (alc, domain, collectible);
-
-       domain->alcs = g_slist_prepend (domain->alcs, alc);
-       if (is_default)
-               domain->default_alc = alc;
-leave:
-       mono_domain_alcs_unlock (domain);
-       return alc;
-}
-
-void
-mono_domain_create_default_alc (MonoDomain *domain)
-{
-       if (domain->default_alc)
-               return;
-       create_alc (domain, TRUE, FALSE);
-}
-
-MonoAssemblyLoadContext *
-mono_domain_create_individual_alc (MonoDomain *domain, MonoGCHandle this_gchandle, gboolean collectible, MonoError *error)
-{
-       MonoAssemblyLoadContext *alc = create_alc (domain, FALSE, collectible);
-       alc->gchandle = this_gchandle;
-       return alc;
-}
-
-static void
-mono_alc_free (MonoAssemblyLoadContext *alc)
-{
-       mono_alc_cleanup (alc);
-       g_free (alc);
-}
-#endif

diff --git a/src/mono/mono/metadata/loader-internals.h b/src/mono/mono/metadata/loader-internals.h
index 4313454..6d5f493 100644 (file)
--- a/src/mono/mono/metadata/loader-internals.h
+++ b/src/mono/mono/metadata/loader-internals.h
@@ -90,10 +90,10 @@ void
 mono_set_pinvoke_search_directories (int dir_count, char **dirs);
 
 void
-mono_alc_init (MonoAssemblyLoadContext *alc, MonoDomain *domain, gboolean collectible);
+mono_alc_create_default (MonoDomain *domain);
 
-void
-mono_alc_cleanup (MonoAssemblyLoadContext *alc);
+MonoAssemblyLoadContext *
+mono_alc_create_individual (MonoDomain *domain, MonoGCHandle this_gchandle, gboolean collectible, MonoError *error);
 
 void
 mono_alc_assemblies_lock (MonoAssemblyLoadContext *alc);
@@ -115,7 +115,6 @@ mono_alc_invoke_resolve_using_resolve_satellite_nofail (MonoAssemblyLoadContext
 
 MonoAssemblyLoadContext *
 mono_alc_from_gchandle (MonoGCHandle alc_gchandle);
-
 #endif /* ENABLE_NETCORE */
 
 static inline MonoDomain *