Re: [PATCH v2 2/9] nptl: Deallocate the thread stack on setup failure (BZ #19511)

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella via Libc-alpha:

> @@ -424,17 +409,22 @@ start_thread (void *arg)
>  	 have ownership (see CONCURRENCY NOTES above).  */
>        if (__glibc_unlikely (pd->stopped_start))
>  	{
>  	  /* Get the lock the parent locked to force synchronization.  */
>  	  lll_lock (pd->lock, LLL_PRIVATE);
>  
>  	  /* We have ownership of PD now.  */
> +	  if (pd->setup_failed == 1)
> +	    {
> +	      /* In the case of a failed setup, the created thread will be
> +		 responsible to free up the allocated resources.  The
> +		 detached mode ensure it won't be joined and it will trigger
> +		 the required cleanup.  */
> +	      pd->joinid = pd;
> +	      __do_cancel ();
> +	    }
>  
>  	  /* And give it up right away.  */
>  	  lll_unlock (pd->lock, LLL_PRIVATE);
>  	}

I don't think this is quite right.  The failed thread should not linger
around after pthread_create has reported failure to the caller.  It's
better than what we had before, so maybe we should use this version and
clean it up further later.

At the point of the __do_cancel call, unwinding may not yet initialized,
so the implied dlopen happens on a potentially very small stack.  It
should be possible to replace it with a goto to the cleanup code.  Or
maybe put the cleanup code into a separate function and call it here and
on the regular cleanup path.

Apart from that, it looks fine to me.

Thanks,
Florian