From: Ivan Maidanski <ivmai@mail.ru>
Date: Mon, 2 Apr 2018 22:23:01 +0000 (+0300)
Subject: Handle pthread restart signals loss if retry_signals
X-Git-Tag: v8.0.0~260
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3498427;p=gc

Handle pthread restart signals loss if retry_signals

Issue #181 (bdwgc).

* doc/README.environment (GC_RETRY_SIGNALS, GC_NO_RETRY_SIGNALS):
Update documentation (support of restart signals loss, try signals
if compiled with ASan/MSan/TSan).
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL]
(GC_retry_signals): Set true also if ADDRESS_SANITIZER or
MEMORY_SANITIZER.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL
&& !GC_NETBSD_THREADS_WORKAROUND] (GC_suspend_handler_inner): Call
sem_post(GC_suspend_ack_sem) at the function end if GC_retry_signals;
update comment about the RESTART signal loss.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL
&& !GC_OPENBSD_UTHREADS] (GC_start_world): Call
resend_lost_signals(GC_restart_all) and update n_live_threads value.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL
&& !GC_OPENBSD_UTHREADS && !GC_NETBSD_THREADS_WORKAROUND]
(GC_start_world): Call suspend_restart_barrier() if GC_retry_signals.
* pthread_stop_world.c [!GC_OPENBSD_UTHREADS && !NACL] (GC_stop_init):
Update the message logged if GC_retry_signals.
---

diff --git a/doc/README.environment b/doc/README.environment
index 9721bb6f..b35eb894 100644
--- a/doc/README.environment
+++ b/doc/README.environment
@@ -97,11 +97,9 @@ GC_PRINT_BACK_HEIGHT - Print max length of chain through unreachable objects
                      (http://www.hpl.hp.com/techreports/2001/HPL-2001-251.html).
 
 GC_RETRY_SIGNALS, GC_NO_RETRY_SIGNALS - Try to compensate for lost
-                     thread suspend signals (Pthreads only).  On by
-                     default for OSF1, off otherwise.  Note
-                     that this does not work around a possible loss of
-                     thread restart signals.  This seems to be necessary for
-                     some versions of Tru64.  Since we've previously seen
+                     thread suspend and restart signals (Pthreads only).
+                     On by default for OSF1 (Tru64) or if the library is
+                     sanitized, off otherwise.  Since we've previously seen
                      similar issues on some other operating systems, it
                      was turned into a runtime flag to enable last-minute
                      work-arounds.
diff --git a/pthread_stop_world.c b/pthread_stop_world.c
index 8b13d346..b81274f2 100644
--- a/pthread_stop_world.c
+++ b/pthread_stop_world.c
@@ -123,7 +123,8 @@ STATIC volatile AO_t GC_world_is_stopped = FALSE;
                         /* before they are expected to stop (unless     */
                         /* they have stopped voluntarily).              */
 
-#if defined(GC_OSF1_THREADS) || defined(THREAD_SANITIZER)
+#if defined(GC_OSF1_THREADS) || defined(THREAD_SANITIZER) \
+    || defined(ADDRESS_SANITIZER) || defined(MEMORY_SANITIZER)
   STATIC GC_bool GC_retry_signals = TRUE;
 #else
   STATIC GC_bool GC_retry_signals = FALSE;
@@ -362,18 +363,20 @@ STATIC void GC_suspend_handler_inner(ptr_t dummy GC_ATTR_UNUSED,
       sigsuspend (&suspend_handler_mask);
   } while (AO_load_acquire(&GC_world_is_stopped)
            && AO_load(&GC_stop_count) == my_stop_count);
-  /* If the RESTART signal gets lost, we can still lose.  That should   */
-  /* be less likely than losing the SUSPEND signal, since we don't do   */
-  /* much between the sem_post and sigsuspend.                          */
-  /* We'd need more handshaking to work around that.                    */
-  /* Simply dropping the sigsuspend call should be safe, but is         */
-  /* unlikely to be efficient.                                          */
 
 # ifdef DEBUG_THREADS
     GC_log_printf("Continuing %p\n", (void *)self);
 # endif
 # ifdef GC_NETBSD_THREADS_WORKAROUND
     sem_post(&GC_suspend_ack_sem);
+# else
+    if (GC_retry_signals) {
+      /* If the RESTART signal loss is possible (though it should be    */
+      /* less likely than losing the SUSPEND signal as we do not do     */
+      /* much between the sem_post and sigsuspend calls), more          */
+      /* handshaking is provided to work around it.                     */
+      sem_post(&GC_suspend_ack_sem);
+    }
 # endif
   RESTORE_CANCEL(cancel_state);
 }
@@ -1107,8 +1110,15 @@ GC_INNER void GC_start_world(void)
                     /* the list of functions which synchronize memory). */
 #   endif
     n_live_threads = GC_restart_all();
-#   if !defined(GC_OPENBSD_UTHREADS) && defined(GC_NETBSD_THREADS_WORKAROUND)
-      suspend_restart_barrier(n_live_threads);
+#   ifndef GC_OPENBSD_UTHREADS
+      if (GC_retry_signals)
+        n_live_threads = resend_lost_signals(n_live_threads, GC_restart_all);
+#     ifdef GC_NETBSD_THREADS_WORKAROUND
+        suspend_restart_barrier(n_live_threads);
+#     else
+        if (GC_retry_signals)
+          suspend_restart_barrier(n_live_threads);
+#     endif
 #   else
       (void)n_live_threads;
 #   endif
@@ -1193,7 +1203,8 @@ GC_INNER void GC_stop_init(void)
         GC_retry_signals = FALSE;
     }
     if (GC_retry_signals) {
-      GC_COND_LOG_PRINTF("Will retry suspend signal if necessary\n");
+      GC_COND_LOG_PRINTF(
+                "Will retry suspend and restart signals if necessary\n");
     }
 #   ifndef NO_SIGNALS_UNBLOCK_IN_MAIN
       /* Explicitly unblock the signals once before new threads creation. */