Re: [PATCH 2/3] run-command: factor out running_main_thread function

From: Jeff King <peff@peff.net>
Date: Apr 16, 2013, 02:53 UTC
Message-ID: <20130416025340.GB20932@sigill.intra.peff.net>
In-Reply-To: <20130416014504.GD3262@elie.Belkin>

On Mon, Apr 15, 2013 at 06:45:04PM -0700, Jonathan Nieder wrote:

Show 28 quoted lines

> Jeff King wrote:
> 
> > --- a/run-command.c
> > +++ b/run-command.c
> > @@ -603,7 +603,7 @@ static NORETURN void die_async(const char *err, va_list params)
> >  {
> >  	vreportf("fatal: ", err, params);
> >  
> > -	if (!pthread_equal(main_thread, pthread_self())) {
> > +	if (!running_main_thread()) {
> >  		struct async *async = pthread_getspecific(async_key);
> >  		if (async->proc_in >= 0)
> >  			close(async->proc_in);
> > @@ -614,6 +614,19 @@ static NORETURN void die_async(const char *err, va_list params)
> >  
> >  	exit(128);
> >  }
> > +
> > +int running_main_thread(void)
> > +{
> > +	return pthread_equal(main_thread, pthread_self());
> > +}
> 
> Would it make sense to do something like
> 
> 	return !main_thread_set || pthread_equal(...);
> 
> in case someone tries to call this before the first start_async call?

I considered that, but assumed that pthread_equal would always return false against the zeroed out static main_thread. Mostly because we appeared not to be bothering with such a check already. But actually, in the existing code, it is only checked from die_async, which is only put into place when we _do_ start a thread, so we would never hit this code path without main_thread being set.

So yes, a check probably would make sense. However, I think I'm convinced that we should drop this in favor of using thread-local storage for the "dying" counter. So this patch can just go away.

Thanks for reviewing this version, though.

-Peff