Cygwin: signal: Avoid frequent TLS lock/unlock for SIGCONT processing

It seems that current _cygtls::handle_SIGCONT() code sometimes falls into a deadlock due to frequent TLS lock/unlock operation in the yield() loop. With this patch, the yield() in the wait loop is placed outside the TLS lock to avoid frequent TLS lock/unlock. Fixes: 9ae51bc ("Cygwin: signal: Fix another deadlock between main and sig thread") Reviewed-by: Corinna Vinschen <[email protected]> Signed-off-by: Takashi Yano <[email protected]>
msys2 · Jan 22, 2025 · 25a3862 · 25a3862
1 parent ae26e17
commit 25a3862
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 25 deletions.
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
@@ -1420,7 +1420,7 @@ api_fatal_debug ()
 
 /* Attempt to carefully handle SIGCONT when we are stopped. */
 void
-_cygtls::handle_SIGCONT (threadlist_t * &tl_entry)
+_cygtls::handle_SIGCONT ()
 {
   if (NOTSTATE (myself, PID_STOPPED))
     return;
@@ -1431,23 +1431,17 @@ _cygtls::handle_SIGCONT (threadlist_t * &tl_entry)
      Make sure that any pending signal is handled before trying to
      send a new one.  Then make sure that SIGCONT has been recognized
      before exiting the loop.  */
-  bool sigsent = false;
-  while (1)
-    if (sig)		/* Assume that it's ok to just test sig outside of a
-			   lock since setup_handler does it this way.  */
-      {
-	cygheap->unlock_tls (tl_entry);
-	yield ();	/* Attempt to schedule another thread.  */
-	tl_entry = cygheap->find_tls (_main_tls);
-      }
-    else if (sigsent)
-      break;		/* SIGCONT has been recognized by other thread */
-    else
-      {
-	sig = SIGCONT;
-	set_signal_arrived (); /* alert sig_handle_tty_stop */
-	sigsent = true;
-      }
+  while (sig)  /* Assume that it's ok to just test sig outside of a */
+    yield ();  /* lock since setup_handler does it this way.  */
+
+  lock ();
+  sig = SIGCONT;
+  set_signal_arrived (); /* alert sig_handle_tty_stop */
+  unlock ();
+
+  while (sig == SIGCONT)
+    yield ();
+
   /* Clear pending stop signals */
   sig_clear (SIGSTOP, false);
   sig_clear (SIGTSTP, false);
@@ -1479,11 +1473,7 @@ sigpacket::process ()
   myself->rusage_self.ru_nsignals++;
 
   if (si.si_signo == SIGCONT)
-    {
-      tl_entry = cygheap->find_tls (_main_tls);
-      _main_tls->handle_SIGCONT (tl_entry);
-      cygheap->unlock_tls (tl_entry);
-    }
+    _main_tls->handle_SIGCONT ();
 
   /* SIGKILL is special.  It always goes through.  */
   if (si.si_signo == SIGKILL)

diff --git a/winsup/cygwin/local_includes/cygtls.h b/winsup/cygwin/local_includes/cygtls.h
@@ -195,7 +195,7 @@ class _cygtls
   class cygthread *_ctinfo;
   class san *andreas;
   waitq wq;
-  int sig;
+  volatile int sig;
   unsigned incyg;
   volatile unsigned spinning;
   volatile unsigned stacklock;
@@ -276,7 +276,7 @@ class _cygtls
   {
     will_wait_for_signal = false;
   }
-  void handle_SIGCONT (threadlist_t * &);
+  void handle_SIGCONT ();
   static void cleanup_early(struct _reent *);
 private:
   void call2 (DWORD (*) (void *, void *), void *, void *);