[profiler] Acquire the exclusive buffer lock after the suspend lock.

This prevents possible STW deadlocks.

diff --git a/libgc/include/gc.h b/libgc/include/gc.h
index e7929918ad0bb9df8d12b07a7803f660a8a3244f..7b1460a888ae20f5756aa3e8ad71781094541ffd 100644 (file)
--- a/libgc/include/gc.h
+++ b/libgc/include/gc.h
@@ -93,6 +93,7 @@ GC_API GC_PTR (*GC_oom_fn) GC_PROTO((size_t bytes_requested));
                        /* pointer to a previously allocated heap       */
                        /* object.                                      */
 
+// Keep somewhat in sync with mono/metadata/profiler.h:enum MonoGCEvent
 typedef enum {
        GC_EVENT_START,
        GC_EVENT_MARK_START,

diff --git a/mono/metadata/boehm-gc.c b/mono/metadata/boehm-gc.c
index 846e34757b04076f355b7369a47b7ffcf274a34d..b78411093ae6be4943a8e340706ffe3ea246f0d2 100644 (file)
--- a/mono/metadata/boehm-gc.c
+++ b/mono/metadata/boehm-gc.c
@@ -428,7 +428,6 @@ on_gc_notification (GC_EventType event)
        switch (e) {
        case MONO_GC_EVENT_PRE_STOP_WORLD:
                MONO_GC_WORLD_STOP_BEGIN ();
-               mono_thread_info_suspend_lock ();
                break;
 
        case MONO_GC_EVENT_POST_STOP_WORLD:
@@ -441,7 +440,6 @@ on_gc_notification (GC_EventType event)
 
        case MONO_GC_EVENT_POST_START_WORLD:
                MONO_GC_WORLD_RESTART_END (1);
-               mono_thread_info_suspend_unlock ();
                break;
 
        case MONO_GC_EVENT_START:
@@ -481,7 +479,21 @@ on_gc_notification (GC_EventType event)
        }
 
        mono_profiler_gc_event (e, 0);
+
+       switch (e) {
+       case MONO_GC_EVENT_PRE_STOP_WORLD:
+               mono_thread_info_suspend_lock ();
+               mono_profiler_gc_event (MONO_GC_EVENT_PRE_STOP_WORLD_LOCKED, 0);
+               break;
+       case MONO_GC_EVENT_POST_START_WORLD:
+               mono_thread_info_suspend_unlock ();
+               mono_profiler_gc_event (MONO_GC_EVENT_POST_START_WORLD_UNLOCKED, 0);
+               break;
+       default:
+               break;
+       }
 }
+
  
 static void
 on_gc_heap_resize (size_t new_size)

diff --git a/mono/metadata/profiler.h b/mono/metadata/profiler.h
index b793548ef8fbd98782f7975b1b08ffd0d0a3634d..278465b1f2a749ee389815f16fdb5c5c3e30325b 100644 (file)
--- a/mono/metadata/profiler.h
+++ b/mono/metadata/profiler.h
@@ -39,6 +39,7 @@ typedef enum {
        MONO_PROFILE_FAILED
 } MonoProfileResult;
 
+// Keep somewhat in sync with libgc/include/gc.h:enum GC_EventType
 typedef enum {
        MONO_GC_EVENT_START,
        MONO_GC_EVENT_MARK_START,
@@ -46,10 +47,26 @@ typedef enum {
        MONO_GC_EVENT_RECLAIM_START,
        MONO_GC_EVENT_RECLAIM_END,
        MONO_GC_EVENT_END,
+       /*
+        * This is the actual arrival order of the following events:
+        *
+        * MONO_GC_EVENT_PRE_STOP_WORLD
+        * MONO_GC_EVENT_PRE_STOP_WORLD_LOCKED
+        * MONO_GC_EVENT_POST_STOP_WORLD
+        * MONO_GC_EVENT_PRE_START_WORLD
+        * MONO_GC_EVENT_POST_START_WORLD_UNLOCKED
+        * MONO_GC_EVENT_POST_START_WORLD
+        *
+        * The LOCKED and UNLOCKED events guarantee that, by the time they arrive,
+        * the GC and suspend locks will both have been acquired and released,
+        * respectively.
+        */
        MONO_GC_EVENT_PRE_STOP_WORLD,
        MONO_GC_EVENT_POST_STOP_WORLD,
        MONO_GC_EVENT_PRE_START_WORLD,
-       MONO_GC_EVENT_POST_START_WORLD
+       MONO_GC_EVENT_POST_START_WORLD,
+       MONO_GC_EVENT_PRE_STOP_WORLD_LOCKED,
+       MONO_GC_EVENT_POST_START_WORLD_UNLOCKED
 } MonoGCEvent;
 
 /* coverage info */

diff --git a/mono/metadata/sgen-stw.c b/mono/metadata/sgen-stw.c
index ea14448ccfbfaeb7b376fbf06abd59135cf6b680..60e5933450fba844a1888577df232f4255102127 100644 (file)
--- a/mono/metadata/sgen-stw.c
+++ b/mono/metadata/sgen-stw.c
@@ -209,6 +209,8 @@ sgen_client_stop_world (int generation)
 
        acquire_gc_locks ();
 
+       mono_profiler_gc_event (MONO_GC_EVENT_PRE_STOP_WORLD_LOCKED, generation);
+
        /* We start to scan after locks are taking, this ensures we won't be interrupted. */
        sgen_process_togglerefs ();
 
@@ -283,6 +285,8 @@ sgen_client_restart_world (int generation, gint64 *stw_time)
         */
        release_gc_locks ();
 
+       mono_profiler_gc_event (MONO_GC_EVENT_POST_START_WORLD_UNLOCKED, generation);
+
        *stw_time = usec;
 }
 

diff --git a/mono/profiler/decode.c b/mono/profiler/decode.c
index e8d53afea686ed76588370c99fe74beac2c0718b..35ecd56c78aed7c2ef4e1cb7b6546b4ab01ffa10 100644 (file)
--- a/mono/profiler/decode.c
+++ b/mono/profiler/decode.c
@@ -1862,9 +1862,11 @@ gc_event_name (int ev)
        case MONO_GC_EVENT_RECLAIM_END: return "reclaim end";
        case MONO_GC_EVENT_END: return "end";
        case MONO_GC_EVENT_PRE_STOP_WORLD: return "pre stop";
+       case MONO_GC_EVENT_PRE_STOP_WORLD_LOCKED: return "pre stop lock";
        case MONO_GC_EVENT_POST_STOP_WORLD: return "post stop";
        case MONO_GC_EVENT_PRE_START_WORLD: return "pre start";
        case MONO_GC_EVENT_POST_START_WORLD: return "post start";
+       case MONO_GC_EVENT_POST_START_WORLD_UNLOCKED: return "post start unlock";
        default:
                return "unknown";
        }

diff --git a/mono/profiler/proflog.c b/mono/profiler/proflog.c
index a444a9afced8f1f5f72ebed3d2ec2e65038b0df9..3ca9b875bf660187409d1f959cb9a6f87f36d4b8 100644 (file)
--- a/mono/profiler/proflog.c
+++ b/mono/profiler/proflog.c
@@ -1339,7 +1339,7 @@ gc_event (MonoProfiler *profiler, MonoGCEvent ev, int generation)
                if (generation == mono_gc_max_generation ())
                        gc_count++;
                break;
-       case MONO_GC_EVENT_PRE_STOP_WORLD:
+       case MONO_GC_EVENT_PRE_STOP_WORLD_LOCKED:
                /*
                 * Ensure that no thread can be in the middle of writing to
                 * a buffer when the world stops...
@@ -1364,7 +1364,7 @@ gc_event (MonoProfiler *profiler, MonoGCEvent ev, int generation)
        case MONO_GC_EVENT_PRE_START_WORLD:
                heap_walk (profiler);
                break;
-       case MONO_GC_EVENT_POST_START_WORLD:
+       case MONO_GC_EVENT_POST_START_WORLD_UNLOCKED:
                /*
                 * Similarly, we must now make sure that any object moves
                 * written to the GC thread's buffer are flushed. Otherwise,

diff --git a/mono/profiler/proflog.h b/mono/profiler/proflog.h
index ecb0e43b82334160ce788b255733b12902fce6ae..43ebbb68c470437bf0410c8b43c12046bb839fb3 100644 (file)
--- a/mono/profiler/proflog.h
+++ b/mono/profiler/proflog.h
@@ -5,7 +5,7 @@
 #define LOG_HEADER_ID 0x4D505A01
 #define LOG_VERSION_MAJOR 0
 #define LOG_VERSION_MINOR 4
-#define LOG_DATA_VERSION 12
+#define LOG_DATA_VERSION 13
 /*
  * Changes in data versions:
  * version 2: added offsets in heap walk
@@ -27,6 +27,7 @@
                added TYPE_GC_HANDLE_{CREATED,DESTROYED}_BT
                TYPE_JIT events are no longer guaranteed to have code start/size info (can be zero)
  * version 12: added MONO_COUNTER_PROFILER
+ * version 13: added MONO_GC_EVENT_{PRE_STOP_WORLD_LOCKED,POST_START_WORLD_UNLOCKED}
  */
 
 enum {