[newlib-cygwin] Cygwin: signal: Fix another deadlock between main and sig thread

Takashi Yano tyan0@sourceware.org
Thu Nov 28 11:25:54 GMT 2024


https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=9ae51bcc51a7901559c476e6301597760c2726fd

commit 9ae51bcc51a7901559c476e6301597760c2726fd
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Wed Nov 27 20:03:55 2024 +0900

    Cygwin: signal: Fix another deadlock between main and sig thread
    
    In _cygtls::handle_SIGCONT(), the sig thread waits for the main thread
    to process the signal without unlocking the TLS area. This causes a
    deadlock if the main thread tries to acquire a lock for the TLS area
    in the meantime. With this patch, unlock the TLS before calling yield()
    in handle_SIGCONT().
    
    Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html
    Fixes: 26158dc3e9c2("* exceptions.cc (sigpacket::process): Lock _cygtls area of thread before accessing it.")
    Reported-by: Christian Franke <Christian.Franke@t-online.de>
    Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
    Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>

Diff:
---
 winsup/cygwin/exceptions.cc           | 10 +++++++---
 winsup/cygwin/local_includes/cygtls.h |  4 +++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 3b31e65c1..0f8c21939 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1421,7 +1421,7 @@ api_fatal_debug ()
 
 /* Attempt to carefully handle SIGCONT when we are stopped. */
 void
-_cygtls::handle_SIGCONT ()
+_cygtls::handle_SIGCONT (threadlist_t * &tl_entry)
 {
   if (NOTSTATE (myself, PID_STOPPED))
     return;
@@ -1436,7 +1436,11 @@ _cygtls::handle_SIGCONT ()
   while (1)
     if (current_sig)	/* Assume that it's ok to just test sig outside of a
 			   lock since setup_handler does it this way.  */
-      yield ();		/* Attempt to schedule another thread.  */
+      {
+	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
@@ -1478,7 +1482,7 @@ sigpacket::process ()
   if (si.si_signo == SIGCONT)
     {
       tl_entry = cygheap->find_tls (_main_tls);
-      _main_tls->handle_SIGCONT ();
+      _main_tls->handle_SIGCONT (tl_entry);
       cygheap->unlock_tls (tl_entry);
     }
 
diff --git a/winsup/cygwin/local_includes/cygtls.h b/winsup/cygwin/local_includes/cygtls.h
index 28bbe60f0..e5a377d6b 100644
--- a/winsup/cygwin/local_includes/cygtls.h
+++ b/winsup/cygwin/local_includes/cygtls.h
@@ -159,6 +159,8 @@ extern "C" int __ljfault (jmp_buf, int);
 
 typedef uintptr_t __tlsstack_t;
 
+struct threadlist_t;
+
 class _cygtls
 {
 public: /* Do NOT remove this public: line, it's a marker for gentls_offsets. */
@@ -262,7 +264,7 @@ public: /* Do NOT remove this public: line, it's a marker for gentls_offsets. */
   {
     will_wait_for_signal = false;
   }
-  void handle_SIGCONT ();
+  void handle_SIGCONT (threadlist_t * &);
   static void cleanup_early(struct _reent *);
 private:
   void call2 (DWORD (*) (void *, void *), void *, void *);


More information about the Cygwin-cvs mailing list