This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: [PATCH] Cygwin: timerfd: avoid a deadlock
- From: Ken Brown <kbrown at cornell dot edu>
- To: "cygwin-patches at cygwin dot com" <cygwin-patches at cygwin dot com>
- Date: Tue, 25 Jun 2019 13:24:52 +0000
- Subject: Re: [PATCH] Cygwin: timerfd: avoid a deadlock
- Arc-authentication-results: i=1; test.office365.com 1;spf=none;dmarc=none;dkim=none;arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=testarcselector01; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=p35NZfUhmfkwSlowDB5XhAzUKmVjTHd4vw38uGmtC/E=; b=iTjtFihJsJaLtaDGqfs9mA8/oCteEfT8HvKbgwknyeHLjPS72UDriaTpoYXqfhhaXarsayN8T7Q0Q2vd64L53rwxUSLR60XjG10T8TFlWE4WNr5siVLlZCXuj+FIEbsvBlAn3dvy+vzmLhXqPOBUF1cy7KCm/aCKXRUOg4VNoi8=
- Arc-seal: i=1; a=rsa-sha256; s=testarcselector01; d=microsoft.com; cv=none; b=FihZCfem8Bnc23KOzc+xs7aJc246tkeB5lybzc6tratax1CiNkNfAsPYS5Ub/QKp+AV2A5QOWMWxdJPKWlo4JsIyKkyAM0w8g7p2VyKxXsf6nkJY9HWNodfE/m185Kf6QBXpcmiBMb9kabi+N9WLet9mQeNr8Vw7RhUeWTQSWpo=
- References: <20190624201852.26148-1-kbrown@cornell.edu> <20190625074348.GF5738@calimero.vinschen.de>
On 6/25/2019 3:43 AM, Corinna Vinschen wrote:
> Hi Ken,
>
> On Jun 24 20:19, Ken Brown wrote:
>> If a timer expires while the timerfd thread is in its inner loop,
>> check for the thread cancellation event before trying to enter
>> a_critical_section. It's possible that timerfd_tracker::dtor has
>> entered its critical section and is trying to cancel the thread. See
>> http://www.cygwin.org/ml/cygwin/2019-06/msg00096.html.
>> ---
>> winsup/cygwin/timerfd.cc | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/winsup/cygwin/timerfd.cc b/winsup/cygwin/timerfd.cc
>> index 8e4c94e66..e8261ef2e 100644
>> --- a/winsup/cygwin/timerfd.cc
>> +++ b/winsup/cygwin/timerfd.cc
>> @@ -137,6 +137,11 @@ timerfd_tracker::thread_func ()
>> continue;
>> }
>>
>> + /* Avoid a deadlock if dtor has just entered its critical
>> + section and is trying to cancel the thread. */
>> + if (IsEventSignalled (cancel_evt))
>> + goto canceled;
>
> This looks still racy, what if cancel_evt is set just between the
> IsEventSignalled() and enter_critical_section() calls?
>
> Hmm.
>
> What if we redefine enter_critical_section() to return three
> states. It calls WFMO on cancel_evt and _access_mtx, in this order,
> so that a cancel event is honored. Or maybe introduce another
> function like enter_critical_section_cancelable() which is only
> called in this single instance in timerfd_tracker::thread_func?
Yes, that's much better. I'll send v2 shortly, after some testing.
Ken