[PATCH v3 2/2] nptl: Do not use pthread set_tid_address as state synchronization (BZ #19951)

[Florian Weimer]

* Adhemerval Zanella:

> diff --git a/nptl/pthread_cancel.c b/nptl/pthread_cancel.c
> index b838273881..f0828ea870 100644
> --- a/nptl/pthread_cancel.c
> +++ b/nptl/pthread_cancel.c
> @@ -60,7 +60,8 @@ __pthread_cancel (pthread_t th)
>  {
>    volatile struct pthread *pd = (volatile struct pthread *) th;
>  
> -  if (pd->tid == 0)
> +  int state = atomic_load_acquire (&pd->joinstate);
> +  if (state == THREAD_STATE_EXITED || state == THREAD_STATE_EXITING)
>      /* The thread has already exited on the kernel side.  Its outcome
>         (regular exit, other cancelation) has already been
>         determined.  */

The “has already exited on the kernel side” part is not true anymore.

In pthread_kill, we have  this comment for a similar situation:

    /* The thread is about to exit (or has exited).  Sending the
       signal is either not observable (the target thread has already
       blocked signals at this point), or it will fail, or it might be
       delivered to a new, unrelated thread that has reused the TID.
       So do not actually send the signal.  */

>    if (__glibc_unlikely (atomic_fetch_add_relaxed (&__nptl_nthreads, -1) == 1))
>      /* This was the last thread.  */
>      exit (0);
> @@ -574,20 +592,21 @@ start_thread (void *arg)
>        pd->setxid_futex = 0;
>      }
>  
> -  /* If the thread is detached free the TCB.  */
> -  if (IS_DETACHED (pd))
> +  if (prevstate == THREAD_STATE_DETACHED)
>      /* Free the TCB.  */
>      __nptl_free_tcb (pd);
>  
>    /* Remove the associated name from the thread stack.  */
>    name_stack_maps (pd, false);
>  
> +  pd->tid = 0;

We have this:

int
__pthread_getcpuclockid (pthread_t threadid, clockid_t *clockid)
{
  struct pthread *pd = (struct pthread *) threadid;

  /* Make sure the descriptor is valid.  */
  if (INVALID_TD_P (pd))
    /* Not a valid thread handle.  */
    return ESRCH;
…
}

I asume the pd->tid assignment amove ensures that pthread_getcpuclockid
can still be used t probe whether a thread is running?  Do we have a
test for this?  There should be a comment on the assignment.

We anticipated a change as far as pthread_gettid_np is concerned and
mentioned the possible behaviors in the manual.

>From a debuggability point of view, it may be beneficial not to do the
pd->tid assignment, so that it's possible to see previous TIDs in
coredumps.  We can look at pd->joinstate in pthread_getcpuclockid
and keep the current behavior, I think.

Thanks,
Florian