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

(Cherry-pick commit 9b7ef4d from 'release-7_6' branch.)

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 start_mark_threads).
* 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]
(start_mark_threads): 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]
(start_mark_threads): 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 67f9575bc1785fc6a1efdebef350f3fc8dfdb1d5..1ad2dac86c705f4d014406c88138e876c62753a6 100644 (file)
--- a/pthread_support.c
+++ b/pthread_support.c
@@ -384,9 +384,12 @@ STATIC pthread_t GC_mark_threads[MAX_MARKERS];
 #ifdef CAN_HANDLE_FORK
   static int available_markers_m1 = 0;
 # define start_mark_threads GC_start_mark_threads
+  static pthread_cond_t mark_cv;
+                        /* initialized by start_mark_threads */
   GC_API void GC_CALL
 #else
 # define available_markers_m1 GC_markers_m1
+  static pthread_cond_t mark_cv = PTHREAD_COND_INITIALIZER;
   static void
 #endif
 start_mark_threads(void)
@@ -399,6 +402,18 @@ start_mark_threads(void)
 #   ifdef CAN_HANDLE_FORK
       if (available_markers_m1 <= 0 || GC_parallel) return;
                 /* Skip if parallel markers disabled or already started. */
+
+      /* 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
 
     INIT_REAL_SYMS(); /* for pthread_create */
@@ -2078,11 +2093,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");
@@ -2093,6 +2107,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 6de84227c4bd25c39fbe91952da172ae3f54df20..61100add3acbf7782634f5d57116aaef1a5523e9 100644 (file)
--- a/win32_threads.c
+++ b/win32_threads.c
@@ -1742,6 +1742,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 start_mark_threads */
+#   else
+      static pthread_cond_t mark_cv = PTHREAD_COND_INITIALIZER;
+#   endif
+
     /* start_mark_threads is the same as in pthread_support.c except    */
     /* for thread stack that is assumed to be large enough.             */
 #   ifdef CAN_HANDLE_FORK
@@ -1762,6 +1769,12 @@ GC_INNER void GC_get_next_stack(char *start, char *limit,
 #     ifdef CAN_HANDLE_FORK
         if (available_markers_m1 <= 0 || GC_parallel) return;
                 /* Skip if parallel markers disabled or already started. */
+
+        /* 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
 
       if (0 != pthread_attr_init(&attr)) ABORT("pthread_attr_init failed");
@@ -1866,10 +1879,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");
@@ -1880,6 +1892,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");
       }