From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 2155) id D83E63858D28; Mon, 17 Jul 2023 11:05:10 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org D83E63858D28 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cygwin.com; s=default; t=1689591910; bh=NVBptv8991HNpVywS0/5d58XZL+2DorrzjgiB8kG+GQ=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=OtB9yuehLCQzeZYTS10zpr8SpPD6H1fGQ4gBr6ID8N4B1zdanEG8ZDAVkSP6bf3C1 fXMvS2xJUQAEZN8IAup5ufM3GjEhnApI17coQieD99JAm42rcOfYHYNQvDflISWihn XLGFAtlECxuPPC/w0SRQNeacGgEvLGbG2QSGWA+0= Received: by calimero.vinschen.de (Postfix, from userid 500) id D6824A80D3F; Mon, 17 Jul 2023 13:05:08 +0200 (CEST) Date: Mon, 17 Jul 2023 13:05:08 +0200 From: Corinna Vinschen To: Jon Turney Cc: cygwin-patches@cygwin.com Subject: Re: [PATCH 08/11] Cygwin: testsuite: Busy-wait in cancel3 and cancel5 Message-ID: Reply-To: cygwin-patches@cygwin.com Mail-Followup-To: Jon Turney , cygwin-patches@cygwin.com References: <20230713113904.1752-1-jon.turney@dronecode.org.uk> <20230713113904.1752-9-jon.turney@dronecode.org.uk> <5aa21952-a13d-f304-8b63-18ee4885c308@dronecode.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: List-Id: On Jul 14 20:57, Corinna Vinschen wrote: > On Jul 14 14:04, Jon Turney wrote: > > On 13/07/2023 19:53, Corinna Vinschen wrote: > > > > > Nevertheless, I think this is ok to do. The description of pthread_cancel > > > > > contains this: > > > > > > > > > > Asynchronous cancelability means that the thread can be canceled at > > > > > any time (usually immediately, but the system does not guarantee this). > > > > > > > > > > And > > > > > > > > > > The above steps happen asynchronously with respect to the > > > > > pthread_cancel() call; the return status of pthread_cancel() merely > > > > > informs the caller whether the cancellation request was successfully > > > > > queued. > > > > > > > > > > So any assumption *when* the cancallation takes place is may be wrong. > > > > Yeah. > > > > I think the flakiness is when we happen to try to async cancel while in the > > Windows kernel, which implicitly converts to a deferred cancellation, but > > there are no cancellation points in the the thread, so it arrives at > > pthread_exit() and returns a exit code other than PTHREAD_CANCELED. > > In pthread_join(), right? > > > I did consider making the test non-flaky by adding a final call to > > pthread_testcancel(), to notice any failed async cancellation which has been > > converted to a deferred one. > > > > But then that is just the same as the deferred cancellation tests, and > > confirms the cancellation happens, but not that it's async, which is part of > > the point of the test. > > What if Cygwin checks for a deferred cancellation in pthread::exit, > too? It needs to do this by its own, not calling pthread::testcancel, > otherwise we're in an infinite loop. Since cancel is basically like > exit, just with a PTHREAD_CANCELED return value, the only additional > action would be to set retval to PTHREAD_CANCELED explicitely. Kind of like this: diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc index f614e01c42f6..fceb9bda1806 100644 --- a/winsup/cygwin/thread.cc +++ b/winsup/cygwin/thread.cc @@ -546,6 +546,13 @@ pthread::exit (void *value_ptr) class pthread *thread = this; _cygtls *tls = cygtls; /* Save cygtls before deleting this. */ + /* Deferred cancellation still pending? */ + if (canceled) + { + WaitForSingleObject (cancel_event, INFINITE); + value_ptr = PTHREAD_CANCELED; + } + // run cleanup handlers pop_all_cleanup_handlers (); What do you think? Corinna