Fix deadlock in GC_help_marker caused by use of mark_cv of parent process

The marker threads of the parent process are blocked on mark_cv at
fork.  So pthread_cond_wait() malfunction (hang) is possible in the
child process without mark_cv state cleanup (reinitialization).

* pthread_support.c [PARALLEL_MARK] (mark_cv): Move static variable
definition upper to be before GC_start_mark_threads_inner).
* win32_threads.c [GC_PTHREADS_PARAMARK] (mark_cv): Likewise.
* pthread_support.c [PARALLEL_MARK && CAN_HANDLE_FORK] (mark_cv):
Do not initialize statically; add comment.
* win32_threads.c [GC_PTHREADS_PARAMARK && CAN_HANDLE_FORK] (mark_cv):
Likewise.
* pthread_support.c [PARALLEL_MARK && CAN_HANDLE_FORK]
(GC_start_mark_threads_inner): Initialize mark_cv to
PTHREAD_COND_INITIALIZER (unless available_markers_m1 <= 0 or
GC_parallel); add comment.
* win32_threads.c [GC_PTHREADS_PARAMARK && CAN_HANDLE_FORK]
(GC_start_mark_threads_inner): Likewise.
* pthread_support.c [PARALLEL_MARK] (GC_wait_marker,
GC_notify_all_marker): Add assertion that GC_parallel is true (so
mark_cv is initialized).
* win32_threads.c [GC_PTHREADS_PARAMARK] (GC_wait_marker,
GC_notify_all_marker): Likewise.

diff --git a/pthread_support.c b/pthread_support.c
index f996c7559fee508cd96cbcf6699397e967af352b..14b184cbb08c685942c44e1f6b9a555de0c68206 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -385,8 +385,11 @@ STATIC pthread_t GC_mark_threads[MAX_MARKERS];
 
 #ifdef CAN_HANDLE_FORK
   static int available_markers_m1 = 0;
+  static pthread_cond_t mark_cv;
+                        /* initialized by GC_start_mark_threads_inner   */
 #else
 # define available_markers_m1 GC_markers_m1
+  static pthread_cond_t mark_cv = PTHREAD_COND_INITIALIZER;
 #endif
 
 GC_INNER void GC_start_mark_threads_inner(void)
@@ -402,6 +405,18 @@ GC_INNER void GC_start_mark_threads_inner(void)
                 /* Skip if parallel markers disabled or already started. */
 #   ifdef CAN_HANDLE_FORK
       if (GC_parallel) return;
+
+      /* Initialize mark_cv (for the first time), or cleanup its value  */
+      /* after forking in the child process.  All the marker threads in */
+      /* the parent process were blocked on this variable at fork, so   */
+      /* pthread_cond_wait() malfunction (hang) is possible in the      */
+      /* child process without such a cleanup.                          */
+      /* TODO: This is not portable, it is better to shortly unblock    */
+      /* all marker threads in the parent process at fork.              */
+      {
+        pthread_cond_t mark_cv_local = PTHREAD_COND_INITIALIZER;
+        BCOPY(&mark_cv_local, &mark_cv, sizeof(mark_cv));
+      }
 #   endif
 
     GC_ASSERT(GC_fl_builder_count == 0);
@@ -2153,11 +2168,10 @@ GC_INNER void GC_notify_all_builder(void)
     }
 }
 
-static pthread_cond_t mark_cv = PTHREAD_COND_INITIALIZER;
-
 GC_INNER void GC_wait_marker(void)
 {
     ASSERT_CANCEL_DISABLED();
+    GC_ASSERT(GC_parallel);
     UNSET_MARK_LOCK_HOLDER;
     if (pthread_cond_wait(&mark_cv, &mark_mutex) != 0) {
         ABORT("pthread_cond_wait failed");
@@ -2168,6 +2182,7 @@ GC_INNER void GC_wait_marker(void)
 
 GC_INNER void GC_notify_all_marker(void)
 {
+    GC_ASSERT(GC_parallel);
     if (pthread_cond_broadcast(&mark_cv) != 0) {
         ABORT("pthread_cond_broadcast failed");
     }

diff --git a/win32_threads.c b/win32_threads.c
index a17e018c616f082cfec2ac689f328b12deb70369..b34429f4741a535d5a20b24a62d34e2c542d3e56 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1780,6 +1780,13 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       /* Id not guaranteed to be unique. */
 #   endif
 
+#   ifdef CAN_HANDLE_FORK
+      static pthread_cond_t mark_cv;
+                        /* initialized by GC_start_mark_threads_inner   */
+#   else
+      static pthread_cond_t mark_cv = PTHREAD_COND_INITIALIZER;
+#   endif
+
     /* GC_start_mark_threads is the same as in pthread_support.c except */
     /* for thread stack that is assumed to be large enough.             */
 
@@ -1797,6 +1804,12 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
                 /* Skip if parallel markers disabled or already started. */
 #     ifdef CAN_HANDLE_FORK
         if (GC_parallel) return;
+
+        /* Reset mark_cv state after forking (as in pthread_support.c). */
+        {
+          pthread_cond_t mark_cv_local = PTHREAD_COND_INITIALIZER;
+          BCOPY(&mark_cv_local, &mark_cv, sizeof(mark_cv));
+        }
 #     endif
 
       GC_ASSERT(GC_fl_builder_count == 0);
@@ -1925,10 +1938,9 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
       }
     }
 
-    static pthread_cond_t mark_cv = PTHREAD_COND_INITIALIZER;
-
     GC_INNER void GC_wait_marker(void)
     {
+      GC_ASSERT(GC_parallel);
       UNSET_MARK_LOCK_HOLDER;
       if (pthread_cond_wait(&mark_cv, &mark_mutex) != 0) {
         ABORT("pthread_cond_wait failed");
@@ -1939,6 +1951,7 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 
     GC_INNER void GC_notify_all_marker(void)
     {
+      GC_ASSERT(GC_parallel);
       if (pthread_cond_broadcast(&mark_cv) != 0) {
         ABORT("pthread_cond_broadcast failed");
       }