[mono][interp] Fix memory leaks for interpreted dynamic methods. (#88892)

* [mono] Set the 'dynamic' flag on method builders on creation so a MonoDynamicMethod is allocated instead of a MonoMethodWrapper.

* [mono][interp] Fix memory leaks for interpreted dynamic methods.

* Add a mempool to MonoDynamicMethod.
* Modify the intepreter code to allocate from this mempool when
  interpreting dynamic methods.

Author: vargaz

src/mono/mono/metadata/class-internals.h

@@ -102,6 +102,7 @@ struct _MonoMethodWrapper {
 struct _MonoDynamicMethod {
 	MonoMethodWrapper method;
 	MonoAssembly *assembly;
+	MonoMemPool *mp;
 };
 
 struct _MonoMethodPInvoke {

src/mono/mono/metadata/marshal-lightweight.c

@@ -2185,12 +2185,6 @@ mb_skip_visibility_ilgen (MonoMethodBuilder *mb)
 	mb->skip_visibility = 1;
 }
 
-static void
-mb_set_dynamic_ilgen (MonoMethodBuilder *mb)
-{
-	mb->dynamic = 1;
-}
-
 static void
 emit_synchronized_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethod *method, MonoGenericContext *ctx, MonoGenericContainer *container, MonoMethod *enter_method, MonoMethod *exit_method, MonoMethod *gettypefromhandle_method)
 {
@@ -3392,7 +3386,6 @@ mono_marshal_lightweight_init (void)
 	cb.emit_return = emit_return_ilgen;
 	cb.emit_vtfixup_ftnptr = emit_vtfixup_ftnptr_ilgen;
 	cb.mb_skip_visibility = mb_skip_visibility_ilgen;
-	cb.mb_set_dynamic = mb_set_dynamic_ilgen;
 	cb.mb_emit_exception = mb_emit_exception_ilgen;
 	cb.mb_emit_exception_for_error = mb_emit_exception_for_error_ilgen;
 	cb.mb_emit_byte = mb_emit_byte_ilgen;

src/mono/mono/metadata/marshal.c

@@ -4073,7 +4073,10 @@ marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, Mono
 
 	sig = mono_method_signature_internal (method);
 
-	mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_NATIVE_TO_MANAGED);
+	if (target_handle)
+		mb = mono_mb_new_dynamic (method->klass, method->name, MONO_WRAPPER_NATIVE_TO_MANAGED);
+	else
+		mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_NATIVE_TO_MANAGED);
 
 	/*the target gchandle must be the first entry after size and the wrapper itself.*/
 	mono_mb_add_data (mb, target_handle);
@@ -4181,7 +4184,6 @@ marshal_get_managed_wrapper (MonoMethod *method, MonoClass *delegate_klass, Mono
 												 mb, csig, sig->param_count + 16,
 												 info, NULL);
 		} else {
-			get_marshal_cb ()->mb_set_dynamic (mb);
 			res = mono_mb_create (mb, csig, sig->param_count + 16, NULL);
 		}
 	}
@@ -4254,7 +4256,7 @@ mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16 type)
 		mspecs = g_new0 (MonoMarshalSpec*, sig->param_count + 1);
 		mono_method_get_marshal_info (method, mspecs);
 
-		mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_NATIVE_TO_MANAGED);
+		mb = mono_mb_new_dynamic (method->klass, method->name, MONO_WRAPPER_NATIVE_TO_MANAGED);
 		csig = mono_metadata_signature_dup_full (image, sig);
 		csig->hasthis = 0;
 		csig->pinvoke = 1;
@@ -4277,7 +4279,6 @@ mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16 type)
 		mono_marshal_emit_managed_wrapper (mb, sig, mspecs, &m, method, 0, error);
 		mono_error_assert_ok (error);
 
-		get_marshal_cb ()->mb_set_dynamic (mb);
 		method = mono_mb_create (mb, csig, sig->param_count + 16, NULL);
 		mono_mb_free (mb);
 
@@ -4292,11 +4293,10 @@ mono_marshal_get_vtfixup_ftnptr (MonoImage *image, guint32 token, guint16 type)
 	}
 
 	sig = mono_method_signature_internal (method);
-	mb = mono_mb_new (method->klass, method->name, MONO_WRAPPER_MANAGED_TO_MANAGED);
+	mb = mono_mb_new_dynamic (method->klass, method->name, MONO_WRAPPER_MANAGED_TO_MANAGED);
 
 	param_count = sig->param_count + sig->hasthis;
 	get_marshal_cb ()->emit_vtfixup_ftnptr (mb, method, param_count, type);
-	get_marshal_cb ()->mb_set_dynamic (mb);
 
 	method = mono_mb_create (mb, sig, param_count, NULL);
 	mono_mb_free (mb);

src/mono/mono/metadata/marshal.h

@@ -351,7 +351,6 @@ typedef struct {
 	void (*emit_return) (MonoMethodBuilder *mb);
 	void (*emit_vtfixup_ftnptr) (MonoMethodBuilder *mb, MonoMethod *method, int param_count, guint16 type);
 	void (*mb_skip_visibility) (MonoMethodBuilder *mb);
-	void (*mb_set_dynamic) (MonoMethodBuilder *mb);
 	void (*mb_emit_exception) (MonoMethodBuilder *mb, const char *exc_nspace, const char *exc_name, const char *msg);
 	void (*mb_emit_exception_for_error) (MonoMethodBuilder *mb, const MonoError *emitted_error);
 	void (*mb_emit_byte) (MonoMethodBuilder *mb, guint8 op);

src/mono/mono/metadata/method-builder-ilgen.c

@@ -29,7 +29,7 @@ enum {
 #undef OPDEF
 
 static MonoMethodBuilder *
-new_base_ilgen (MonoClass *klass, MonoWrapperType type)
+new_base_ilgen (MonoClass *klass, MonoWrapperType type, gboolean dynamic)
 {
 	MonoMethodBuilder *mb;
 	MonoMethod *m;
@@ -38,7 +38,10 @@ new_base_ilgen (MonoClass *klass, MonoWrapperType type)
 
 	mb = g_new0 (MonoMethodBuilder, 1);
 
-	mb->method = m = (MonoMethod *)g_new0 (MonoMethodWrapper, 1);
+	if (dynamic)
+		mb->method = m = (MonoMethod *)g_new0 (MonoDynamicMethod, 1);
+	else
+		mb->method = m = (MonoMethod *)g_new0 (MonoMethodWrapper, 1);
 
 	m->klass = klass;
 	m->inline_info = 1;
@@ -47,6 +50,7 @@ new_base_ilgen (MonoClass *klass, MonoWrapperType type)
 	mb->code_size = 40;
 	mb->code = (unsigned char *)g_malloc (mb->code_size);
 	mb->init_locals = TRUE;
+	mb->dynamic = dynamic;
 
 	/* placeholder for the wrapper always at index 1 */
 	mono_mb_add_data (mb, NULL);
@@ -109,7 +113,7 @@ create_method_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *signature, int
 	image = m_class_get_image (mb->method->klass);
 
 	if (mb->dynamic) {
-		/* Allocated in reflection_methodbuilder_to_mono_method () */
+		/* Allocated in reflection_methodbuilder_to_mono_method ()/mb_new () */
 		method = mb->method;
 	} else {
 		method = (MonoMethod *)mb_alloc0 (mb, sizeof (MonoMethodWrapper));

src/mono/mono/metadata/method-builder.c

@@ -77,7 +77,7 @@ get_mb_cb (void)
 MonoMethodBuilder *
 mono_mb_new_no_dup_name (MonoClass *klass, const char *name, MonoWrapperType type)
 {
-	MonoMethodBuilder *mb = get_mb_cb ()->new_base (klass, type);
+	MonoMethodBuilder *mb = get_mb_cb ()->new_base (klass, type, FALSE);
 	mb->name = (char*)name;
 	mb->no_dup_name = TRUE;
 	return mb;
@@ -89,7 +89,15 @@ mono_mb_new_no_dup_name (MonoClass *klass, const char *name, MonoWrapperType typ
 MonoMethodBuilder *
 mono_mb_new (MonoClass *klass, const char *name, MonoWrapperType type)
 {
-	MonoMethodBuilder *mb = get_mb_cb ()->new_base (klass, type);
+	MonoMethodBuilder *mb = get_mb_cb ()->new_base (klass, type, FALSE);
+	mb->name = g_strdup (name);
+	return mb;
+}
+
+MonoMethodBuilder *
+mono_mb_new_dynamic (MonoClass *klass, const char *name, MonoWrapperType type)
+{
+	MonoMethodBuilder *mb = get_mb_cb ()->new_base (klass, type, TRUE);
 	mb->name = g_strdup (name);
 	return mb;
 }

src/mono/mono/metadata/method-builder.h

@@ -26,14 +26,17 @@ typedef struct _MonoMethodBuilder MonoMethodBuilder;
 
 typedef struct {
 	int version;
-	MonoMethodBuilder* (*new_base) (MonoClass *klass, MonoWrapperType type);
+	MonoMethodBuilder* (*new_base) (MonoClass *klass, MonoWrapperType type, gboolean dynamic);
 	void (*free) (MonoMethodBuilder *mb);
 	MonoMethod* (*create_method) (MonoMethodBuilder *mb, MonoMethodSignature *signature, int max_stack);
 } MonoMethodBuilderCallbacks;
 
 MONO_COMPONENT_API MonoMethodBuilder *
 mono_mb_new (MonoClass *klass, const char *name, MonoWrapperType type);
 
+MONO_COMPONENT_API MonoMethodBuilder *
+mono_mb_new_dynamic (MonoClass *klass, const char *name, MonoWrapperType type);
+
 MonoMethodBuilder *
 mono_mb_new_no_dup_name (MonoClass *klass, const char *name, MonoWrapperType type);
 

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

@@ -488,7 +488,15 @@ mono_interp_get_imethod (MonoMethod *method)
 
 	sig = mono_method_signature_internal (method);
 
-	imethod = (InterpMethod*)m_method_alloc0 (method, sizeof (InterpMethod));
+	MonoMemPool *dyn_mp = NULL;
+	if (method->dynamic)
+		// FIXME: Use this in more places
+		dyn_mp = mono_mempool_new_size (sizeof (InterpMethod));
+
+	if (dyn_mp)
+		imethod = (InterpMethod*)mono_mempool_alloc0 (dyn_mp, sizeof (InterpMethod));
+	else
+		imethod = (InterpMethod*)m_method_alloc0 (method, sizeof (InterpMethod));
 	imethod->method = method;
 	imethod->param_count = sig->param_count;
 	imethod->hasthis = sig->hasthis;
@@ -504,15 +512,22 @@ mono_interp_get_imethod (MonoMethod *method)
 		imethod->rtype = m_class_get_byval_arg (mono_defaults.string_class);
 	else
 		imethod->rtype = mini_get_underlying_type (sig->ret);
-	imethod->param_types = (MonoType**)m_method_alloc0 (method, sizeof (MonoType*) * sig->param_count);
+	if (dyn_mp)
+		imethod->param_types = (MonoType**)mono_mempool_alloc0 (dyn_mp, sizeof (MonoType*) * sig->param_count);
+	else
+		imethod->param_types = (MonoType**)m_method_alloc0 (method, sizeof (MonoType*) * sig->param_count);
 	for (i = 0; i < sig->param_count; ++i)
 		imethod->param_types [i] = mini_get_underlying_type (sig->params [i]);
 
 	jit_mm_lock (jit_mm);
 	InterpMethod *old_imethod;
-	if (!((old_imethod = mono_internal_hash_table_lookup (&jit_mm->interp_code_hash, method))))
+	if (!((old_imethod = mono_internal_hash_table_lookup (&jit_mm->interp_code_hash, method)))) {
 		mono_internal_hash_table_insert (&jit_mm->interp_code_hash, method, imethod);
-	else {
+		if (method->dynamic)
+			((MonoDynamicMethod*)method)->mp = dyn_mp;
+	} else {
+		if (dyn_mp)
+			mono_mempool_destroy (dyn_mp);
 		imethod = old_imethod; /* leak the newly allocated InterpMethod to the mempool */
 	}
 	jit_mm_unlock (jit_mm);
@@ -1268,6 +1283,15 @@ compute_arg_offset (MonoMethodSignature *sig, int index)
 	return offset;
 }
 
+static gpointer
+imethod_alloc0 (InterpMethod *imethod, size_t size)
+{
+	if (imethod->method->dynamic)
+		return mono_mempool_alloc0 (((MonoDynamicMethod*)imethod->method)->mp, (guint)size);
+	else
+		return m_method_alloc0 (imethod->method, (guint)size);
+}
+
 static guint32*
 initialize_arg_offsets (InterpMethod *imethod, MonoMethodSignature *csig)
 {
@@ -1280,7 +1304,7 @@ initialize_arg_offsets (InterpMethod *imethod, MonoMethodSignature *csig)
 	if (!sig)
 		sig = mono_method_signature_internal (imethod->method);
 	int arg_count = sig->hasthis + sig->param_count;
-	guint32 *arg_offsets = (guint32*) g_malloc ((arg_count + 1) * sizeof (int));
+	guint32 *arg_offsets = (guint32*)imethod_alloc0 (imethod, (arg_count + 1) * sizeof (int));
 	int index = 0, offset = 0;
 
 	if (sig->hasthis) {
@@ -1302,8 +1326,8 @@ initialize_arg_offsets (InterpMethod *imethod, MonoMethodSignature *csig)
 	arg_offsets [index] = ALIGN_TO (offset, MINT_STACK_SLOT_SIZE);
 
 	mono_memory_write_barrier ();
-	if (mono_atomic_cas_ptr ((gpointer*)&imethod->arg_offsets, arg_offsets, NULL) != NULL)
-		g_free (arg_offsets);
+	/* If this fails, the new one is leaked in the mem manager */
+	mono_atomic_cas_ptr ((gpointer*)&imethod->arg_offsets, arg_offsets, NULL);
 	return imethod->arg_offsets;
 }
 
@@ -3259,8 +3283,7 @@ interp_create_method_pointer_llvmonly (MonoMethod *method, gboolean unbox, MonoE
 
 	addr = mini_llvmonly_create_ftndesc (method, entry_wrapper, entry_ftndesc);
 
-	// FIXME:
-	MonoJitMemoryManager *jit_mm = get_default_jit_mm ();
+	MonoJitMemoryManager *jit_mm = jit_mm_for_method (method);
 	jit_mm_lock (jit_mm);
 	if (!jit_mm->interp_method_pointer_hash)
 		jit_mm->interp_method_pointer_hash = g_hash_table_new (NULL, NULL);
@@ -3438,12 +3461,24 @@ static void
 interp_free_method (MonoMethod *method)
 {
 	MonoJitMemoryManager *jit_mm = jit_mm_for_method (method);
+	InterpMethod *imethod;
+	MonoDynamicMethod *dmethod = (MonoDynamicMethod*)method;
 
 	jit_mm_lock (jit_mm);
-	/* InterpMethod is allocated in the domain mempool. We might haven't
-	 * allocated an InterpMethod for this instance yet */
+	imethod = (InterpMethod*)mono_internal_hash_table_lookup (&jit_mm->interp_code_hash, method);
 	mono_internal_hash_table_remove (&jit_mm->interp_code_hash, method);
+	if (imethod && jit_mm->interp_method_pointer_hash) {
+		if (imethod->jit_entry)
+			g_hash_table_remove (jit_mm->interp_method_pointer_hash, imethod->jit_entry);
+		if (imethod->llvmonly_unbox_entry)
+			g_hash_table_remove (jit_mm->interp_method_pointer_hash, imethod->llvmonly_unbox_entry);
+	}
 	jit_mm_unlock (jit_mm);
+
+	if (dmethod->mp) {
+		mono_mempool_destroy (dmethod->mp);
+		dmethod->mp = NULL;
+	}
 }
 
 #if COUNT_OPS
@@ -8007,7 +8042,7 @@ interp_run_clause_with_il_state (gpointer il_state_ptr, int clause_index, MonoOb
 	imethod = mono_interp_get_imethod (il_state->method);
 	if (!imethod->transformed) {
 		// In case method is in process of being tiered up, make sure it is compiled
-                mono_interp_transform_method (imethod, context, error);
+		mono_interp_transform_method (imethod, context, error);
 		mono_error_assert_ok (error);
 	}
 

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

@@ -1287,6 +1287,15 @@ emit_ldptr (TransformData *td, gpointer data)
 static MintICallSig
 interp_get_icall_sig (MonoMethodSignature *sig);
 
+static gpointer
+imethod_alloc0 (TransformData *td, size_t size)
+{
+	if (td->rtm->method->dynamic)
+		return mono_mempool_alloc0 (((MonoDynamicMethod*)td->rtm->method)->mp, (guint)size);
+	else
+		return mono_mem_manager_alloc0 (td->mem_manager, (guint)size);
+}
+
 static void
 interp_generate_icall_throw (TransformData *td, MonoJitICallInfo *icall_info, gpointer arg1, gpointer arg2)
 {
@@ -7657,7 +7666,7 @@ generate_code (TransformData *td, MonoMethod *method, MonoMethodHeader *header,
 				push_simple_type (td, STACK_TYPE_I);
 				interp_ins_set_dreg (td->last_ins, td->sp [-1].local);
 				/* This is a memory slot used by the wrapper */
-				gpointer addr = mono_mem_manager_alloc0 (td->mem_manager, sizeof (gpointer));
+				gpointer addr = imethod_alloc0 (td, sizeof (gpointer));
 				td->last_ins->data [0] = get_data_item_index (td, addr);
 				break;
 			}
@@ -8637,11 +8646,11 @@ generate_compacted_code (InterpMethod *rtm, TransformData *td)
 	size = compute_native_offset_estimates (td);
 
 	// Generate the compacted stream of instructions
-	td->new_code = ip = (guint16*)mono_mem_manager_alloc0 (td->mem_manager, size * sizeof (guint16));
+	td->new_code = ip = (guint16*)imethod_alloc0 (td, size * sizeof (guint16));
 
 	if (td->patchpoint_data_n) {
 		g_assert (mono_interp_tiering_enabled ());
-		td->patchpoint_data = (int*)mono_mem_manager_alloc0 (td->mem_manager, (td->patchpoint_data_n * 2 + 1) * sizeof (int));
+		td->patchpoint_data = (int*)imethod_alloc0 (td, (td->patchpoint_data_n * 2 + 1) * sizeof (int));
 		td->patchpoint_data [td->patchpoint_data_n * 2] = G_MAXINT32;
 	}
 
@@ -8697,7 +8706,7 @@ generate_compacted_code (InterpMethod *rtm, TransformData *td)
 
 #if HOST_BROWSER
 	if (backward_branch_offsets_count > 0) {
-		rtm->backward_branch_offsets = mono_mem_manager_alloc(td->mem_manager, backward_branch_offsets_count * sizeof(guint16));
+		rtm->backward_branch_offsets = imethod_alloc0 (td, backward_branch_offsets_count * sizeof(guint16));
 		rtm->backward_branch_offsets_count = backward_branch_offsets_count;
 		memcpy(rtm->backward_branch_offsets, backward_branch_offsets, backward_branch_offsets_count * sizeof(guint16));
 	}
@@ -8709,7 +8718,7 @@ generate_compacted_code (InterpMethod *rtm, TransformData *td)
 static void
 interp_mark_reachable_bblocks (TransformData *td)
 {
-	InterpBasicBlock **queue = mono_mem_manager_alloc0 (td->mem_manager, td->bb_count * sizeof (InterpBasicBlock*));
+	InterpBasicBlock **queue = mono_mempool_alloc0 (td->mempool, td->bb_count * sizeof (InterpBasicBlock*));
 	InterpBasicBlock *current;
 	int cur_index = 0;
 	int next_position = 0;
@@ -11208,7 +11217,7 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
 	code_len_u8 = GPTRDIFF_TO_UINT32 ((guint8 *) td->new_code_end - (guint8 *) td->new_code);
 	code_len_u16 = GPTRDIFF_TO_UINT32 (td->new_code_end - td->new_code);
 
-	rtm->clauses = (MonoExceptionClause*)mono_mem_manager_alloc0 (td->mem_manager, header->num_clauses * sizeof (MonoExceptionClause));
+	rtm->clauses = (MonoExceptionClause*)imethod_alloc0 (td, header->num_clauses * sizeof (MonoExceptionClause));
 	memcpy (rtm->clauses, header->clauses, header->num_clauses * sizeof(MonoExceptionClause));
 	rtm->code = (gushort*)td->new_code;
 	rtm->init_locals = header->init_locals;
@@ -11232,6 +11241,8 @@ 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]));
 	memcpy (rtm->data_items, td->data_items, td->n_data_items * sizeof (td->data_items [0]));
 
@@ -11245,7 +11256,7 @@ generate (MonoMethod *method, MonoMethodHeader *header, InterpMethod *rtm, MonoG
 	int jinfo_len;
 	jinfo_len = mono_jit_info_size ((MonoJitInfoFlags)0, header->num_clauses, 0);
 	MonoJitInfo *jinfo;
-	jinfo = (MonoJitInfo *)mono_mem_manager_alloc0 (td->mem_manager, jinfo_len);
+	jinfo = (MonoJitInfo *)imethod_alloc0 (td, jinfo_len);
 	jinfo->is_interp = 1;
 	rtm->jinfo = jinfo;
 	mono_jit_info_init (jinfo, method, (guint8*)rtm->code, code_len_u8, (MonoJitInfoFlags)0, header->num_clauses, 0);