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

Corinna Vinschen corinna@sourceware.org
Fri Dec 6 10:47:12 GMT 2024


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

commit f930a973bd98dfa2ade96e2ae2c4e39554a80f5f
Author:     Takashi Yano <takashi.yano@nifty.ne.jp>
AuthorDate: Wed Nov 27 20:03:55 2024 +0900
Commit:     Corinna Vinschen <corinna@vinschen.de>
CommitDate: Fri Dec 6 11:30:24 2024 +0100

    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>
    (cherry picked from commit 9ae51bcc51a7901559c476e6301597760c2726fd)

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 60c1f594f8d4..42556093adf5 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1419,7 +1419,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;
@@ -1434,7 +1434,11 @@ _cygtls::handle_SIGCONT ()
   while (1)
     if (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
@@ -1476,7 +1480,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 f67e9136c326..f967bb9cc272 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