From: Alex Rønne Petersen Date: Fri, 4 Sep 2015 07:21:12 +0000 (+0200) Subject: [runtime] Actually clean up context-static data segments. X-Git-Url: http://wien.tomnetworks.com/gitweb/?a=commitdiff_plain;h=6d15e070affb30a6a19b3bce06dfb308747b5ae1;hp=-c;p=mono.git [runtime] Actually clean up context-static data segments. --- 6d15e070affb30a6a19b3bce06dfb308747b5ae1 diff --git a/mcs/class/corlib/System.Runtime.Remoting.Contexts/Context.cs b/mcs/class/corlib/System.Runtime.Remoting.Contexts/Context.cs index d94abd59f38..1820cbe7fef 100644 --- a/mcs/class/corlib/System.Runtime.Remoting.Contexts/Context.cs +++ b/mcs/class/corlib/System.Runtime.Remoting.Contexts/Context.cs @@ -55,6 +55,7 @@ namespace System.Runtime.Remoting.Contexts { int domain_id; int context_id; UIntPtr static_data; /* GC-tracked */ + UIntPtr data; #endregion #pragma warning restore 169, 414 diff --git a/mcs/class/corlib/System/Environment.cs b/mcs/class/corlib/System/Environment.cs index d9267ac3230..e2edf0cb84d 100644 --- a/mcs/class/corlib/System/Environment.cs +++ b/mcs/class/corlib/System/Environment.cs @@ -57,7 +57,7 @@ namespace System { * of icalls, do not require an increment. */ #pragma warning disable 169 - private const int mono_corlib_version = 138; + private const int mono_corlib_version = 139; #pragma warning restore 169 [ComVisible (true)] diff --git a/mono/metadata/appdomain.c b/mono/metadata/appdomain.c index b015e748b0a..1e6fd0c6f75 100644 --- a/mono/metadata/appdomain.c +++ b/mono/metadata/appdomain.c @@ -79,7 +79,7 @@ * Changes which are already detected at runtime, like the addition * of icalls, do not require an increment. */ -#define MONO_CORLIB_VERSION 138 +#define MONO_CORLIB_VERSION 139 typedef struct { diff --git a/mono/metadata/domain-internals.h b/mono/metadata/domain-internals.h index b58b891c002..0eed290a2d2 100644 --- a/mono/metadata/domain-internals.h +++ b/mono/metadata/domain-internals.h @@ -235,11 +235,17 @@ struct _MonoJitInfo { #define MONO_SIZEOF_JIT_INFO (offsetof (struct _MonoJitInfo, clauses)) +typedef struct { + gpointer *static_data; /* Used to free the static data without going through the MonoAppContext object itself. */ + uint32_t gc_handle; +} ContextStaticData; + struct _MonoAppContext { MonoObject obj; gint32 domain_id; gint32 context_id; gpointer *static_data; + ContextStaticData *data; }; /* Lock-free allocator */ diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index fd83f30929e..6dc723925b1 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -149,6 +149,9 @@ static MonoGHashTable *threads=NULL; */ static GHashTable *contexts = NULL; +/* Cleanup queue for contexts. */ +static MonoReferenceQueue *context_queue; + /* * Threads which are starting up and they are not in the 'threads' hash yet. * When handle_store is called for a thread, it will be removed from this hash table. @@ -2552,6 +2555,31 @@ ves_icall_System_Threading_Volatile_Write_T (void *ptr, MonoObject *value) mono_gc_wbarrier_generic_store_atomic (ptr, value); } +static void +free_context (void *user_data) +{ + ContextStaticData *data = user_data; + + mono_threads_lock (); + + /* + * There is no guarantee that, by the point this reference queue callback + * has been invoked, the GC handle associated with the object will fail to + * resolve as one might expect. So if we don't free and remove the GC + * handle here, free_context_static_data_helper () could end up resolving + * a GC handle to an actually-dead context which would contain a pointer + * to an already-freed static data segment, resulting in a crash when + * accessing it. + */ + g_hash_table_remove (contexts, GUINT_TO_POINTER (data->gc_handle)); + + mono_threads_unlock (); + + mono_gchandle_free (data->gc_handle); + mono_free_static_data (data->static_data); + g_free (data); +} + void ves_icall_System_Runtime_Remoting_Contexts_Context_RegisterContext (MonoAppContext *ctx) { @@ -2562,10 +2590,24 @@ ves_icall_System_Runtime_Remoting_Contexts_Context_RegisterContext (MonoAppConte if (!contexts) contexts = g_hash_table_new (NULL, NULL); - context_adjust_static_data (ctx); + if (!context_queue) + context_queue = mono_gc_reference_queue_new (free_context); + gpointer gch = GUINT_TO_POINTER (mono_gchandle_new_weakref (&ctx->obj, FALSE)); g_hash_table_insert (contexts, gch, gch); + /* + * We use this intermediate structure to contain a duplicate pointer to + * the static data because we can't rely on being able to resolve the GC + * handle in the reference queue callback. + */ + ContextStaticData *data = g_new0 (ContextStaticData, 1); + data->gc_handle = GPOINTER_TO_UINT (gch); + ctx->data = data; + + context_adjust_static_data (ctx); + mono_gc_reference_queue_add (context_queue, &ctx->obj, data); + mono_threads_unlock (); mono_profiler_context_loaded (ctx); @@ -2577,9 +2619,7 @@ ves_icall_System_Runtime_Remoting_Contexts_Context_ReleaseContext (MonoAppContex /* * NOTE: Since finalizers are unreliable for the purposes of ensuring * cleanup in exceptional circumstances, we don't actually do any - * cleanup work here. We instead do this when we iterate the `contexts` - * hash table. The only purpose of this finalizer, at the moment, is to - * notify the profiler. + * cleanup work here. We instead do this via a reference queue. */ //g_print ("Releasing context %d in domain %d\n", ctx->context_id, ctx->domain_id); @@ -3716,6 +3756,7 @@ context_adjust_static_data (MonoAppContext *ctx) if (context_static_info.offset || context_static_info.idx > 0) { guint32 offset = MAKE_SPECIAL_STATIC_OFFSET (context_static_info.idx, context_static_info.offset, 0); mono_alloc_static_data (&ctx->static_data, offset, FALSE); + ctx->data->static_data = ctx->static_data; } } @@ -3734,21 +3775,17 @@ alloc_thread_static_data_helper (gpointer key, gpointer value, gpointer user) /* * LOCKING: requires that threads_mutex is held */ -static gboolean +static void alloc_context_static_data_helper (gpointer key, gpointer value, gpointer user) { - uint32_t gch = GPOINTER_TO_INT (key); - MonoAppContext *ctx = (MonoAppContext *) mono_gchandle_get_target (gch); + MonoAppContext *ctx = (MonoAppContext *) mono_gchandle_get_target (GPOINTER_TO_INT (key)); - if (!ctx) { - mono_gchandle_free (gch); - return TRUE; // Remove this key/value pair - } + if (!ctx) + return; guint32 offset = GPOINTER_TO_UINT (user); mono_alloc_static_data (&ctx->static_data, offset, FALSE); - - return FALSE; // Don't remove it + ctx->data->static_data = ctx->static_data; } static StaticDataFreeList* @@ -3840,7 +3877,7 @@ mono_alloc_special_static_data (guint32 static_type, guint32 size, guint32 align mono_g_hash_table_foreach (threads, alloc_thread_static_data_helper, GUINT_TO_POINTER (offset)); } else { if (contexts != NULL) - g_hash_table_foreach_remove (contexts, alloc_context_static_data_helper, GUINT_TO_POINTER (offset)); + g_hash_table_foreach (contexts, alloc_context_static_data_helper, GUINT_TO_POINTER (offset)); ACCESS_SPECIAL_STATIC_OFFSET (offset, type) = SPECIAL_STATIC_OFFSET_TYPE_CONTEXT; } @@ -3894,16 +3931,13 @@ free_thread_static_data_helper (gpointer key, gpointer value, gpointer user) /* * LOCKING: requires that threads_mutex is held */ -static gboolean +static void free_context_static_data_helper (gpointer key, gpointer value, gpointer user) { - uint32_t gch = GPOINTER_TO_INT (key); - MonoAppContext *ctx = (MonoAppContext *) mono_gchandle_get_target (gch); + MonoAppContext *ctx = (MonoAppContext *) mono_gchandle_get_target (GPOINTER_TO_INT (key)); - if (!ctx) { - mono_gchandle_free (gch); - return TRUE; // Remove this key/value pair - } + if (!ctx) + return; OffsetSize *data = user; int idx = ACCESS_SPECIAL_STATIC_OFFSET (data->offset, index); @@ -3911,12 +3945,10 @@ free_context_static_data_helper (gpointer key, gpointer value, gpointer user) char *ptr; if (!ctx->static_data || !ctx->static_data [idx]) - return FALSE; // Don't remove this key/value pair + return; ptr = ((char*) ctx->static_data [idx]) + off; mono_gc_bzero_atomic (ptr, data->size); - - return FALSE; // Don't remove this key/value pair } static void @@ -3945,7 +3977,7 @@ do_free_special_slot (guint32 offset, guint32 size) mono_g_hash_table_foreach (threads, free_thread_static_data_helper, &data); } else { if (contexts != NULL) - g_hash_table_foreach_remove (contexts, free_context_static_data_helper, &data); + g_hash_table_foreach (contexts, free_context_static_data_helper, &data); } if (!mono_runtime_is_shutting_down ()) {