From 77a8ed1b39af8caa6da04163605ec40283942062 Mon Sep 17 00:00:00 2001 From: Ludovic Henry Date: Fri, 16 Jun 2017 19:14:13 -0400 Subject: [PATCH] [threads] Rework MonoThreadInfoCallbacks.thread_{register,detach,unregister} callbacks (#5055) * [threads] Use MonoThreadInfo.{stack_start_limit,stack_end} in more places * [threads] Remove dead code * [threads] Rename MonoThreadInfoCallbacks.thread_{register,detach,unregister} to MonoThreadInfoCallbacks.thread_{attach,detach,detach_with_lock} respectively --- mono/metadata/boehm-gc.c | 23 ++++----- mono/metadata/domain.c | 4 +- mono/metadata/null-gc.c | 9 +--- mono/metadata/sgen-mono.c | 68 +++++++-------------------- mono/metadata/sgen-stw.c | 8 ++-- mono/metadata/threads.c | 20 ++------ mono/sgen/gc-internal-agnostic.h | 6 --- mono/sgen/sgen-client.h | 6 +-- mono/sgen/sgen-debug.c | 4 +- mono/sgen/sgen-gc.c | 8 ++-- mono/sgen/sgen-gc.h | 4 +- mono/unit-tests/test-conc-hashtable.c | 12 ++--- mono/utils/mono-threads.c | 67 ++++++++++++++------------ mono/utils/mono-threads.h | 23 +++++---- 14 files changed, 104 insertions(+), 158 deletions(-) diff --git a/mono/metadata/boehm-gc.c b/mono/metadata/boehm-gc.c index 42f4d16597d..4ad522d4dc3 100644 --- a/mono/metadata/boehm-gc.c +++ b/mono/metadata/boehm-gc.c @@ -57,9 +57,9 @@ static gboolean gc_initialized = FALSE; static mono_mutex_t mono_gc_lock; static void* -boehm_thread_register (MonoThreadInfo* info, void *baseptr); +boehm_thread_attach (MonoThreadInfo* info); static void -boehm_thread_unregister (MonoThreadInfo *p); +boehm_thread_detach_with_lock (MonoThreadInfo *p); static void boehm_thread_detach (MonoThreadInfo *p); static void @@ -110,7 +110,6 @@ mono_gc_base_init (void) { MonoThreadInfoCallbacks cb; const char *env; - int dummy; if (gc_initialized) return; @@ -239,16 +238,16 @@ mono_gc_base_init (void) } memset (&cb, 0, sizeof (cb)); - cb.thread_register = boehm_thread_register; - cb.thread_unregister = boehm_thread_unregister; + cb.thread_attach = boehm_thread_attach; cb.thread_detach = boehm_thread_detach; + cb.thread_detach_with_lock = boehm_thread_detach_with_lock; cb.mono_method_is_critical = (gboolean (*)(void *))mono_runtime_is_critical_method; mono_threads_init (&cb, sizeof (MonoThreadInfo)); mono_os_mutex_init (&mono_gc_lock); mono_os_mutex_init_recursive (&handle_section); - mono_thread_info_attach (&dummy); + mono_thread_info_attach (); GC_set_on_collection_event (on_gc_notification); GC_on_heap_resize = on_gc_heap_resize; @@ -376,20 +375,14 @@ mono_gc_is_gc_thread (void) return GC_thread_is_registered (); } -gboolean -mono_gc_register_thread (void *baseptr) -{ - return mono_thread_info_attach (baseptr) != NULL; -} - static void* -boehm_thread_register (MonoThreadInfo* info, void *baseptr) +boehm_thread_attach (MonoThreadInfo* info) { struct GC_stack_base sb; int res; /* TODO: use GC_get_stack_base instead of baseptr. */ - sb.mem_base = baseptr; + sb.mem_base = info->stack_end; res = GC_register_my_thread (&sb); if (res == GC_UNIMPLEMENTED) return NULL; /* Cannot happen with GC v7+. */ @@ -400,7 +393,7 @@ boehm_thread_register (MonoThreadInfo* info, void *baseptr) } static void -boehm_thread_unregister (MonoThreadInfo *p) +boehm_thread_detach_with_lock (MonoThreadInfo *p) { MonoNativeThreadId tid; diff --git a/mono/metadata/domain.c b/mono/metadata/domain.c index 7a12a7f4f72..1d353b1da07 100644 --- a/mono/metadata/domain.c +++ b/mono/metadata/domain.c @@ -464,7 +464,7 @@ mono_init_internal (const char *filename, const char *exe_filename, const char * MonoAssembly *ass = NULL; MonoImageOpenStatus status = MONO_IMAGE_OK; const MonoRuntimeInfo* runtimes [G_N_ELEMENTS (supported_runtimes) + 1] = { NULL }; - int n, dummy; + int n; #ifdef DEBUG_DOMAIN_UNLOAD debug_domain_unload = TRUE; @@ -501,7 +501,7 @@ mono_init_internal (const char *filename, const char *exe_filename, const char * mono_counters_register ("Max HashTable Chain Length", MONO_COUNTER_INT|MONO_COUNTER_METADATA, &mono_g_hash_table_max_chain_length); mono_gc_base_init (); - mono_thread_info_attach (&dummy); + mono_thread_info_attach (); mono_coop_mutex_init_recursive (&appdomains_mutex); diff --git a/mono/metadata/null-gc.c b/mono/metadata/null-gc.c index 597224a7672..9be275916e1 100644 --- a/mono/metadata/null-gc.c +++ b/mono/metadata/null-gc.c @@ -23,7 +23,6 @@ void mono_gc_base_init (void) { MonoThreadInfoCallbacks cb; - int dummy; mono_counters_init (); @@ -39,7 +38,7 @@ mono_gc_base_init (void) mono_threads_init (&cb, sizeof (MonoThreadInfo)); - mono_thread_info_attach (&dummy); + mono_thread_info_attach (); } void @@ -94,12 +93,6 @@ mono_gc_is_gc_thread (void) return TRUE; } -gboolean -mono_gc_register_thread (void *baseptr) -{ - return TRUE; -} - int mono_gc_walk_heap (int flags, MonoGCReferences callback, void *data) { diff --git a/mono/metadata/sgen-mono.c b/mono/metadata/sgen-mono.c index 0314150604f..508a9c68e7a 100644 --- a/mono/metadata/sgen-mono.c +++ b/mono/metadata/sgen-mono.c @@ -72,7 +72,7 @@ ptr_on_stack (void *ptr) gpointer stack_start = &stack_start; SgenThreadInfo *info = mono_thread_info_current (); - if (ptr >= stack_start && ptr < (gpointer)info->client_info.stack_end) + if (ptr >= stack_start && ptr < (gpointer)info->client_info.info.stack_end) return TRUE; return FALSE; } @@ -2209,11 +2209,8 @@ mono_gc_get_gc_callbacks () } void -sgen_client_thread_register (SgenThreadInfo* info, void *stack_bottom_fallback) +sgen_client_thread_attach (SgenThreadInfo* info) { - size_t stsize = 0; - guint8 *staddr = NULL; - mono_tls_set_sgen_thread_info (info); info->client_info.skip = 0; @@ -2225,17 +2222,6 @@ sgen_client_thread_register (SgenThreadInfo* info, void *stack_bottom_fallback) info->client_info.signal = 0; #endif - mono_thread_info_get_stack_bounds (&staddr, &stsize); - if (staddr) { - info->client_info.stack_start_limit = staddr; - info->client_info.stack_end = staddr + stsize; - } else { - gsize stack_bottom = (gsize)stack_bottom_fallback; - stack_bottom += 4095; - stack_bottom &= ~4095; - info->client_info.stack_end = (char*)stack_bottom; - } - memset (&info->client_info.ctx, 0, sizeof (MonoContext)); if (mono_gc_get_gc_callbacks ()->thread_attach_func) @@ -2243,13 +2229,13 @@ sgen_client_thread_register (SgenThreadInfo* info, void *stack_bottom_fallback) binary_protocol_thread_register ((gpointer)mono_thread_info_get_tid (info)); - SGEN_LOG (3, "registered thread %p (%p) stack end %p", info, (gpointer)mono_thread_info_get_tid (info), info->client_info.stack_end); + SGEN_LOG (3, "registered thread %p (%p) stack end %p", info, (gpointer)mono_thread_info_get_tid (info), info->client_info.info.stack_end); info->client_info.info.handle_stack = mono_handle_stack_alloc (); } void -sgen_client_thread_unregister (SgenThreadInfo *p) +sgen_client_thread_detach_with_lock (SgenThreadInfo *p) { MonoNativeThreadId tid; @@ -2297,13 +2283,6 @@ thread_in_critical_region (SgenThreadInfo *info) return info->client_info.in_critical_region; } -static void -sgen_thread_attach (SgenThreadInfo *info) -{ - if (mono_gc_get_gc_callbacks ()->thread_attach_func && !info->client_info.runtime_data) - info->client_info.runtime_data = mono_gc_get_gc_callbacks ()->thread_attach_func (); -} - static void sgen_thread_detach (SgenThreadInfo *p) { @@ -2317,15 +2296,6 @@ sgen_thread_detach (SgenThreadInfo *p) mono_thread_detach_internal (mono_thread_internal_current ()); } -/** - * mono_gc_register_thread: - */ -gboolean -mono_gc_register_thread (void *baseptr) -{ - return mono_thread_info_attach (baseptr) != NULL; -} - /** * mono_gc_is_gc_thread: */ @@ -2393,20 +2363,20 @@ sgen_client_scan_thread_data (void *start_nursery, void *end_nursery, gboolean p void *aligned_stack_start; if (info->client_info.skip) { - SGEN_LOG (3, "Skipping dead thread %p, range: %p-%p, size: %zd", info, info->client_info.stack_start, info->client_info.stack_end, (char*)info->client_info.stack_end - (char*)info->client_info.stack_start); + SGEN_LOG (3, "Skipping dead thread %p, range: %p-%p, size: %zd", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start); skip_reason = 1; } else if (info->client_info.gc_disabled) { - SGEN_LOG (3, "GC disabled for thread %p, range: %p-%p, size: %zd", info, info->client_info.stack_start, info->client_info.stack_end, (char*)info->client_info.stack_end - (char*)info->client_info.stack_start); + SGEN_LOG (3, "GC disabled for thread %p, range: %p-%p, size: %zd", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start); skip_reason = 2; } else if (!mono_thread_info_is_live (info)) { - SGEN_LOG (3, "Skipping non-running thread %p, range: %p-%p, size: %zd (state %x)", info, info->client_info.stack_start, info->client_info.stack_end, (char*)info->client_info.stack_end - (char*)info->client_info.stack_start, info->client_info.info.thread_state); + SGEN_LOG (3, "Skipping non-running thread %p, range: %p-%p, size: %zd (state %x)", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start, info->client_info.info.thread_state); skip_reason = 3; } else if (!info->client_info.stack_start) { SGEN_LOG (3, "Skipping starting or detaching thread %p", info); skip_reason = 4; } - binary_protocol_scan_stack ((gpointer)mono_thread_info_get_tid (info), info->client_info.stack_start, info->client_info.stack_end, skip_reason); + binary_protocol_scan_stack ((gpointer)mono_thread_info_get_tid (info), info->client_info.stack_start, info->client_info.info.stack_end, skip_reason); if (skip_reason) { if (precise) { @@ -2421,7 +2391,7 @@ sgen_client_scan_thread_data (void *start_nursery, void *end_nursery, gboolean p } g_assert (info->client_info.stack_start); - g_assert (info->client_info.stack_end); + g_assert (info->client_info.info.stack_end); aligned_stack_start = (void*)(mword) ALIGN_TO ((mword)info->client_info.stack_start, SIZEOF_VOID_P); #ifdef HOST_WIN32 @@ -2441,16 +2411,16 @@ sgen_client_scan_thread_data (void *start_nursery, void *end_nursery, gboolean p #endif g_assert (info->client_info.suspend_done); - SGEN_LOG (3, "Scanning thread %p, range: %p-%p, size: %zd, pinned=%zd", info, info->client_info.stack_start, info->client_info.stack_end, (char*)info->client_info.stack_end - (char*)info->client_info.stack_start, sgen_get_pinned_count ()); + SGEN_LOG (3, "Scanning thread %p, range: %p-%p, size: %zd, pinned=%zd", info, info->client_info.stack_start, info->client_info.info.stack_end, (char*)info->client_info.info.stack_end - (char*)info->client_info.stack_start, sgen_get_pinned_count ()); if (mono_gc_get_gc_callbacks ()->thread_mark_func && !conservative_stack_mark) { - mono_gc_get_gc_callbacks ()->thread_mark_func (info->client_info.runtime_data, (guint8 *)aligned_stack_start, (guint8 *)info->client_info.stack_end, precise, &ctx); + mono_gc_get_gc_callbacks ()->thread_mark_func (info->client_info.runtime_data, (guint8 *)aligned_stack_start, (guint8 *)info->client_info.info.stack_end, precise, &ctx); } else if (!precise) { if (!conservative_stack_mark) { fprintf (stderr, "Precise stack mark not supported - disabling.\n"); conservative_stack_mark = TRUE; } //FIXME we should eventually use the new stack_mark from coop - sgen_conservatively_pin_objects_from ((void **)aligned_stack_start, (void **)info->client_info.stack_end, start_nursery, end_nursery, PIN_TYPE_STACK); + sgen_conservatively_pin_objects_from ((void **)aligned_stack_start, (void **)info->client_info.info.stack_end, start_nursery, end_nursery, PIN_TYPE_STACK); } if (!precise) { @@ -2460,7 +2430,7 @@ sgen_client_scan_thread_data (void *start_nursery, void *end_nursery, gboolean p { // This is used on Coop GC for platforms where we cannot get the data for individual registers. // We force a spill of all registers into the stack and pass a chunk of data into sgen. - //FIXME under coop, for now, what we need to ensure is that we scan any extra memory from info->client_info.stack_end to stack_mark + //FIXME under coop, for now, what we need to ensure is that we scan any extra memory from info->client_info.info.stack_end to stack_mark MonoThreadUnwindState *state = &info->client_info.info.thread_saved_state [SELF_SUSPEND_STATE_INDEX]; if (state && state->gc_stackdata) { sgen_conservatively_pin_objects_from ((void **)state->gc_stackdata, (void**)((char*)state->gc_stackdata + state->gc_stackdata_size), @@ -2501,8 +2471,8 @@ mono_gc_set_stack_end (void *stack_end) LOCK_GC; info = mono_thread_info_current (); if (info) { - SGEN_ASSERT (0, stack_end < info->client_info.stack_end, "Can only lower stack end"); - info->client_info.stack_end = stack_end; + SGEN_ASSERT (0, stack_end < info->client_info.info.stack_end, "Can only lower stack end"); + info->client_info.info.stack_end = stack_end; } UNLOCK_GC; } @@ -2887,13 +2857,11 @@ sgen_client_vtable_get_name (MonoVTable *vt) void sgen_client_init (void) { - int dummy; MonoThreadInfoCallbacks cb; - cb.thread_register = sgen_thread_register; - cb.thread_detach = sgen_thread_detach; - cb.thread_unregister = sgen_thread_unregister; cb.thread_attach = sgen_thread_attach; + cb.thread_detach = sgen_thread_detach; + cb.thread_detach_with_lock = sgen_thread_detach_with_lock; cb.mono_thread_in_critical_region = thread_in_critical_region; cb.ip_in_critical_region = ip_in_critical_region; @@ -2909,7 +2877,7 @@ sgen_client_init (void) mono_tls_init_gc_keys (); - mono_gc_register_thread (&dummy); + mono_thread_info_attach (); } gboolean diff --git a/mono/metadata/sgen-stw.c b/mono/metadata/sgen-stw.c index 3bd1b332286..be999592a6a 100644 --- a/mono/metadata/sgen-stw.c +++ b/mono/metadata/sgen-stw.c @@ -66,7 +66,7 @@ update_current_thread_stack (void *start) info->client_info.stack_start = align_pointer (&stack_guard); g_assert (info->client_info.stack_start); - g_assert (info->client_info.stack_start >= info->client_info.stack_start_limit && info->client_info.stack_start < info->client_info.stack_end); + g_assert (info->client_info.stack_start >= info->client_info.info.stack_start_limit && info->client_info.stack_start < info->client_info.info.stack_end); #if !defined(MONO_CROSS_COMPILE) && MONO_ARCH_HAS_MONO_CONTEXT MONO_CONTEXT_GET_CURRENT (info->client_info.ctx); @@ -363,8 +363,8 @@ sgen_unified_suspend_stop_world (void) /* Once we remove the old suspend code, we should move sgen to directly access the state in MonoThread */ info->client_info.stack_start = (gpointer) ((char*)MONO_CONTEXT_GET_SP (&info->client_info.ctx) - REDZONE_SIZE); - if (info->client_info.stack_start < info->client_info.stack_start_limit - || info->client_info.stack_start >= info->client_info.stack_end) { + if (info->client_info.stack_start < info->client_info.info.stack_start_limit + || info->client_info.stack_start >= info->client_info.info.stack_end) { /* * Thread context is in unhandled state, most likely because it is * dying. We don't scan it. @@ -378,7 +378,7 @@ sgen_unified_suspend_stop_world (void) binary_protocol_thread_suspend ((gpointer) mono_thread_info_get_tid (info), stopped_ip); THREADS_STW_DEBUG ("[GC-STW-SUSPEND-END] thread %p is suspended, stopped_ip = %p, stack = %p -> %p\n", - mono_thread_info_get_tid (info), stopped_ip, info->client_info.stack_start, info->client_info.stack_start ? info->client_info.stack_end : NULL); + mono_thread_info_get_tid (info), stopped_ip, info->client_info.stack_start, info->client_info.stack_start ? info->client_info.info.stack_end : NULL); } FOREACH_THREAD_END } diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index 6f1a34e2dfc..60848191ddc 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -884,11 +884,11 @@ start_wrapper (gpointer data) start_info = (StartInfo*) data; g_assert (start_info); - info = mono_thread_info_attach (&res); + info = mono_thread_info_attach (); info->runtime_thread = TRUE; /* Run the actual main function of the thread */ - res = start_wrapper_internal (start_info, &res); + res = start_wrapper_internal (start_info, info->stack_end); mono_thread_info_exit (res); @@ -1095,7 +1095,6 @@ mono_thread_attach_full (MonoDomain *domain, gboolean force_attach) MonoThread *thread; MonoThreadInfo *info; MonoNativeThreadId tid; - gsize stack_ptr; if (mono_thread_internal_current_is_attached ()) { if (domain != mono_domain_get ()) @@ -1104,7 +1103,7 @@ mono_thread_attach_full (MonoDomain *domain, gboolean force_attach) return mono_thread_current (); } - info = mono_thread_info_attach (&stack_ptr); + info = mono_thread_info_attach (); g_assert (info); tid=mono_native_thread_id_get (); @@ -1121,17 +1120,8 @@ mono_thread_attach_full (MonoDomain *domain, gboolean force_attach) THREAD_DEBUG (g_message ("%s: Attached thread ID %"G_GSIZE_FORMAT" (handle %p)", __func__, tid, internal->handle)); - if (mono_thread_attach_cb) { - guint8 *staddr; - size_t stsize; - - mono_thread_info_get_stack_bounds (&staddr, &stsize); - - if (staddr == NULL) - mono_thread_attach_cb (MONO_NATIVE_THREAD_ID_TO_UINT (tid), &stack_ptr); - else - mono_thread_attach_cb (MONO_NATIVE_THREAD_ID_TO_UINT (tid), staddr + stsize); - } + if (mono_thread_attach_cb) + mono_thread_attach_cb (MONO_NATIVE_THREAD_ID_TO_UINT (tid), info->stack_end); /* Can happen when we attach the profiler helper thread in order to heapshot. */ if (!mono_thread_info_current ()->tools_thread) diff --git a/mono/sgen/gc-internal-agnostic.h b/mono/sgen/gc-internal-agnostic.h index 54953e85d4c..0367d145774 100644 --- a/mono/sgen/gc-internal-agnostic.h +++ b/mono/sgen/gc-internal-agnostic.h @@ -80,12 +80,6 @@ typedef void* MonoGCDescriptor; #define MONO_GC_DESCRIPTOR_NULL NULL #endif -/* - * Try to register a foreign thread with the GC, if we fail or the backend - * can't cope with this concept - we return FALSE. - */ -extern gboolean mono_gc_register_thread (void *baseptr); - gboolean mono_gc_parse_environment_string_extract_number (const char *str, size_t *out); MonoGCDescriptor mono_gc_make_descr_for_object (gsize *bitmap, int numbits, size_t obj_size); diff --git a/mono/sgen/sgen-client.h b/mono/sgen/sgen-client.h index 11e6451d5b6..a492ad74a97 100644 --- a/mono/sgen/sgen-client.h +++ b/mono/sgen/sgen-client.h @@ -153,11 +153,11 @@ void sgen_client_pre_collection_checks (void); * Must set the thread's thread info to `info`. If the thread's small ID was not already * initialized in `sgen_client_init()` (for the main thread, usually), it must be done here. * - * `stack_bottom_fallback` is the value passed through via `sgen_thread_register()`. + * `stack_bottom_fallback` is the value passed through via `sgen_thread_attach()`. */ -void sgen_client_thread_register (SgenThreadInfo* info, void *stack_bottom_fallback); +void sgen_client_thread_attach (SgenThreadInfo* info); -void sgen_client_thread_unregister (SgenThreadInfo *p); +void sgen_client_thread_detach_with_lock (SgenThreadInfo *p); /* * Called on each worker thread when it starts up. Must initialize the thread's small ID. diff --git a/mono/sgen/sgen-debug.c b/mono/sgen/sgen-debug.c index b4da324df1f..904460bdfdd 100644 --- a/mono/sgen/sgen-debug.c +++ b/mono/sgen/sgen-debug.c @@ -502,9 +502,9 @@ find_pinning_ref_from_thread (char *obj, size_t size) char **start = (char**)info->client_info.stack_start; if (info->client_info.skip || info->client_info.gc_disabled) continue; - while (start < (char**)info->client_info.stack_end) { + while (start < (char**)info->client_info.info.stack_end) { if (*start >= obj && *start < endobj) - SGEN_LOG (0, "Object %p referenced in thread %p (id %p) at %p, stack: %p-%p", obj, info, (gpointer)mono_thread_info_get_tid (info), start, info->client_info.stack_start, info->client_info.stack_end); + SGEN_LOG (0, "Object %p referenced in thread %p (id %p) at %p, stack: %p-%p", obj, info, (gpointer)mono_thread_info_get_tid (info), start, info->client_info.stack_start, info->client_info.info.stack_end); start++; } diff --git a/mono/sgen/sgen-gc.c b/mono/sgen/sgen-gc.c index 6cd939553a9..2f761cc8a13 100644 --- a/mono/sgen/sgen-gc.c +++ b/mono/sgen/sgen-gc.c @@ -2831,19 +2831,19 @@ sgen_get_current_collection_generation (void) } void* -sgen_thread_register (SgenThreadInfo* info, void *stack_bottom_fallback) +sgen_thread_attach (SgenThreadInfo* info) { info->tlab_start = info->tlab_next = info->tlab_temp_end = info->tlab_real_end = NULL; - sgen_client_thread_register (info, stack_bottom_fallback); + sgen_client_thread_attach (info); return info; } void -sgen_thread_unregister (SgenThreadInfo *p) +sgen_thread_detach_with_lock (SgenThreadInfo *p) { - sgen_client_thread_unregister (p); + sgen_client_thread_detach_with_lock (p); } /* diff --git a/mono/sgen/sgen-gc.h b/mono/sgen/sgen-gc.h index e3fe2ed2267..6967f29ea7c 100644 --- a/mono/sgen/sgen-gc.h +++ b/mono/sgen/sgen-gc.h @@ -918,8 +918,8 @@ GCObject* sgen_try_alloc_obj_nolock (GCVTable vtable, size_t size); /* Threads */ -void* sgen_thread_register (SgenThreadInfo* info, void *addr); -void sgen_thread_unregister (SgenThreadInfo *p); +void* sgen_thread_attach (SgenThreadInfo* info); +void sgen_thread_detach_with_lock (SgenThreadInfo *p); /* Finalization/ephemeron support */ diff --git a/mono/unit-tests/test-conc-hashtable.c b/mono/unit-tests/test-conc-hashtable.c index 712e2efa381..f01f01b574a 100644 --- a/mono/unit-tests/test-conc-hashtable.c +++ b/mono/unit-tests/test-conc-hashtable.c @@ -69,7 +69,7 @@ static void* pw_sr_thread (void *arg) { int i, idx = 1000 * GPOINTER_TO_INT (arg); - mono_thread_info_attach ((gpointer)&arg); + mono_thread_info_attach (); for (i = 0; i < 1000; ++i) { mono_os_mutex_lock (&global_mutex); @@ -118,7 +118,7 @@ static void* pr_sw_thread (void *arg) { int i = 0, idx = 100 * GPOINTER_TO_INT (arg); - mono_thread_info_attach ((gpointer)&arg); + mono_thread_info_attach (); while (i < 100) { gpointer res = mono_conc_hashtable_lookup (hash, GINT_TO_POINTER (i + idx + 1)); @@ -178,7 +178,7 @@ static void* pw_pr_r_thread (void *arg) { int key, val, i; - mono_thread_info_attach ((gpointer)&arg); + mono_thread_info_attach (); /* i will not be incremented as long as running is set to 1, this guarantee that we loop over all the keys at least once after the writer threads have finished */ @@ -200,7 +200,7 @@ pw_pr_w_add_thread (void *arg) { int i, idx = 1000 * GPOINTER_TO_INT (arg); - mono_thread_info_attach ((gpointer)&arg); + mono_thread_info_attach (); for (i = idx; i < idx + 1000; i++) { mono_os_mutex_lock (&global_mutex); @@ -215,7 +215,7 @@ pw_pr_w_del_thread (void *arg) { int i, idx = 1000 * GPOINTER_TO_INT (arg); - mono_thread_info_attach ((gpointer)&arg); + mono_thread_info_attach (); for (i = idx; i < idx + 1000; i++) { mono_os_mutex_lock (&global_mutex); @@ -340,7 +340,7 @@ main (void) mono_w32handle_init (); #endif - mono_thread_info_attach ((gpointer)&cb); + mono_thread_info_attach (); // benchmark_conc (); // benchmark_glib (); diff --git a/mono/utils/mono-threads.c b/mono/utils/mono-threads.c index 6afb74daf8d..0f85ba1ee1d 100644 --- a/mono/utils/mono-threads.c +++ b/mono/utils/mono-threads.c @@ -353,15 +353,15 @@ thread_handle_destroy (gpointer data) g_free (thread_handle); } -static void* -register_thread (MonoThreadInfo *info, gpointer baseptr) +static gboolean +register_thread (MonoThreadInfo *info) { size_t stsize = 0; guint8 *staddr = NULL; - int small_id = mono_thread_info_register_small_id (); gboolean result; + + info->small_id = mono_thread_info_register_small_id (); mono_thread_info_set_tid (info, mono_native_thread_id_get ()); - info->small_id = small_id; info->handle = g_new0 (MonoThreadHandle, 1); mono_refcount_init (info->handle, thread_handle_destroy); @@ -372,17 +372,6 @@ register_thread (MonoThreadInfo *info, gpointer baseptr) /*set TLS early so SMR works */ mono_native_tls_set_value (thread_info_key, info); - THREADS_DEBUG ("registering info %p tid %p small id %x\n", info, mono_thread_info_get_tid (info), info->small_id); - - if (threads_callbacks.thread_register) { - if (threads_callbacks.thread_register (info, baseptr) == NULL) { - // g_warning ("thread registation failed\n"); - mono_native_tls_set_value (thread_info_key, NULL); - g_free (info); - return NULL; - } - } - mono_thread_info_get_stack_bounds (&staddr, &stsize); g_assert (staddr); g_assert (stsize); @@ -393,6 +382,16 @@ register_thread (MonoThreadInfo *info, gpointer baseptr) mono_threads_suspend_register (info); + THREADS_DEBUG ("registering info %p tid %p small id %x\n", info, mono_thread_info_get_tid (info), info->small_id); + + if (threads_callbacks.thread_attach) { + if (!threads_callbacks.thread_attach (info)) { + // g_warning ("thread registation failed\n"); + mono_native_tls_set_value (thread_info_key, NULL); + return FALSE; + } + } + /* Transition it before taking any locks or publishing itself to reduce the chance of others witnessing a detached thread. @@ -405,7 +404,8 @@ register_thread (MonoThreadInfo *info, gpointer baseptr) result = mono_thread_info_insert (info); g_assert (result); mono_thread_info_suspend_unlock (); - return info; + + return TRUE; } static void @@ -469,8 +469,8 @@ unregister_thread (void *arg) be done while holding the suspend lock to give no other thread chance to suspend it. */ - if (threads_callbacks.thread_unregister) - threads_callbacks.thread_unregister (info); + if (threads_callbacks.thread_detach_with_lock) + threads_callbacks.thread_detach_with_lock (info); /* The thread is no longer active, so unref its handle */ mono_threads_close_thread_handle (info->handle); @@ -492,6 +492,8 @@ unregister_thread (void *arg) mono_threads_signal_thread_handle (handle); mono_threads_close_thread_handle (handle); + + mono_native_tls_set_value (thread_info_key, NULL); } static void @@ -583,7 +585,6 @@ mono_thread_info_list_head (void) void mono_threads_attach_tools_thread (void) { - int dummy = 0; MonoThreadInfo *info; /* Must only be called once */ @@ -593,36 +594,39 @@ mono_threads_attach_tools_thread (void) mono_thread_info_usleep (10); } - info = mono_thread_info_attach (&dummy); + info = mono_thread_info_attach (); g_assert (info); info->tools_thread = TRUE; } MonoThreadInfo* -mono_thread_info_attach (void *baseptr) +mono_thread_info_attach (void) { MonoThreadInfo *info; + +#ifdef HOST_WIN32 if (!mono_threads_inited) { -#ifdef HOST_WIN32 /* This can happen from DllMain(DLL_THREAD_ATTACH) on Windows, if a * thread is created before an embedding API user initialized Mono. */ THREADS_DEBUG ("mono_thread_info_attach called before mono_threads_init\n"); return NULL; -#else - g_assert (mono_threads_inited); -#endif } +#endif + + g_assert (mono_threads_inited); + info = (MonoThreadInfo *) mono_native_tls_get_value (thread_info_key); if (!info) { info = (MonoThreadInfo *) g_malloc0 (thread_info_size); THREADS_DEBUG ("attaching %p\n", info); - if (!register_thread (info, baseptr)) + if (!register_thread (info)) { + g_free (info); return NULL; - } else if (threads_callbacks.thread_attach) { - threads_callbacks.thread_attach (info); + } } + return info; } @@ -630,6 +634,8 @@ void mono_thread_info_detach (void) { MonoThreadInfo *info; + +#ifdef HOST_WIN32 if (!mono_threads_inited) { /* This can happen from DllMain(THREAD_DETACH) on Windows, if a thread @@ -637,11 +643,14 @@ mono_thread_info_detach (void) THREADS_DEBUG ("mono_thread_info_detach called before mono_threads_init\n"); return; } +#endif + + g_assert (mono_threads_inited); + info = (MonoThreadInfo *) mono_native_tls_get_value (thread_info_key); if (info) { THREADS_DEBUG ("detaching %p\n", info); unregister_thread (info); - mono_native_tls_set_value (thread_info_key, NULL); } } diff --git a/mono/utils/mono-threads.h b/mono/utils/mono-threads.h index 63cad57d868..95dd0368834 100644 --- a/mono/utils/mono-threads.h +++ b/mono/utils/mono-threads.h @@ -222,22 +222,21 @@ typedef struct { } MonoThreadInfo; typedef struct { - void* (*thread_register)(THREAD_INFO_TYPE *info, void *baseaddr); + void* (*thread_attach)(THREAD_INFO_TYPE *info); /* - This callback is called with @info still on the thread list. - This call is made while holding the suspend lock, so don't do callbacks. - SMR remains functional as its small_id has not been reclaimed. - */ - void (*thread_unregister)(THREAD_INFO_TYPE *info); - /* - This callback is called right before thread_unregister. This is called + This callback is called right before thread_detach_with_lock. This is called without any locks held so it's the place for complicated cleanup. - The thread must remain operational between this call and thread_unregister. - It must be possible to successfully suspend it after thread_unregister completes. + The thread must remain operational between this call and thread_detach_with_lock. + It must be possible to successfully suspend it after thread_detach completes. */ void (*thread_detach)(THREAD_INFO_TYPE *info); - void (*thread_attach)(THREAD_INFO_TYPE *info); + /* + This callback is called with @info still on the thread list. + This call is made while holding the suspend lock, so don't do callbacks. + SMR remains functional as its small_id has not been reclaimed. + */ + void (*thread_detach_with_lock)(THREAD_INFO_TYPE *info); gboolean (*mono_method_is_critical) (void *method); gboolean (*ip_in_critical_region) (MonoDomain *domain, gpointer ip); gboolean (*mono_thread_in_critical_region) (THREAD_INFO_TYPE *info); @@ -314,7 +313,7 @@ int mono_thread_info_register_small_id (void); THREAD_INFO_TYPE * -mono_thread_info_attach (void *baseptr); +mono_thread_info_attach (void); MONO_API void mono_thread_info_detach (void); -- 2.25.1