Merge pull request #2720 from mono/fix-39325

[Profiler] Keep a reference to assemblies while coverage is running

In certain circumstances, for example when running under nunit-console, assemblies could be unloaded while the profiler still had a reference to them in order to generate the coverage data. In these cases, the memory referred to by the profiler would be invalid and would crash.

Fix this by incrementing the reference count of the assemblies preventing them from being unloaded while coverage is being collected.

Fixes BXC #39325

diff --git a/mono/metadata/metadata-internals.h b/mono/metadata/metadata-internals.h
index 57fc425083ef37f62392541ec68810018d0ebe15..6b8fdcb44ca396a788f220bac28c4795f2bd4752 100644 (file)
--- a/mono/metadata/metadata-internals.h
+++ b/mono/metadata/metadata-internals.h
@@ -774,7 +774,7 @@ gboolean
 mono_metadata_generic_param_equal (MonoGenericParam *p1, MonoGenericParam *p2);
 
 void mono_dynamic_stream_reset  (MonoDynamicStream* stream);
-void mono_assembly_addref       (MonoAssembly *assembly);
+MONO_API void mono_assembly_addref       (MonoAssembly *assembly);
 void mono_assembly_load_friends (MonoAssembly* ass);
 gboolean mono_assembly_has_skip_verification (MonoAssembly* ass);
 

diff --git a/mono/profiler/proflog.c b/mono/profiler/proflog.c
index c32f40c62d32009737404f7732419fbdd4447a19..188a40a26479643bb1e32c3b4ca84e439d8d22da 100644 (file)
--- a/mono/profiler/proflog.c
+++ b/mono/profiler/proflog.c
@@ -12,6 +12,7 @@
 
 #include <config.h>
 #include "../mini/jit.h"
+#include "../metadata/metadata-internals.h"
 #include <mono/metadata/profiler.h>
 #include <mono/metadata/threads.h>
 #include <mono/metadata/mono-gc.h>
@@ -3772,6 +3773,11 @@ coverage_filter (MonoProfiler *prof, MonoMethod *method)
 
        assembly = mono_image_get_assembly (image);
 
+       // Need to keep the assemblies around for as long as they are kept in the hashtable
+       // Nunit, for example, has a habit of unloading them before the coverage statistics are
+       // generated causing a crash. See https://bugzilla.xamarin.com/show_bug.cgi?id=39325
+       mono_assembly_addref (assembly);
+
        mono_os_mutex_lock (&coverage_mutex);
        mono_conc_hashtable_insert (coverage_methods, method, method);
        mono_conc_hashtable_insert (coverage_assemblies, assembly, assembly);
@@ -3913,6 +3919,13 @@ coverage_init (MonoProfiler *prof)
 #endif /* DISABLE_HELPER_THREAD */
 }
 
+static void
+unref_coverage_assemblies (gpointer key, gpointer value, gpointer userdata)
+{
+       MonoAssembly *assembly = (MonoAssembly *)value;
+       mono_assembly_close (assembly);
+}
+
 static void
 log_shutdown (MonoProfiler *prof)
 {
@@ -3962,6 +3975,10 @@ log_shutdown (MonoProfiler *prof)
        mono_os_mutex_destroy (&prof->method_table_mutex);
 
        if (coverage_initialized) {
+               mono_os_mutex_lock (&coverage_mutex);
+               mono_conc_hashtable_foreach (coverage_assemblies, unref_coverage_assemblies, prof);
+               mono_os_mutex_unlock (&coverage_mutex);
+
                mono_conc_hashtable_destroy (coverage_methods);
                mono_conc_hashtable_destroy (coverage_assemblies);
                mono_conc_hashtable_destroy (coverage_classes);