[PATCH v6 1/4] nptl: Set cancellation type and state on pthread_exit (BZ #28267)

[Adhemerval Zanella Netto]

On 08/12/25 17:03, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> diff --git a/nptl/tst-cleanup5.c b/nptl/tst-cleanup5.c
>> new file mode 100644
>> index 0000000000..582ce68063
>> --- /dev/null
>> +++ b/nptl/tst-cleanup5.c
> 
>> +/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on
>> +   pthread_cleanup_pop sets the correct state and type as pthread_exit.  */
>> +static void *
>> +tf_cancel (void *arg)
>> +{
>> +  int *cancel_type = (int *) (arg);
>> +
>> +  TEST_VERIFY (pthread_setcanceltype (*cancel_type, NULL) == 0);
>> +
>> +  xpthread_barrier_wait (&b);
>> +
>> +  pthread_cleanup_push (clh, NULL);
>> +
>> +  xread (pipefds[0], &(char){0}, 1);
>> +
>> +  pthread_cleanup_pop (1);
>> +
>> +  return NULL;
>> +}
>> +
>> +/* Check if a thread with PTHREAD_CANCEL_DEFERRED cancellation on
>> +   blocked read() sets the correct state and type as pthread_exit.  */
>> +static void *
>> +tf_testcancel (void *arg)
>> +{
>> +  int *cancel_type = (int *) (arg);
>> +
>> +  TEST_VERIFY (pthread_setcanceltype (*cancel_type, NULL) == 0);
>> +
>> +  xpthread_barrier_wait (&b);
>> +
>> +  pthread_cleanup_push (clh, NULL);
>> +
>> +  xread (pipefds[0], &(char){0}, 1);
>> +
>> +  pthread_testcancel ();
>> +
>> +  pthread_cleanup_pop (1);
>> +
>> +  return NULL;
>> +}
> 
> Gemini claims that the pthread_cancel is dead code.

The issue in fact the xread, since the function is actually checking
pthread_testcancel.  I will fix it.

> 
> I think there should probably be a test failure marker after each xread.

Ack.

> 
> I think the references to PTHREAD_CANCEL_DEFERRED in the comments are no
> longer correct?

Ack, I will change the comment.

> 
>> diff --git a/sysdeps/nptl/pthreadP.h b/sysdeps/nptl/pthreadP.h
>> index 4f649267b3..881a37cedd 100644
>> --- a/sysdeps/nptl/pthreadP.h
>> +++ b/sysdeps/nptl/pthreadP.h
>> @@ -252,7 +252,20 @@ __do_cancel (void *result)
>>    self->result = result;
>>  
>>    /* Make sure we get no more cancellations.  */
>> -  atomic_fetch_or_relaxed (&self->cancelhandling, EXITING_BITMASK);
>> +  int oldval = atomic_load_relaxed (&self->cancelhandling);
>> +  int newval;
>> +  do
>> +    {
>> +      /* It is required by POSIX XSH 2.9.5 Thread Cancellation under the
>> +	 heading Thread Cancellation Cleanup Handlers and also avoid further
>> +	 cancellation wrapper to act on cancellation.  */
>> +      newval = oldval | CANCELSTATE_BITMASK | EXITING_BITMASK;
>> +      newval = newval & ~CANCELTYPE_BITMASK;
>> +      if (oldval == newval)
> 
> “and also prevents further cancellation points from acting on
> cancellation”
> 
> (I think)

Ack.

> 
> Thanks,
> Florian
>