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 9b060775b52c6a0034effb755b91c9c96cd114f9..8f05bc1f6c0906e85d2fafccbd54b9a4496d428c 100644 (file)
--- a/include/private/gc_priv.h
+++ b/include/private/gc_priv.h
@@ -2401,6 +2401,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 642dfebdff05bd8022c983d82b97395c75724bee..9c3820780a1e8566ddedd209975e579831129b33 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 e15912b643a33419c7b96456679930ac82bf6eb8..558ef9cadc5873f939b6a8705030b0439d52840f 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -372,6 +372,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   */
@@ -408,6 +414,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. */
@@ -446,6 +453,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 911370897c96d00eb8da37a478493e64f9c08e7b..d386f076df24373d736a3befbe1938dea68b7a89 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1704,12 +1704,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;
@@ -1723,6 +1721,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     */
@@ -1772,6 +1776,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. */
@@ -1792,6 +1797,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);
     }
 
@@ -1915,6 +1921,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) {
@@ -1961,6 +1968,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);