Fix race (and potential deadlock) at marker threads initialization

* include/private/gc_priv.h (GC_wait_for_markers_init): New prototype.
* mark.c (GC_wait_for_markers_init): New function (if PARALLEL_MARK).
* pthread_support.c (GC_mark_thread): Notify start_mark_threads()
about completion of marker data initialization.
* win32_threads.c (GC_mark_thread): Likewise.
* pthread_support.c (start_mark_threads): Add assertion about
GC_fl_builder_count; call GC_wait_for_markers_init.
* win32_threads.c (start_mark_threads): Likewise.
* win32_threads.c (GC_mark_thread): Reformat code.

diff --git a/include/private/gc_priv.h b/include/private/gc_priv.h
index d267c7e540180d71898c854ee8bb3d160d79f727..5ef73e81d028dd3d82fcd6ec04201153a94923ec 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2372,6 +2372,7 @@ GC_INNER ptr_t GC_store_debug_info(ptr_t p, word sz, const char *str,
   /* GC_notify_all_builder() is called when GC_fl_builder_count         */
   /* reaches 0.                                                         */
 
+  GC_INNER void GC_wait_for_markers_init(void);
   GC_INNER void GC_acquire_mark_lock(void);
   GC_INNER void GC_release_mark_lock(void);
   GC_INNER void GC_notify_all_builder(void);

diff --git a/mark.c b/mark.c
index 6e594804513ddcb147ad3c04f7c6b1a28c25158c..869265fcf063725374420f804dc68c056dbe99f9 100644 (file)
--- a/mark.c
+++ b/mark.c
@@ -888,6 +888,24 @@ GC_INNER word GC_mark_no = 0;
         /* we don't overflow half of it in a single call to             */
         /* GC_mark_from.                                                */
 
+/* Wait all markers to finish initialization (i.e. store        */
+/* marker_[b]sp, marker_mach_threads, GC_marker_Id).            */
+GC_INNER void GC_wait_for_markers_init(void)
+{
+  word count;
+
+  if (GC_markers_m1 == 0)
+    return;
+
+  /* Reuse marker lock and builders count to synchronize        */
+  /* marker threads startup.                                    */
+  GC_acquire_mark_lock();
+  GC_fl_builder_count += (word)GC_markers_m1;
+  count = GC_fl_builder_count;
+  GC_release_mark_lock();
+  if (count != 0)
+    GC_wait_for_reclaim();
+}
 
 /* Steal mark stack entries starting at mse low into mark stack local   */
 /* until we either steal mse high, or we have max entries.              */

diff --git a/pthread_support.c b/pthread_support.c
index 883a5690a641aff8dfb229137798337fa66d7ee8..8ec39bf3db790f48f8bac3c9183f0671c2f094fc 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -370,6 +370,12 @@ STATIC void * GC_mark_thread(void * id)
     marker_mach_threads[(word)id] = mach_thread_self();
 # endif
 
+  /* Inform start_mark_threads() about completion of marker data init.  */
+  GC_acquire_mark_lock();
+  if (0 == --GC_fl_builder_count)
+    GC_notify_all_builder();
+  GC_release_mark_lock();
+
   for (;; ++my_mark_no) {
     /* GC_mark_no is passed only to allow GC_help_marker to terminate   */
     /* promptly.  This is important if it were called from the signal   */
@@ -406,6 +412,7 @@ start_mark_threads(void)
     pthread_attr_t attr;
 
     GC_ASSERT(I_DONT_HOLD_LOCK());
+    GC_ASSERT(GC_fl_builder_count == 0);
 #   ifdef CAN_HANDLE_FORK
       if (available_markers_m1 <= 0 || GC_parallel) return;
                 /* Skip if parallel markers disabled or already started. */
@@ -444,6 +451,7 @@ start_mark_threads(void)
     }
     GC_markers_m1 = i;
     (void)pthread_attr_destroy(&attr);
+    GC_wait_for_markers_init();
     GC_COND_LOG_PRINTF("Started %d mark helper threads\n", GC_markers_m1);
 }
 

diff --git a/win32_threads.c b/win32_threads.c
index 2073b6a7c0dd81e7f931a3bb37178b06ef19010b..bb15259cafbeb273444ce64c875d5f865efc434b 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1681,12 +1681,10 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
   /* GC_mark_thread() is the same as in pthread_support.c */
 # ifdef GC_PTHREADS_PARAMARK
     STATIC void * GC_mark_thread(void * id)
+# elif defined(MSWINCE)
+    STATIC DWORD WINAPI GC_mark_thread(LPVOID id)
 # else
-#   ifdef MSWINCE
-      STATIC DWORD WINAPI GC_mark_thread(LPVOID id)
-#   else
-      STATIC unsigned __stdcall GC_mark_thread(void * id)
-#   endif
+    STATIC unsigned __stdcall GC_mark_thread(void * id)
 # endif
   {
     word my_mark_no = 0;
@@ -1700,6 +1698,12 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       GC_marker_Id[(word)id] = GetCurrentThreadId();
 #   endif
 
+    /* Inform start_mark_threads() about completion of marker data init. */
+    GC_acquire_mark_lock();
+    if (0 == --GC_fl_builder_count)
+      GC_notify_all_builder();
+    GC_release_mark_lock();
+
     for (;; ++my_mark_no) {
       if (my_mark_no - GC_mark_no > (word)2) {
         /* resynchronize if we get far off, e.g. because GC_mark_no     */
@@ -1749,6 +1753,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       pthread_t new_thread;
 
       GC_ASSERT(I_DONT_HOLD_LOCK());
+      GC_ASSERT(GC_fl_builder_count == 0);
 #     ifdef CAN_HANDLE_FORK
         if (available_markers_m1 <= 0 || GC_parallel) return;
                 /* Skip if parallel markers disabled or already started. */
@@ -1769,6 +1774,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       }
       GC_markers_m1 = i;
       (void)pthread_attr_destroy(&attr);
+      GC_wait_for_markers_init();
       GC_COND_LOG_PRINTF("Started %d mark helper threads\n", GC_markers_m1);
     }
 
@@ -1890,6 +1896,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
         unsigned thread_id;
 #     endif
 
+      GC_ASSERT(GC_fl_builder_count == 0);
       /* Initialize GC_marker_cv[] fully before starting the    */
       /* first helper thread.                                   */
       for (i = 0; i < GC_markers_m1; ++i) {
@@ -1936,6 +1943,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
         GC_markers_m1--;
         CloseHandle(GC_marker_cv[GC_markers_m1]);
       }
+      GC_wait_for_markers_init();
       GC_COND_LOG_PRINTF("Started %d mark helper threads\n", GC_markers_m1);
       if (i == 0) {
         CloseHandle(mark_cv);