[coop] Checked build GC critical regions.

For local handles in coop suspend configuartions we need the idea of a
"critical region": a situation when transitioning from GC
Unsafe (running) to GC Safe (blocking) mode constitutes a bug. In
checked builds, we'd like to assert when that happens.

This PR adds MONO_PREPARE_GC_CRITICAL_REGION,
MONO_FINISH_GC_CRITICAL_REGION and MONO_REQ_GC_NOT_CRITICAL.

In regular build, these are nops. In checked build, they use a new
thread-local flag to prevent transitions from running to blocking.

diff --git a/mono/metadata/handle.h b/mono/metadata/handle.h
index 7f8039786d6f41fcef32ccc6117e7d8727d227f9..bf94296791fc6a9b06c82fa25870344919569b70 100644 (file)
--- a/mono/metadata/handle.h
+++ b/mono/metadata/handle.h
@@ -45,16 +45,14 @@ struct _MonoHandleStorage {
 #else
 
 static inline void
-mono_handle_check_in_critical_section (const gchar* file, const gint lineno)
+mono_handle_check_in_critical_section ()
 {
-       MonoThreadInfo *info = (MonoThreadInfo*) mono_thread_info_current_unchecked ();
-       if (info && !info->inside_critical_region)
-               g_error ("Assertion at %s:%d: mono_handle_check_in_critical_section failed", file, lineno);
+       MONO_REQ_GC_UNSAFE_MODE;
 }
 
-#define mono_handle_obj(handle) (mono_handle_check_in_critical_section (__FILE__, __LINE__), (handle)->obj)
+#define mono_handle_obj(handle) (mono_handle_check_in_critical_section (), (handle)->obj)
 
-#define mono_handle_assign(handle,rawptr) do { mono_handle_check_in_critical_section (__FILE__, __LINE__); (handle)->obj = (rawptr); } while (0)
+#define mono_handle_assign(handle,rawptr) do { mono_handle_check_in_critical_section (); (handle)->obj = (rawptr); } while (0)
 
 #endif
 
@@ -84,31 +82,31 @@ mono_handle_domain (MonoHandle handle)
 #define MONO_HANDLE_SETREF(handle,fieldname,value)                     \
        do {                                                            \
                MonoHandle __value = (MonoHandle) (value);              \
-               MONO_PREPARE_CRITICAL;                                  \
+               MONO_PREPARE_GC_CRITICAL_REGION;                                        \
                MONO_OBJECT_SETREF (mono_handle_obj ((handle)), fieldname, mono_handle_obj (__value)); \
-               MONO_FINISH_CRITICAL;                                   \
+               MONO_FINISH_GC_CRITICAL_REGION;                                 \
        } while (0)
 
 #define MONO_HANDLE_SET(handle,fieldname,value)        \
        do {    \
-               MONO_PREPARE_CRITICAL;  \
+               MONO_PREPARE_GC_CRITICAL_REGION;        \
                mono_handle_obj ((handle))->fieldname = (value);        \
-               MONO_FINISH_CRITICAL;   \
+               MONO_FINISH_GC_CRITICAL_REGION; \
        } while (0)
 
 #define MONO_HANDLE_ARRAY_SETREF(handle,index,value)                   \
        do {                                                            \
                MonoHandle __value = (MonoHandle) (value);              \
-               MONO_PREPARE_CRITICAL;                                  \
+               MONO_PREPARE_GC_CRITICAL_REGION;                                        \
                mono_array_setref (mono_handle_obj ((handle)), (index), mono_handle_obj (__value)); \
-               MONO_FINISH_CRITICAL;                                   \
+               MONO_FINISH_GC_CRITICAL_REGION;                                 \
        } while (0)
 
 #define MONO_HANDLE_ARRAY_SET(handle,type,index,value) \
        do {    \
-               MONO_PREPARE_CRITICAL;  \
+               MONO_PREPARE_GC_CRITICAL_REGION;        \
                mono_array_set (mono_handle_obj ((handle)), (type), (index), (value));  \
-               MONO_FINISH_CRITICAL;   \
+               MONO_FINISH_GC_CRITICAL_REGION; \
        } while (0)
 
 /* handle arena specific functions */

diff --git a/mono/utils/checked-build.c b/mono/utils/checked-build.c
index 9690d9d030d76309fc92a8e3c50ae6b94513e4f0..5c4fd4249e9acd79cd6635d5a0bd82504be451f2 100644 (file)
--- a/mono/utils/checked-build.c
+++ b/mono/utils/checked-build.c
@@ -81,6 +81,7 @@ translate_backtrace (gpointer native_trace[], int size)
 
 typedef struct {
        GPtrArray *transitions;
+       gboolean in_gc_critical_region;
 } CheckState;
 
 typedef struct {
@@ -230,6 +231,34 @@ assert_gc_neutral_mode (void)
        }
 }
 
+void *
+critical_gc_region_begin(void)
+{
+       CheckState *state = get_state ();
+       state->in_gc_critical_region = TRUE;
+       return state;
+}
+
+
+void
+critical_gc_region_end(void* token)
+{
+       CheckState *state = get_state();
+       g_assert (state == token);
+       state->in_gc_critical_region = FALSE;
+}
+
+void
+assert_not_in_gc_critical_region(void)
+{
+       CheckState *state = get_state();
+       if (state->in_gc_critical_region) {
+               MonoThreadInfo *cur = mono_thread_info_current();
+               state = mono_thread_info_current_state(cur);
+               assertion_fail("Expected GC Unsafe mode, but was in %s state", mono_thread_state_name(state));
+       }
+}
+
 // check_metadata_store et al: The goal of these functions is to verify that if there is a pointer from one mempool into
 // another, that the pointed-to memory is protected by the reference count mechanism for MonoImages.
 //

diff --git a/mono/utils/checked-build.h b/mono/utils/checked-build.h
index 8fac45b018fa0a00687ef1c11ba331b92d6b6c0f..83efd67c054899a2865326e8d65fb9a0b35e34d4 100644 (file)
--- a/mono/utils/checked-build.h
+++ b/mono/utils/checked-build.h
@@ -78,6 +78,24 @@ Functions that can be called from both coop or preept modes.
        assert_gc_neutral_mode ();      \
 } while (0);
 
+/* In a GC critical region, the thread is not allowed to switch to GC safe mode.
+ * For example if the thread is about to call a method that will manipulate managed objects.
+ * The GC critical region must only occur in unsafe mode.
+ */
+#define MONO_PREPARE_GC_CRITICAL_REGION                                        \
+       MON_REQ_GC_UNSAFE_MODE                                          \
+       do {                                                            \
+               void* __critical_gc_region_cookie = critical_gc_region_begin()
+
+#define MONO_FINISH_GC_CRITICAL_REGION                 \
+       critical_gc_region_end(__critical_gc_region_cookie);    \
+       } while(0)
+
+/* Verify that the thread is not currently in a GC critical region. */
+#define MONO_REQ_GC_NOT_CRITICAL do {                  \
+               assert_not_in_gc_critical_region();     \
+       } while(0)
+
 // Use when writing a pointer from one image or imageset to another.
 #define CHECKED_METADATA_WRITE_PTR(ptr, val) do {    \
     check_metadata_store (&(ptr), (val));    \
@@ -116,6 +134,10 @@ void assert_gc_safe_mode (void);
 void assert_gc_unsafe_mode (void);
 void assert_gc_neutral_mode (void);
 
+void* critical_gc_region_begin(void);
+void critical_gc_region_end(void* token);
+void assert_not_in_gc_critical_region(void);
+
 void checked_build_init (void);
 void checked_build_thread_transition(const char *transition, void *info, int from_state, int suspend_count, int next_state, int suspend_count_delta);
 
@@ -132,6 +154,12 @@ void check_metadata_store_local(void *from, void *to);
 #define MONO_REQ_API_ENTRYPOINT
 #define MONO_REQ_RUNTIME_ENTRYPOINT
 
+#define MONO_PREPARE_GC_CRITICAL_REGION
+#define MONO_FINISH_GC_CRITICAL_REGION
+
+#define MONO_REQ_GC_NOT_CRITICAL
+
+
 #define CHECKED_MONO_INIT()
 #define CHECKED_BUILD_THREAD_TRANSITION(transition, info, from_state, suspend_count, next_state, suspend_count_delta)
 

diff --git a/mono/utils/mono-threads-coop.h b/mono/utils/mono-threads-coop.h
index 91f3cf811eccf5ef9b6131ae2dd6f4808cc43c13..52efd8309f3523ea7e5bfa86530ec563ce5e5e43 100644 (file)
--- a/mono/utils/mono-threads-coop.h
+++ b/mono/utils/mono-threads-coop.h
@@ -13,6 +13,8 @@
 #include <config.h>
 #include <glib.h>
 
+#include "checked-build.h"
+
 G_BEGIN_DECLS
 
 /* JIT specific interface */
@@ -58,6 +60,7 @@ mono_threads_safepoint (void)
 }
 
 #define MONO_PREPARE_BLOCKING  \
+       MONO_REQ_GC_NOT_CRITICAL;               \
        do {    \
                gpointer __dummy;       \
                gpointer __blocking_cookie = mono_threads_prepare_blocking (&__dummy)