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