From e37a4cc883593d33dfb58eda3179656c07e23d65 Mon Sep 17 00:00:00 2001 From: Zoltan Varga Date: Sat, 17 Oct 2015 01:37:47 -0400 Subject: [PATCH] Make the thread dump code safe by doing only async safe operations when a thread is suspended. --- mono/metadata/threads.c | 131 ++++++++++++++++++++++++---------------- 1 file changed, 78 insertions(+), 53 deletions(-) diff --git a/mono/metadata/threads.c b/mono/metadata/threads.c index c676d8eaa58..8fc787d346a 100644 --- a/mono/metadata/threads.c +++ b/mono/metadata/threads.c @@ -3140,55 +3140,102 @@ void mono_thread_suspend_all_other_threads (void) } } +typedef struct { + MonoInternalThread *thread; + MonoStackFrameInfo *frames; + int nframes, max_frames; +} ThreadDumpUserData; + static gboolean thread_dump_requested; -static G_GNUC_UNUSED gboolean -print_stack_frame_to_string (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer data) +/* This needs to be async safe */ +static gboolean +collect_frame (MonoStackFrameInfo *frame, MonoContext *ctx, gpointer data) { - GString *p = (GString*)data; - MonoMethod *method = NULL; - if (frame->type == FRAME_TYPE_MANAGED) - method = mono_jit_info_get_method (frame->ji); + ThreadDumpUserData *ud = data; - if (method) { - gchar *location = mono_debug_print_stack_frame (method, frame->native_offset, frame->domain); - g_string_append_printf (p, " %s\n", location); - g_free (location); - } else - g_string_append_printf (p, " at <0x%05x>\n", frame->native_offset); + if (ud->nframes < ud->max_frames) { + memcpy (&ud->frames [ud->nframes], frame, sizeof (MonoStackFrameInfo)); + ud->nframes ++; + } return FALSE; } +/* This needs to be async safe */ static SuspendThreadResult -print_thread_dump (MonoThreadInfo *info, gpointer ud) +get_thread_dump (MonoThreadInfo *info, gpointer ud) +{ + ThreadDumpUserData *user_data = ud; + MonoInternalThread *thread = user_data->thread; + +#if 0 +/* This no longer works with remote unwinding */ +#ifndef HOST_WIN32 + wapi_desc = wapi_current_thread_desc (); + g_string_append_printf (text, " tid=0x%p this=0x%p %s\n", (gpointer)(gsize)thread->tid, thread, wapi_desc); + free (wapi_desc); +#endif +#endif + + if (thread == mono_thread_internal_current ()) + mono_get_eh_callbacks ()->mono_walk_stack_with_ctx (collect_frame, NULL, MONO_UNWIND_SIGNAL_SAFE, ud); + else + mono_get_eh_callbacks ()->mono_walk_stack_with_state (collect_frame, mono_thread_info_get_suspend_state (info), MONO_UNWIND_SIGNAL_SAFE, ud); + + return MonoResumeThread; +} + +static void +dump_thread (gpointer key, gpointer value, gpointer user) { - MonoInternalThread *thread = ud; + ThreadDumpUserData *ud = user; + MonoInternalThread *thread = value; GString* text = g_string_new (0); char *name; GError *error = NULL; + int i; + + ud->thread = thread; + ud->nframes = 0; + + /* Collect frames for the thread */ + if (thread == mono_thread_internal_current ()) { + get_thread_dump (mono_thread_info_current (), ud); + } else { + mono_thread_info_safe_suspend_and_run (thread_get_tid (thread), FALSE, get_thread_dump, ud); + } + /* + * Do all the non async-safe work outside of get_thread_dump. + */ if (thread->name) { name = g_utf16_to_utf8 (thread->name, thread->name_len, NULL, NULL, &error); g_assert (!error); g_string_append_printf (text, "\n\"%s\"", name); g_free (name); } - else if (thread->threadpool_thread) + else if (thread->threadpool_thread) { g_string_append (text, "\n\"\""); - else + } else { g_string_append (text, "\n\"\""); + } -#if 0 -/* This no longer works with remote unwinding */ -#ifndef HOST_WIN32 - wapi_desc = wapi_current_thread_desc (); - g_string_append_printf (text, " tid=0x%p this=0x%p %s\n", (gpointer)(gsize)thread->tid, thread, wapi_desc); - free (wapi_desc); -#endif -#endif + for (i = 0; i < ud->nframes; ++i) { + MonoStackFrameInfo *frame = &ud->frames [i]; + MonoMethod *method = NULL; - mono_get_eh_callbacks ()->mono_walk_stack_with_state (print_stack_frame_to_string, mono_thread_info_get_suspend_state (info), MONO_UNWIND_SIGNAL_SAFE, text); + if (frame->type == FRAME_TYPE_MANAGED) + method = mono_jit_info_get_method (frame->ji); + + if (method) { + gchar *location = mono_debug_print_stack_frame (method, frame->native_offset, frame->domain); + g_string_append_printf (text, " %s\n", location); + g_free (location); + } else { + g_string_append_printf (text, " at <0x%05x>\n", frame->native_offset); + } + } fprintf (stdout, "%s", text->str); @@ -3198,48 +3245,26 @@ print_thread_dump (MonoThreadInfo *info, gpointer ud) g_string_free (text, TRUE); fflush (stdout); - return MonoResumeThread; -} - -static void -dump_thread (gpointer key, gpointer value, gpointer user) -{ - MonoInternalThread *thread = (MonoInternalThread *)value; - - if (thread == mono_thread_internal_current ()) - return; - - /* - FIXME This still can hang if we stop a thread during malloc. - FIXME This can hang if we suspend on a critical method and the GC kicks in. A fix might be to have function - that takes a callback and runs it with the target suspended. - We probably should loop a bit around trying to get it to either managed code - or WSJ state. - */ - mono_thread_info_safe_suspend_and_run (thread_get_tid (thread), FALSE, print_thread_dump, thread); } void mono_threads_perform_thread_dump (void) { + ThreadDumpUserData ud; + if (!thread_dump_requested) return; printf ("Full thread dump:\n"); - /* We take the loader lock and the root domain lock as to increase our odds of not deadlocking if - something needs then in the process. - */ - mono_loader_lock (); - mono_domain_lock (mono_get_root_domain ()); + memset (&ud, 0, sizeof (ud)); + ud.frames = g_new0 (MonoStackFrameInfo, 256); + ud.max_frames = 256; mono_threads_lock (); - mono_g_hash_table_foreach (threads, dump_thread, NULL); + mono_g_hash_table_foreach (threads, dump_thread, &ud); mono_threads_unlock (); - mono_domain_unlock (mono_get_root_domain ()); - mono_loader_unlock (); - thread_dump_requested = FALSE; } -- 2.25.1