On 14/07/2023 14:04, Jon Turney wrote:
> On 13/07/2023 19:53, Corinna Vinschen wrote:
>>>>> normally after 10 seconds. (See the commentary in pthread::cancel() in
>>>>> thread.cc, where it checks if the target thread is inside the kernel,
>>>>> and silently converts the cancellation into a deferred one)
>>>>
>>>> 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.
> 
> 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.
> 
> I guess this could also check that not all of the threads ran for all 10 
> seconds, which would indicate that at least some of them were cancelled 
> asynchronously.

I wrote this, attached, which does indeed make this test more reliable.

You point is well made that this is making assumptions about how quickly 
async cancellation works that are not required by the standard

(It would be a valid, if strange implementation, if async cancellation 
always took 10 seconds to take effect, which this test assumes isn't the 
case)

Perhaps there is a better way to write a test that async cancellation 
works in the absence of cancellation points, but it eludes me...