From a84bc29a1b1ba74314ed71e5fe6d74ceddef5c3c Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 28 Aug 2025 12:22:07 +0300 Subject: [PATCH 1/4] [mono][interp] Fix leaking of compilation data For dynamic methods, imethod_alloc0 allocates from the dynamic method's mempool which is freed when the method is collected. We were previously allocating only from the global memory manager. --- src/mono/mono/mini/interp/transform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 266c658ba1563b..6e4b6dc9a31c7f 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -4655,7 +4655,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; @@ -4753,7 +4753,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 ()); From d4c23e868a18e3dbcc391079c23d8b3ca00599d9 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 28 Aug 2025 15:41:31 +0300 Subject: [PATCH 2/4] [mono][interp] Stop leaking data items Previously we were registering imethod locations from these data items in order for them to be patched once a method is tiered up. Because of this registering, the data item had to always be around. We now free the data item for dynamic methods and also we deregister the patch locations when freeing such a method. --- src/mono/mono/mini/interp/interp-internals.h | 1 + src/mono/mono/mini/interp/interp.c | 4 +- src/mono/mono/mini/interp/tiering.c | 50 ++++++++++++++++++++ src/mono/mono/mini/interp/tiering.h | 3 ++ src/mono/mono/mini/interp/transform.c | 5 +- 5 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/mini/interp/interp-internals.h b/src/mono/mono/mini/interp/interp-internals.h index 5fe04bf314c8c7..41407a2fd67235 100644 --- a/src/mono/mono/mini/interp/interp-internals.h +++ b/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; diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index f3cc76774ce8ea..6662284a130775 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -3635,11 +3635,13 @@ 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); +#if HOST_BROWSER mono_jiterp_free_method_data (method, imethod); #endif + 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); diff --git a/src/mono/mono/mini/interp/tiering.c b/src/mono/mono/mini/interp/tiering.c index a49b591e9d090e..6d332acf76bd17 100644 --- a/src/mono/mono/mini/interp/tiering.c +++ b/src/mono/mono/mini/interp/tiering.c @@ -128,6 +128,56 @@ register_imethod_data_item (gpointer data, gpointer user_data) } } +void +mono_interp_clear_data_items_patch_sites (gpointer *data_items, int n_data_items) +{ + // 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) { diff --git a/src/mono/mono/mini/interp/tiering.h b/src/mono/mono/mini/interp/tiering.h index dbd7da87ecd4d9..c50e4988b43972 100644 --- a/src/mono/mono/mini/interp/tiering.h +++ b/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); diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 6e4b6dc9a31c7f..368e6c620cac45 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -9856,10 +9856,9 @@ 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; From 60049836544cca148dfcfbf15666377ffbd016f9 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Thu, 28 Aug 2025 16:14:43 +0300 Subject: [PATCH 3/4] [mono][interp] Free headers allocated for inlined methods We add them to a list for later freeing. This uses the same pattern as jit. --- src/mono/mono/mini/interp/transform.c | 5 +++++ src/mono/mono/mini/interp/transform.h | 1 + 2 files changed, 6 insertions(+) diff --git a/src/mono/mono/mini/interp/transform.c b/src/mono/mono/mini/interp/transform.c index 368e6c620cac45..b642438261cc95 100644 --- a/src/mono/mono/mini/interp/transform.c +++ b/src/mono/mono/mini/interp/transform.c @@ -3310,6 +3310,8 @@ interp_inline_newobj (TransformData *td, MonoMethod *target_method, MonoMethodSi goto fail; td->ip += 5; + td->headers_to_free = g_slist_prepend_mempool (td->mempool, td->headers_to_free, mheader); + push_var (td, dreg); return TRUE; fail: @@ -3911,6 +3913,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; } @@ -9935,6 +9938,8 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG 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) { diff --git a/src/mono/mono/mini/interp/transform.h b/src/mono/mono/mini/interp/transform.h index 16e5cc015ae039..3fe99ac32f7f4a 100644 --- a/src/mono/mono/mini/interp/transform.h +++ b/src/mono/mono/mini/interp/transform.h @@ -316,6 +316,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; From 4515bc741799a432b2ebdba1a2308f2157466873 Mon Sep 17 00:00:00 2001 From: Vlad Brezae Date: Fri, 29 Aug 2025 16:12:21 +0300 Subject: [PATCH 4/4] [mono][interp] Skip interp free for methods not yet compiled --- src/mono/mono/mini/interp/interp.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/mono/mono/mini/interp/interp.c b/src/mono/mono/mini/interp/interp.c index 6662284a130775..0a12875dcce179 100644 --- a/src/mono/mono/mini/interp/interp.c +++ b/src/mono/mono/mini/interp/interp.c @@ -3636,13 +3636,16 @@ interp_free_method (MonoMethod *method) jit_mm_lock (jit_mm); InterpMethod *imethod = (InterpMethod*)mono_internal_hash_table_lookup (&jit_mm->interp_code_hash, method); + if (imethod) { #if HOST_BROWSER - mono_jiterp_free_method_data (method, imethod); + mono_jiterp_free_method_data (method, imethod); #endif - mono_interp_clear_data_items_patch_sites (imethod->data_items, imethod->n_data_items); + 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); + } - mono_internal_hash_table_remove (&jit_mm->interp_code_hash, method); jit_mm_unlock (jit_mm); if (dmethod->mp) {