[newlib-cygwin] Cygwin: timerfd: avoid a deadlock

Ken Brown kbrown@sourceware.org
Tue Jun 25 19:47:00 GMT 2019


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

commit 9604a251bdeb3ae4a4c958efc14b4694d7324d11
Author: Ken Brown <kbrown@cornell.edu>
Date:   Mon Jun 24 12:28:48 2019 -0400

    Cygwin: timerfd: avoid a deadlock
    
    Add a function timerfd_tracker::enter_critical_section_cancelable,
    which is like enter_critical_section but honors a cancel event.  Call
    this when a timer expires while the timerfd thread is in its inner
    loop.  This avoids a deadlock if timerfd_tracker::dtor has entered its
    critical section and is trying to cancel the thread.  See
    https://cygwin.com/ml/cygwin/2019-06/msg00096.html.

Diff:
---
 winsup/cygwin/release/3.1.0 |  3 +++
 winsup/cygwin/timerfd.cc    | 24 +++++++++++++++++++++++-
 winsup/cygwin/timerfd.h     |  2 ++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/release/3.1.0 b/winsup/cygwin/release/3.1.0
index a2bdc8f..e8144d4 100644
--- a/winsup/cygwin/release/3.1.0
+++ b/winsup/cygwin/release/3.1.0
@@ -34,3 +34,6 @@ Bug Fixes
 - Define missing MSG_EOR.  It's unsupported by the underlying Winsock
   layer so using it in send(2), sendto(2), or sendmsg(2) will return -1
   with errno set to EOPNOTSUPP and recvmsg(2) will never return it.
+
+- Fix a race condition in the timerfd code.
+  Addresses: https://cygwin.com/ml/cygwin/2019-06/msg00096.html
diff --git a/winsup/cygwin/timerfd.cc b/winsup/cygwin/timerfd.cc
index 8e4c94e..f1a4c28 100644
--- a/winsup/cygwin/timerfd.cc
+++ b/winsup/cygwin/timerfd.cc
@@ -89,6 +89,25 @@ timerfd_tracker::handle_timechange_window ()
     }
 }
 
+/* Like enter_critical_section, but returns -1 on a cancel event. */
+int
+timerfd_tracker::enter_critical_section_cancelable ()
+{
+  HANDLE w[2] = { cancel_evt, _access_mtx };
+  DWORD waitret = WaitForMultipleObjects (2, w, FALSE, INFINITE);
+
+  switch (waitret)
+    {
+    case WAIT_OBJECT_0:
+      return -1;
+    case WAIT_OBJECT_0 + 1:
+    case WAIT_ABANDONED_0 + 1:
+      return 1;
+    default:
+      return 0;
+    }
+}
+
 DWORD
 timerfd_tracker::thread_func ()
 {
@@ -137,7 +156,10 @@ timerfd_tracker::thread_func ()
 	      continue;
 	    }
 
-	  if (!enter_critical_section ())
+	  int ec = enter_critical_section_cancelable ();
+	  if (ec < 0)
+	    goto canceled;
+	  else if (!ec)
 	    continue;
 	  /* Make sure we haven't been abandoned and/or disarmed
 	     in the meantime */
diff --git a/winsup/cygwin/timerfd.h b/winsup/cygwin/timerfd.h
index 154be08..80688e7 100644
--- a/winsup/cygwin/timerfd.h
+++ b/winsup/cygwin/timerfd.h
@@ -86,6 +86,8 @@ class timerfd_tracker		/* cygheap! */
       return (WaitForSingleObject (_access_mtx, INFINITE) & ~WAIT_ABANDONED_0)
 	      == WAIT_OBJECT_0;
     }
+  /* A version that honors a cancel event, for use in thread_func. */
+  int enter_critical_section_cancelable ();
   void leave_critical_section ()
     {
       ReleaseMutex (_access_mtx);



More information about the Cygwin-cvs mailing list