From 370cd0f65ccba610325d560c91297e506bfe3944 Mon Sep 17 00:00:00 2001 From: Mark Probst Date: Wed, 30 Sep 2009 16:43:14 +0000 Subject: [PATCH] 2009-09-30 Mark Probst * assembly.c, domain.c, image.c, metadata-internals.h: Split domain and image unloading into two steps. We must do this because clearing the domain in SGen requires the class metadata to be intact, but we need to free the mono_g_hash_tables in case a collection occurs during domain unloading and the roots would trip up the GC. svn path=/trunk/mono/; revision=143047 --- mono/metadata/ChangeLog | 9 +++ mono/metadata/assembly.c | 53 +++++++++++---- mono/metadata/domain.c | 26 +++++--- mono/metadata/image.c | 102 +++++++++++++++++++++-------- mono/metadata/metadata-internals.h | 10 +++ 5 files changed, 150 insertions(+), 50 deletions(-) diff --git a/mono/metadata/ChangeLog b/mono/metadata/ChangeLog index d43ec70096c..d18e37f2df7 100644 --- a/mono/metadata/ChangeLog +++ b/mono/metadata/ChangeLog @@ -1,3 +1,12 @@ +2009-09-30 Mark Probst + + * assembly.c, domain.c, image.c, metadata-internals.h: Split + domain and image unloading into two steps. We must do this + because clearing the domain in SGen requires the class metadata to + be intact, but we need to free the mono_g_hash_tables in case a + collection occurs during domain unloading and the roots would trip + up the GC. + 2009-09-30 Mark Probst * object-internals.h: Remove serialized culture fields from diff --git a/mono/metadata/assembly.c b/mono/metadata/assembly.c index 538767850fc..d02dd17b2eb 100644 --- a/mono/metadata/assembly.c +++ b/mono/metadata/assembly.c @@ -2453,25 +2453,23 @@ mono_assembly_loaded (MonoAssemblyName *aname) return mono_assembly_loaded_full (aname, FALSE); } -/** - * mono_assembly_close: - * @assembly: the assembly to release. - * - * This method releases a reference to the @assembly. The assembly is - * only released when all the outstanding references to it are released. +/* + * Returns whether mono_assembly_close_finish() must be called as + * well. See comment for mono_image_close_except_pools() for why we + * unload in two steps. */ -void -mono_assembly_close (MonoAssembly *assembly) +gboolean +mono_assembly_close_except_image_pools (MonoAssembly *assembly) { GSList *tmp; - g_return_if_fail (assembly != NULL); + g_return_val_if_fail (assembly != NULL, FALSE); if (assembly == REFERENCE_MISSING) - return; - + return FALSE; + /* Might be 0 already */ if (InterlockedDecrement (&assembly->ref_count) > 0) - return; + return FALSE; mono_profiler_assembly_event (assembly, MONO_PROFILE_START_UNLOAD); @@ -2485,7 +2483,8 @@ mono_assembly_close (MonoAssembly *assembly) assembly->image->assembly = NULL; - mono_image_close (assembly->image); + if (!mono_image_close_except_pools (assembly->image)) + assembly->image = NULL; for (tmp = assembly->friend_assembly_names; tmp; tmp = tmp->next) { MonoAssemblyName *fname = tmp->data; @@ -2494,13 +2493,39 @@ mono_assembly_close (MonoAssembly *assembly) } g_slist_free (assembly->friend_assembly_names); g_free (assembly->basedir); + + mono_profiler_assembly_event (assembly, MONO_PROFILE_END_UNLOAD); + + return TRUE; +} + +void +mono_assembly_close_finish (MonoAssembly *assembly) +{ + g_assert (assembly && assembly != REFERENCE_MISSING); + + if (assembly->image) + mono_image_close_finish (assembly->image); + if (assembly->dynamic) { g_free ((char*)assembly->aname.culture); } else { g_free (assembly); } +} - mono_profiler_assembly_event (assembly, MONO_PROFILE_END_UNLOAD); +/** + * mono_assembly_close: + * @assembly: the assembly to release. + * + * This method releases a reference to the @assembly. The assembly is + * only released when all the outstanding references to it are released. + */ +void +mono_assembly_close (MonoAssembly *assembly) +{ + if (mono_assembly_close_except_image_pools (assembly)) + mono_assembly_close_finish (assembly); } MonoImage* diff --git a/mono/metadata/domain.c b/mono/metadata/domain.c index 0ccb5147f50..902590b0551 100644 --- a/mono/metadata/domain.c +++ b/mono/metadata/domain.c @@ -1887,6 +1887,13 @@ mono_domain_free (MonoDomain *domain, gboolean force) appdomains_list [domain->domain_id] = NULL; mono_appdomains_unlock (); + /* must do this early as it accesses fields and types */ + if (domain->special_static_fields) { + mono_alloc_special_static_data_free (domain->special_static_fields); + g_hash_table_destroy (domain->special_static_fields); + domain->special_static_fields = NULL; + } + /* * We must destroy all these hash tables here because they * contain references to managed objects belonging to the @@ -1914,13 +1921,20 @@ mono_domain_free (MonoDomain *domain, gboolean force) for (tmp = domain->domain_assemblies; tmp; tmp = tmp->next) { MonoAssembly *ass = tmp->data; mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Unloading domain %s %p, assembly %s %p, refcount=%d\n", domain->friendly_name, domain, ass->aname.name, ass, ass->ref_count); - mono_assembly_close (ass); + if (!mono_assembly_close_except_image_pools (ass)) + tmp->data = NULL; } - g_slist_free (domain->domain_assemblies); - domain->domain_assemblies = NULL; mono_gc_clear_domain (domain); + for (tmp = domain->domain_assemblies; tmp; tmp = tmp->next) { + MonoAssembly *ass = tmp->data; + if (ass) + mono_assembly_close_finish (ass); + } + g_slist_free (domain->domain_assemblies); + domain->domain_assemblies = NULL; + /* FIXME: free delegate_hash_table when it's used */ if (domain->search_path) { g_strfreev (domain->search_path); @@ -1933,12 +1947,6 @@ mono_domain_free (MonoDomain *domain, gboolean force) domain->null_reference_ex = NULL; domain->stack_overflow_ex = NULL; domain->entry_assembly = NULL; - /* must do this early as it accesses fields and types */ - if (domain->special_static_fields) { - mono_alloc_special_static_data_free (domain->special_static_fields); - g_hash_table_destroy (domain->special_static_fields); - domain->special_static_fields = NULL; - } g_free (domain->friendly_name); domain->friendly_name = NULL; diff --git a/mono/metadata/image.c b/mono/metadata/image.c index 4e039adc681..852349fd3a9 100644 --- a/mono/metadata/image.c +++ b/mono/metadata/image.c @@ -1397,21 +1397,21 @@ free_hash (GHashTable *hash) g_hash_table_destroy (hash); } -/** - * mono_image_close: - * @image: The image file we wish to close - * - * Closes an image file, deallocates all memory consumed and - * unmaps all possible sections of the file +/* + * Returns whether mono_image_close_finish() must be called as well. + * We must unload images in two steps because clearing the domain in + * SGen requires the class metadata to be intact, but we need to free + * the mono_g_hash_tables in case a collection occurs during domain + * unloading and the roots would trip up the GC. */ -void -mono_image_close (MonoImage *image) +gboolean +mono_image_close_except_pools (MonoImage *image) { MonoImage *image2; GHashTable *loaded_images; int i; - g_return_if_fail (image != NULL); + g_return_val_if_fail (image != NULL, FALSE); /* * Atomically decrement the refcount and remove ourselves from the hash tables, so @@ -1421,7 +1421,7 @@ mono_image_close (MonoImage *image) if (InterlockedDecrement (&image->ref_count) > 0) { mono_images_unlock (); - return; + return FALSE; } loaded_images = image->ref_only ? loaded_images_refonly_hash : loaded_images_hash; @@ -1442,7 +1442,7 @@ mono_image_close (MonoImage *image) /* Image will be closed by _CorDllMain. */ FreeLibrary ((HMODULE) image->raw_data); mono_images_unlock (); - return; + return FALSE; } mono_images_unlock (); } @@ -1464,12 +1464,16 @@ mono_image_close (MonoImage *image) int i; for (i = 0; i < t->rows; i++) { - if (image->references [i]) - mono_assembly_close (image->references [i]); + if (image->references [i]) { + if (!mono_assembly_close_except_image_pools (image->references [i])) + image->references [i] = NULL; + } + } + } else { + if (image->references) { + g_free (image->references); + image->references = NULL; } - - g_free (image->references); - image->references = NULL; } #ifdef PLATFORM_WIN32 @@ -1587,21 +1591,56 @@ mono_image_close (MonoImage *image) } for (i = 0; i < image->module_count; ++i) { - if (image->modules [i]) - mono_image_close (image->modules [i]); + if (image->modules [i]) { + if (!mono_image_close_except_pools (image->modules [i])) + image->modules [i] = NULL; + } } - if (image->modules) - g_free (image->modules); if (image->modules_loaded) g_free (image->modules_loaded); - if (image->references) - g_free (image->references); - mono_perfcounters->loader_bytes -= mono_mempool_get_allocated (image->mempool); DeleteCriticalSection (&image->szarray_cache_lock); DeleteCriticalSection (&image->lock); /*g_print ("destroy image %p (dynamic: %d)\n", image, image->dynamic);*/ + if (image->dynamic) { + /* Dynamic images are GC_MALLOCed */ + g_free ((char*)image->module_name); + mono_dynamic_image_free ((MonoDynamicImage*)image); + } + + mono_profiler_module_event (image, MONO_PROFILE_END_UNLOAD); + + return TRUE; +} + +void +mono_image_close_finish (MonoImage *image) +{ + int i; + + if (image->references && !image->dynamic) { + MonoTableInfo *t = &image->tables [MONO_TABLE_ASSEMBLYREF]; + int i; + + for (i = 0; i < t->rows; i++) { + if (image->references [i]) + mono_assembly_close_finish (image->references [i]); + } + + g_free (image->references); + image->references = NULL; + } + + for (i = 0; i < image->module_count; ++i) { + if (image->modules [i]) + mono_image_close_finish (image->modules [i]); + } + if (image->modules) + g_free (image->modules); + + mono_perfcounters->loader_bytes -= mono_mempool_get_allocated (image->mempool); + if (!image->dynamic) { if (debug_assembly_unload) mono_mempool_invalidate (image->mempool); @@ -1610,16 +1649,25 @@ mono_image_close (MonoImage *image) g_free (image); } } else { - /* Dynamic images are GC_MALLOCed */ - g_free ((char*)image->module_name); - mono_dynamic_image_free ((MonoDynamicImage*)image); if (debug_assembly_unload) mono_mempool_invalidate (image->mempool); else mono_mempool_destroy (image->mempool); } +} - mono_profiler_module_event (image, MONO_PROFILE_END_UNLOAD); +/** + * mono_image_close: + * @image: The image file we wish to close + * + * Closes an image file, deallocates all memory consumed and + * unmaps all possible sections of the file + */ +void +mono_image_close (MonoImage *image) +{ + if (mono_image_close_except_pools (image)) + mono_image_close_finish (image); } /** diff --git a/mono/metadata/metadata-internals.h b/mono/metadata/metadata-internals.h index 4213ce3aa53..ac698f10186 100644 --- a/mono/metadata/metadata-internals.h +++ b/mono/metadata/metadata-internals.h @@ -413,6 +413,12 @@ mono_image_property_insert (MonoImage *image, gpointer subject, guint32 property void mono_image_property_remove (MonoImage *image, gpointer subject) MONO_INTERNAL; +gboolean +mono_image_close_except_pools (MonoImage *image) MONO_INTERNAL; + +void +mono_image_close_finish (MonoImage *image) MONO_INTERNAL; + MonoType* mono_metadata_get_shared_type (MonoType *type) MONO_INTERNAL; @@ -499,6 +505,10 @@ void mono_assembly_addref (MonoAssembly *assembly) MONO_INTERNAL; void mono_assembly_load_friends (MonoAssembly* ass) MONO_INTERNAL; gboolean mono_assembly_has_skip_verification (MonoAssembly* ass) MONO_INTERNAL; +gboolean mono_assembly_close_except_image_pools (MonoAssembly *assembly) MONO_INTERNAL; +void mono_assembly_close_finish (MonoAssembly *assembly) MONO_INTERNAL; + + gboolean mono_public_tokens_are_equal (const unsigned char *pubt1, const unsigned char *pubt2) MONO_INTERNAL; void mono_config_parse_publisher_policy (const char *filename, MonoAssemblyBindingInfo *binding_info) MONO_INTERNAL; -- 2.25.1