[PATCH v2 4/9] nptl: Remove CANCELING_BITMASK

[Florian Weimer]

* Adhemerval Zanella via Libc-alpha:

> The CANCELING_BITMASK is used as an optimization to avoid sending
> the signal when pthread_cancel is called in a concurrent manner.
>
> This requires then to put both the cancellation state and type on
> a shared state (cancelhandling), since 'pthread_cancel' checks
> whether cancellation is enabled and asynchrnous to either cancel
> itself of sending the signal.
>
> It also requires handle the CANCELING_BITMASK on
> __pthread_disable_asynccancel, however this is incurs in the same

typo: “this is incurs”

> issues described on BZ#12683: the cancellation is acting even *after*

“is acted upon even *after*”?

> the syscalls returns with user visible side-effects.
>
> This patch removes this optimization and simplifies the pthread
> cancellation implementation: pthread_cancel now first check if

typo: check → checks

> cancellation is already pending and if not always send a signal

typo: send → sends

Maybe add a comma before “always”.

> if the target is not itself.  The SIGCANCEL handler is also simpified
> since there is not need to setup a CAS loop.
>
> It also alows to move both the cancellation state and mode out of
> 'cancelhadling' (it is done in subsequent patches).

typo: alows

> diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
> index deb404600c..8dfbcff8c3 100644
> --- a/nptl/pthread_cancel.c
> +++ b/nptl/pthread_cancel.c

> @@ -104,72 +87,33 @@ __pthread_cancel (pthread_t th)
>  		    " must be installed for pthread_cancel to work\n");
>    }
> +
> +  int oldch = atomic_fetch_or_acquire (&pd->cancelhandling, CANCELED_BITMASK);
> +  if ((oldch & CANCELED_BITMASK) != 0)
> +    return 0;
> +
> +  if (pd == THREAD_SELF)
>      {
> +      /* A single-threaded process should be able to kill itself, since there
> +	 is nothing in the POSIX specification that says that it cannot.  So
> +	 we set multiple_threads to true so that cancellation points get
> +	 executed.  */
> +      THREAD_SETMEM (THREAD_SELF, header.multiple_threads, 1);
>  #ifndef TLS_MULTIPLE_THREADS_IN_TCB
> -	__libc_multiple_threads = 1;
> +      __libc_multiple_threads = 1;
>  #endif
> +
> +      THREAD_SETMEM (pd, result, PTHREAD_CANCELED);
> +      if ((oldch & CANCELTYPE_BITMASK) != 0)
> +	__do_cancel ();
> +      return 0;
>      }

The last part is for asynchronous self-cancel, right?  Don't we have to
check that cancellation is enabled, too?

Thanks,
Florian