Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jul 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
7 changes: 0 additions & 7 deletions src/mono/mono/metadata/marshal-lightweight.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 6 additions & 6 deletions src/mono/mono/metadata/marshal.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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);

Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/mono/mono/metadata/marshal.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 7 additions & 3 deletions src/mono/mono/metadata/method-builder-ilgen.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
12 changes: 10 additions & 2 deletions src/mono/mono/metadata/method-builder.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion src/mono/mono/metadata/method-builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

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) {
vargaz marked this conversation as resolved.
Show resolved Hide resolved
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
Loading