From 73e3ff9328170374feee8c3675410adbc67c7757 Mon Sep 17 00:00:00 2001 From: Ken Brown Date: Mon, 24 Jun 2019 12:28:48 -0400 Subject: [PATCH] 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. --- 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 a2bdc8f3c..e8144d4d0 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 8e4c94e66..f1a4c2804 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 154be0847..80688e79e 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);