Re: [PATCH v3] debug: test for more required cacellation points (BZ# 29274)

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

On 21/07/22 18:23, Carlos O'Donell wrote:
> On 7/18/22 12:46, Adhemerval Zanella wrote:
>> +
>> +  /* This test is the early cancel version of the first test, and the intent
>> +     is to have the cancellation happen at one of two regions:
>> +
>> +     1. Before the cancellable syscall registers cancellation.
>> +     2. After the cancellable syscall registers cancellation.
>> +
>> +     This test will exercise if syscall cancellation registration is an atomic
>> +     operation or not since the transition from the regions is designed to be
>> +     atomic.
>> +
>> +     We can not control when the cancellation happens, but it will happen in
>> +     one of the two regions.  The first test (the !only_early version)
>> +     attempts to test the second region, while this test attempts to test the
>> +     first regiont and the transition with some probability.  */
>> +  for (int i = 0; i < array_length (tests); i++)
>> +    {
>> +      xpthread_barrier_init (&barrier, NULL, 2);
>> +      /* Reset the counter for the cleanup handler.  */
>> +      cl_called = 0;
>> +
>> +      /* After this wait the cancellation handler is in place.  */
>> +      pthread_t thr = xpthread_create (0, tests[i].tf, NULL);
>> +
>> +      xpthread_cancel (thr);
>> +      xpthread_barrier_wait (&barrier);
> 
> Shouldn't this be:
> 
> xpthread_barrier_wait (&barrier);
> xpthread_cancel (thr);
> 
> You want to:
> 
> (a) Make sure a cancellation handler is registered.
> (b) Deliver the signal *before* you reach the syscall __pthread_enable_asyncancel.
> (c) Observe the cancelled bit early and act upon it.
> 
> The test is effectively very similar to the first test but with no wait to allow
> the thread to get to and block on the syscall.
> 
In fact the ordering is correct because in this case there is no signal 
involved, pthread_cancel will just mask the thread canceled 
(CANCELING_BITMASK | CANCELED_BITMASK) since asynchronous mode is not set. 

Otherwise, pthread_cancel might be called when __pthread_enable_asyncancel
is already called (changing the mode to asynchronous), and the the signal
handler it will act, not __pthread_enable_asyncancel.