Skip to content

Commit

Permalink
[mono][interp] Fix memory leaks for interpreted dynamic methods.
Browse files Browse the repository at this point in the history
* Add a mempool to MonoDynamicMethod.
* Modify the intepreter code to allocate from this mempool when
  interpreting dynamic methods.
  • Loading branch information
vargaz committed Jul 17, 2023
1 parent b7a4ea2 commit cf435b3
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/mono/mono/metadata/class-internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ struct _MonoMethodWrapper {
struct _MonoDynamicMethod {
MonoMethodWrapper method;
MonoAssembly *assembly;
MonoMemPool *mp;
};

struct _MonoMethodPInvoke {
Expand Down
59 changes: 47 additions & 12 deletions src/mono/mono/mini/interp/interp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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)
{
Expand All @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
25 changes: 18 additions & 7 deletions src/mono/mono/mini/interp/transform.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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));
}
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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]));

Expand All @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions src/mono/mono/mini/llvmonly-runtime.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,12 @@ mini_llvmonly_get_delegate_arg (MonoMethod *method, gpointer method_ptr)
MonoFtnDesc*
mini_llvmonly_create_ftndesc (MonoMethod *m, gpointer addr, gpointer arg)
{
MonoFtnDesc *ftndesc = (MonoFtnDesc*)m_method_alloc0 (m, sizeof (MonoFtnDesc));
MonoFtnDesc *ftndesc;

if (m->dynamic && ((MonoDynamicMethod*)m)->mp)
ftndesc = mono_mempool_alloc0 (((MonoDynamicMethod*)m)->mp, sizeof (MonoFtnDesc));
else
ftndesc = (MonoFtnDesc*)m_method_alloc0 (m, sizeof (MonoFtnDesc));
ftndesc->addr = addr;
ftndesc->arg = arg;
ftndesc->method = m;
Expand Down Expand Up @@ -459,8 +464,8 @@ mini_llvmonly_get_vtable_trampoline (MonoVTable *vt, int slot_index, int index)
if (slot_index < 0) {
/* Initialize the IMT trampoline to a 'trampoline' so the generated code doesn't have to initialize it */
// FIXME: Memory management
gpointer *ftndesc = g_malloc (2 * sizeof (gpointer));
IMTTrampInfo *info = g_new0 (IMTTrampInfo, 1);
gpointer *ftndesc = m_class_alloc0 (vt->klass, 2 * sizeof (gpointer));
IMTTrampInfo *info = m_class_alloc0 (vt->klass, sizeof (IMTTrampInfo));
info->vtable = vt;
info->slot = index;
ftndesc [0] = (gpointer)mini_llvmonly_initial_imt_tramp;
Expand Down

0 comments on commit cf435b3

Please sign in to comment.