Add mono_threads_reset_blocking nesting checks to gc checked mode
authorAndi McClure <andi.mcclure@xamarin.com>
Fri, 18 Mar 2016 19:18:38 +0000 (15:18 -0400)
committerAndi McClure <andi.mcclure@xamarin.com>
Fri, 18 Mar 2016 19:18:38 +0000 (15:18 -0400)
mono/utils/mono-threads-coop.c

index 3b11148aafe3af5d8c7ff2d93368bb3033da1e10..2a31e93e6f141762b0000b4fcaae86fbce9b6f9f 100644 (file)
@@ -25,6 +25,7 @@
 #include <mono/utils/mono-counters.h>
 #include <mono/utils/mono-threads-coop.h>
 #include <mono/utils/mono-threads-api.h>
+#include <mono/utils/checked-build.h>
 
 #ifdef TARGET_OSX
 #include <mono/utils/mach-support.h>
 
 volatile size_t mono_polling_required;
 
+// FIXME: This would be more efficient if instead of instantiating the stack it just pushed a simple depth counter up and down,
+// perhaps with a per-thread cookie in the high bits.
+#ifdef ENABLE_CHECKED_BUILD_GC
+// Maintains a single per-thread stack of ints, used to ensure nesting is not violated
+MonoNativeTlsKey coop_reset_count_stack_key;
+static int coop_tls_push (int v) {
+       GArray *stack = mono_native_tls_get_value (coop_reset_count_stack_key);
+       if (!stack) {
+               stack = g_array_new (FALSE,FALSE,sizeof(int));
+               mono_native_tls_set_value (coop_reset_count_stack_key, stack);
+       }
+       g_array_append_val (stack, v);
+       return stack->len;
+}
+static int coop_tls_pop (int *v) {
+       GArray *stack = mono_native_tls_get_value (coop_reset_count_stack_key);
+       if (!stack || 0 == stack->len)
+               return -1;
+       stack->len--;
+       *v = g_array_index (stack, int, stack->len);
+       int len = stack->len;
+       if (0 == len) {
+               g_array_free (stack,TRUE);
+               mono_native_tls_set_value (coop_reset_count_stack_key, NULL);
+       }
+       return len;
+}
+#endif
+
 static int coop_reset_blocking_count;
 static int coop_try_blocking_count;
 static int coop_do_blocking_count;
@@ -193,9 +223,18 @@ mono_threads_reset_blocking_start (void* stackdata)
        if (!mono_threads_is_coop_enabled ())
                return NULL;
 
+       info = mono_thread_info_current_unchecked ();
+
+#ifdef ENABLE_CHECKED_BUILD_GC
+       int reset_blocking_count = InterlockedIncrement (&coop_reset_blocking_count);
+       // In this mode, the blocking count is used as the reset cookie. We would prefer
+       // (but do not require) this to be unique across invocations and threads.
+       if (reset_blocking_count == 0) // We *do* require it be nonzero
+               reset_blocking_count = coop_reset_blocking_count = 1;
+#else
        ++coop_reset_blocking_count;
+#endif
 
-       info = mono_thread_info_current_unchecked ();
        /* If the thread is not attached, it doesn't make sense prepare for suspend. */
        if (!info || !mono_thread_info_is_live (info))
                return NULL;
@@ -211,28 +250,50 @@ mono_threads_reset_blocking_start (void* stackdata)
                return NULL;
        case AbortBlockingOk:
                info->thread_saved_state [SELF_SUSPEND_STATE_INDEX].valid = FALSE;
-               return info;
+               break;
        case AbortBlockingOkAndPool:
                mono_threads_state_poll ();
-               return info;
+               break;
        default:
                g_error ("Unknown thread state");
        }
+
+#ifdef ENABLE_CHECKED_BUILD_GC
+       if (mono_check_mode_enabled (MONO_CHECK_MODE_GC)) {
+               int level = coop_tls_push (reset_blocking_count);
+               //g_warning("Entering reset nest; level %d; cookie %d\n", level, reset_blocking_count);
+               return (void *)(intptr_t)reset_blocking_count;
+       }
+#endif
+
+       return info;
 }
 
 void
 mono_threads_reset_blocking_end (void *cookie, void* stackdata)
 {
-       MonoThreadInfo *info;
-
        if (!mono_threads_is_coop_enabled ())
                return;
 
-       info = (MonoThreadInfo *)cookie;
-       if (!info)
+       if (!cookie)
                return;
 
-       g_assert (info == mono_thread_info_current_unchecked ());
+#ifdef ENABLE_CHECKED_BUILD_GC
+       if (mono_check_mode_enabled (MONO_CHECK_MODE_GC)) {
+               int received_cookie = (int)(intptr_t)cookie;
+               int desired_cookie;
+               int level = coop_tls_pop (&desired_cookie);
+               //g_warning("Leaving reset nest; back to level %d; desired cookie %d; received cookie %d\n", level, desired_cookie, received_cookie);
+               if (level < 0)
+                       g_error_with_history ("Expected cookie %d but found no stack at all\n", desired_cookie);
+               if (desired_cookie != received_cookie)
+                       g_error_with_history ("Expected cookie %d but received %d\n", desired_cookie, received_cookie);
+       } else // Notice this matches the line after the endif
+#endif
+       {
+               g_assert (((MonoThreadInfo *)cookie) == mono_thread_info_current_unchecked ());
+       }
+
        mono_threads_prepare_blocking (stackdata);
 }
 
@@ -248,6 +309,10 @@ mono_threads_init_coop (void)
        mono_counters_register ("Coop Do Polling", MONO_COUNTER_GC | MONO_COUNTER_INT, &coop_do_polling_count);
        mono_counters_register ("Coop Save Count", MONO_COUNTER_GC | MONO_COUNTER_INT, &coop_save_count);
        //See the above for what's wrong here.
+
+#ifdef ENABLE_CHECKED_BUILD_GC
+       mono_native_tls_alloc (&coop_reset_count_stack_key, NULL);
+#endif
 }
 
 void