[release/9.0-staging] [mono][interp] Fix various leaks, primarily around dynamic methods

src/mono/mono/metadata/loader.c

@@ -36,6 +36,7 @@
 #include <mono/metadata/loader-internals.h>
 #include <mono/metadata/class-init.h>
 #include <mono/metadata/class-internals.h>
+#include <mono/metadata/components.h>
 #include <mono/metadata/debug-helpers.h>
 #include <mono/metadata/reflection.h>
 #include <mono/metadata/profiler-private.h>
@@ -78,6 +79,8 @@ static gint32 memberref_sig_cache_size;
 static gint32 methods_size;
 static gint32 signatures_size;
 
+static gboolean mono_enable_dynfree = FALSE;
+
 void
 mono_loader_init (void)
 {
@@ -103,6 +106,11 @@ mono_loader_init (void)
 		mono_counters_register ("MonoMethodSignature size",
 								MONO_COUNTER_METADATA | MONO_COUNTER_INT, &signatures_size);
 
+		char *env_opt = g_getenv ("MONO_ENABLE_DYNMETHOD_FREE");
+		if (env_opt && env_opt [0] == '1')
+			mono_enable_dynfree = TRUE;
+		g_free (env_opt);
+
 		inited = TRUE;
 	}
 }
@@ -1371,8 +1379,14 @@ mono_free_method  (MonoMethod *method)
 
 	MONO_PROFILER_RAISE (method_free, (method));
 
-	/* FIXME: This hack will go away when the profiler will support freeing methods */
-	if (G_UNLIKELY (mono_profiler_installed ()))
+	if (G_UNLIKELY (mono_profiler_installed () && !mono_enable_dynfree))
+		return;
+
+	// EventPipe might require information about methods to be stored throughout
+	// entire app execution, so stack traces can be resolved at a later time.
+	// Same for debugger, we are being overly conservative
+	if (mono_component_event_pipe ()->component.available () ||
+			mono_component_debugger ()->component.available ())
 		return;
 
 	if (method->signature) {

src/mono/mono/mini/interp/interp-internals.h

@@ -148,6 +148,7 @@ struct InterpMethod {
 	/* locals_size is equal to the offset of the param_area */
 	guint32 locals_size;
 	guint32 alloca_size;
+	int n_data_items;
 	int num_clauses; // clauses
 	int transformed; // boolean
 	unsigned int param_count;

src/mono/mono/mini/interp/interp.c

@@ -3565,12 +3565,17 @@ interp_free_method (MonoMethod *method)
 
 	jit_mm_lock (jit_mm);
 
-#if HOST_BROWSER
 	InterpMethod *imethod = (InterpMethod*)mono_internal_hash_table_lookup (&jit_mm->interp_code_hash, method);
-	mono_jiterp_free_method_data (method, imethod);
+	if (imethod) {
+#if HOST_BROWSER
+		mono_jiterp_free_method_data (method, imethod);
 #endif
 
-	mono_internal_hash_table_remove (&jit_mm->interp_code_hash, method);
+		mono_interp_clear_data_items_patch_sites (imethod->data_items, imethod->n_data_items);
+
+		mono_internal_hash_table_remove (&jit_mm->interp_code_hash, method);
+	}
+
 	jit_mm_unlock (jit_mm);
 
 	if (dmethod->mp) {

src/mono/mono/mini/interp/tiering.c

@@ -128,6 +128,58 @@ register_imethod_data_item (gpointer data, gpointer user_data)
 	}
 }
 
+void
+mono_interp_clear_data_items_patch_sites (gpointer *data_items, int n_data_items)
+{
+	if (!enable_tiering)
+		return;
+	// data_items is part of the memory of a dynamic method that is being freed.
+	// slots within this memory can be registered as patch sites for other imethods
+	// We conservatively assume each slot could be an imethod slot, then look it up
+	// in imethod to patch_sites hashtable. If we find it in the hashtable, we remove
+	// the slot from the patch site list.
+	mono_os_mutex_lock (&tiering_mutex);
+
+	for (int i = 0; i < n_data_items; i++) {
+		GSList *sites;
+		gpointer *slot = data_items + i;
+		gpointer imethod_candidate = *slot;
+
+		if (dn_simdhash_ptr_ptr_try_get_value (patch_sites_table, imethod_candidate, (void **)&sites)) {
+			GSList *prev = NULL;
+
+			// Remove slot from sites list
+			if (sites->data == slot) {
+				// If the slot is found in the first element we will also need to update the hash table since
+				// the list head changes
+				if (!sites->next) {
+					g_slist_free_1 (sites);
+					dn_simdhash_ptr_ptr_try_remove (patch_sites_table, imethod_candidate);
+				} else {
+					prev = sites;
+					sites = sites->next;
+					g_slist_free_1 (prev);
+					dn_simdhash_ptr_ptr_try_replace_value (patch_sites_table, imethod_candidate, sites);
+				}
+			} else {
+				prev = sites;
+				sites = sites->next;
+				while (sites != NULL) {
+					if (sites->data == slot) {
+						prev->next = sites->next;
+						g_slist_free_1 (sites);
+						// duplicates not allowed
+						break;
+					}
+					prev = sites;
+					sites = sites->next;
+				}
+			}
+		}
+	}
+	mono_os_mutex_unlock (&tiering_mutex);
+}
+
 void
 mono_interp_register_imethod_data_items (gpointer *data_items, GSList *indexes)
 {

src/mono/mono/mini/interp/tiering.h

@@ -14,6 +14,9 @@ mono_interp_tiering_enabled (void);
 void
 mono_interp_register_imethod_data_items (gpointer *data_items, GSList *indexes);
 
+void
+mono_interp_clear_data_items_patch_sites (gpointer *data_items, int n_data_items);
+
 void
 mono_interp_register_imethod_patch_site (gpointer *imethod_ptr);
 

src/mono/mono/mini/interp/transform.c

@@ -3176,6 +3176,8 @@ interp_inline_newobj (TransformData *td, MonoMethod *target_method, MonoMethodSi
 	if (!interp_inline_method (td, target_method, mheader, error))
 		goto fail;
 
+	td->headers_to_free = g_slist_prepend_mempool (td->mempool, td->headers_to_free, mheader);
+
 	push_var (td, dreg);
 	return TRUE;
 fail:
@@ -3760,6 +3762,7 @@ interp_transform_call (TransformData *td, MonoMethod *method, MonoMethod *target
 		return_val_if_nok (error, FALSE);
 
 		if (interp_inline_method (td, target_method, mheader, error)) {
+			td->headers_to_free = g_slist_prepend_mempool (td->mempool, td->headers_to_free, mheader);
 			td->ip += 5;
 			goto done;
 		}
@@ -4510,7 +4513,7 @@ interp_method_compute_offsets (TransformData *td, InterpMethod *imethod, MonoMet
 	// 64 vars * 72 bytes = 4608 bytes. Many methods need less than this
 	int target_vars_capacity = num_locals + 64;
 
-	imethod->local_offsets = (guint32*)g_malloc (num_il_locals * sizeof(guint32));
+	imethod->local_offsets = (guint32*)imethod_alloc0 (td, num_il_locals * sizeof(guint32));
 	td->vars = (InterpVar*)g_malloc0 (target_vars_capacity * sizeof (InterpVar));
 	td->vars_size = num_locals;
 	td->vars_capacity = target_vars_capacity;
@@ -4608,7 +4611,7 @@ interp_method_compute_offsets (TransformData *td, InterpMethod *imethod, MonoMet
 	}
 #endif
 
-	imethod->clause_data_offsets = (guint32*)g_malloc (header->num_clauses * sizeof (guint32));
+	imethod->clause_data_offsets = (guint32*)imethod_alloc0 (td, header->num_clauses * sizeof (guint32));
 	td->clause_vars = (int*)mono_mempool_alloc (td->mempool, sizeof (int) * header->num_clauses);
 	for (guint i = 0; i < header->num_clauses; i++) {
 		int var = interp_create_var (td, mono_get_object_type ());
@@ -9424,7 +9427,7 @@ get_native_offset (TransformData *td, int il_offset)
 	}
 }
 
-static void
+static GSList*
 generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoGenericContext *generic_context, MonoError *error)
 {
 	TransformData transform_data;
@@ -9609,12 +9612,10 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
 	rtm->alloca_size = td->total_locals_size + td->max_stack_size;
 	g_assert ((rtm->alloca_size % MINT_STACK_ALIGNMENT) == 0);
 	rtm->locals_size = td->param_area_offset;
-	// FIXME: Can't allocate this using imethod_alloc0 as its registered with mono_interp_register_imethod_data_items ()
-	//rtm->data_items = (gpointer*)imethod_alloc0 (td, td->n_data_items * sizeof (td->data_items [0]));
-	rtm->data_items = (gpointer*)mono_mem_manager_alloc0 (td->mem_manager, td->n_data_items * sizeof (td->data_items [0]));
+	rtm->data_items = (gpointer*)imethod_alloc0 (td, td->n_data_items * sizeof (td->data_items [0]));
 	memcpy (rtm->data_items, td->data_items, td->n_data_items * sizeof (td->data_items [0]));
+	rtm->n_data_items = td->n_data_items;
 
-	mono_interp_register_imethod_data_items (rtm->data_items, td->imethod_items);
 	rtm->patchpoint_data = td->patchpoint_data;
 
 	if (td->ref_slots) {
@@ -9688,14 +9689,17 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
 	g_ptr_array_free (td->seq_points, TRUE);
 	if (td->line_numbers)
 		g_array_free (td->line_numbers, TRUE);
-	g_slist_free (td->imethod_items);
+	for (GSList *l = td->headers_to_free; l; l = l->next)
+		mono_metadata_free_mh ((MonoMethodHeader *)l->data);
 	mono_mempool_destroy (td->mempool);
 	mono_interp_pgo_generate_end ();
 	if (td->retry_compilation) {
 		retry_compilation = TRUE;
 		retry_with_inlining = td->retry_with_inlining;
 		goto retry;
 	}
+
+	return td->imethod_items;
 }
 
 gboolean
@@ -9841,7 +9845,8 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon
 	memcpy (&tmp_imethod, imethod, sizeof (InterpMethod));
 	imethod = &tmp_imethod;
 
-	MONO_TIME_TRACK (mono_interp_stats.transform_time, generate (method, header, imethod, generic_context, error));
+	GSList *imethod_data_items;
+	MONO_TIME_TRACK (mono_interp_stats.transform_time, imethod_data_items = generate (method, header, imethod, generic_context, error));
 
 	mono_metadata_free_mh (header);
 
@@ -9862,6 +9867,8 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon
 		mono_interp_stats.methods_transformed++;
 		mono_atomic_fetch_add_i32 (&mono_jit_stats.methods_with_interp, 1);
 
+		mono_interp_register_imethod_data_items (imethod->data_items, imethod_data_items);
+
 		// FIXME Publishing of seq points seems to be racy with tiereing. We can have both tiered and untiered method
 		// running at the same time. We could therefore get the optimized imethod seq points for the unoptimized method.
 		gpointer seq_points = NULL;
@@ -9871,6 +9878,8 @@ mono_interp_transform_method (InterpMethod *imethod, ThreadContext *context, Mon
 	}
 	jit_mm_unlock (jit_mm);
 
+	g_slist_free (imethod_data_items);
+
 	if (mono_stats_method_desc && mono_method_desc_full_match (mono_stats_method_desc, imethod->method)) {
 		g_printf ("Printing runtime stats at method: %s\n", mono_method_get_full_name (imethod->method));
 		mono_runtime_print_stats ();

src/mono/mono/mini/interp/transform.h

@@ -314,6 +314,7 @@ typedef struct
 	// FIXME: ptr_u32
 	dn_simdhash_ptr_ptr_t *data_hash;
 	GSList *imethod_items;
+	GSList *headers_to_free;
 #ifdef ENABLE_EXPERIMENT_TIERED
 	// FIXME: ptr_u32
 	dn_simdhash_ptr_ptr_t *patchsite_hash;