Re: [PATCH 01/10] nptl: Perform signal initialization upon pthread_create

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella:

>>  int
>>  __pthread_cancel (pthread_t th)
>> @@ -72,14 +129,23 @@ __pthread_cancel (pthread_t th)
>>  						    oldval))
>>  	    goto again;
>>  
>> -	  /* The cancellation handler will take care of marking the
>> -	     thread as canceled.  */
>> -	  pid_t pid = __getpid ();
>> -
>> -	  int val = INTERNAL_SYSCALL_CALL (tgkill, pid, pd->tid,
>> -					   SIGCANCEL);
>> -	  if (INTERNAL_SYSCALL_ERROR_P (val))
>> -	    result = INTERNAL_SYSCALL_ERRNO (val);
>> +	  if (pd == THREAD_SELF)
>> +	    /* This is not merely an optimization: An application may
>> +	       call pthread_cancel (pthread_self ()) without calling
>> +	       pthread_create, so the signal handler may not have been
>> +	       set up for a self-cancel.  */
>> +	    sigcancel_handler ();
>
> I think it would be simple to just call __pthread_exit (PTHREAD_CANCELED)
> here, it won't require to split the cancellation handler, it already
> unwind if cancel state is enabled and asynchronous, and it does not
> require add another PTHREAD_STATIC_FN_REQUIRE hack. 
>
> It would require an extra __libc_unwind_link_get call, but I think we
> can optimize it later (I am working on a patch to simplify it).

It would be correct, I think.  pthread_cancel is not a cancellation
point.

#include <stdio.h>
#include <pthread.h>

int
main (void)
{
  pthread_cancel (pthread_self ());
  puts ("about to exit");
}

This should print “about to exit”.

>> +/* This performs the initialization necessary when going from
>> +   single-threaded to multi-threaded mode for the first time.  */
>> +static void
>> +late_init (void)
>> +{
>> +  struct sigaction sa;
>> +  __sigemptyset (&sa.sa_mask);
>> +
>> +  /* Install the cancellation signal handler (in static builds only if
>> +     pthread_cancel has been linked in).  If for some reason we cannot
>> +     install the handler we do not abort.  Maybe we should, but it is
>> +     only asynchronous cancellation which is affected.  */
>> +#ifndef SHARED
>> +  extern __typeof (__nptl_sigcancel_handler) __nptl_sigcancel_handler
>> +    __attribute__ ((weak));
>> +  if (__nptl_sigcancel_handler != NULL)
>> +#endif
>
> This weak symbol can be avoided if we move the cancellation setup
> on pthread_cancel instead.  I still think this is best approach,
> it disentangle the cancellation handling.

But then we either have to introduce yet another global flag or install
the signal handler unconditionally before every cancel operation.  I do
not think this results in a simplification.

Thanks,
Florian