Re: [PATCH 1/2] run-command: provide in_async query function

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2015, 22:09 UTC
Message-ID: <xmqq613trdu3.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20150901202243.GA6931@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 21 quoted lines

> It's not easy for arbitrary code to find out whether it is
> running in an async process or not. A top-level function
> which is fed to start_async() can know (you just pass down
> an argument saying "you are async"). But that function may
> call other global functions, and we would not want to have
> to pass the information all the way through the call stack.
>
> Nor can we simply set a global variable, as those may be
> shared between async threads and the main thread (if the
> platform supports pthreads). We need pthread tricks _or_ a
> global variable, depending on how start_async is
> implemented.
>
> The callers don't have enough information to do this right,
> so let's provide a simple query function that does.
> Fortunately we can reuse the existing infrastructure to make
> the pthread case simple (and even simplify die_async() by
> using our new function).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

What is not immediately obvious from the above description is why a code may want to care if it is in_async() in the first place.

If there weren't the die_async() update, the readers might have been left utterly baffled (or they can somehow see this is related to 2/2) but it is a bit hard to arrange in "git log" as going to child is harder.

The patch looks good.  Thanks.

Show 63 quoted lines

>  run-command.c | 16 +++++++++++++++-
>  run-command.h |  1 +
>  2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/run-command.c b/run-command.c
> index 3277cf7..c8029f2 100644
> --- a/run-command.c
> +++ b/run-command.c
> @@ -595,7 +595,7 @@ static NORETURN void die_async(const char *err, va_list params)
>  {
>  	vreportf("fatal: ", err, params);
>  
> -	if (!pthread_equal(main_thread, pthread_self())) {
> +	if (in_async()) {
>  		struct async *async = pthread_getspecific(async_key);
>  		if (async->proc_in >= 0)
>  			close(async->proc_in);
> @@ -614,6 +614,13 @@ static int async_die_is_recursing(void)
>  	return ret != NULL;
>  }
>  
> +int in_async(void)
> +{
> +	if (!main_thread_set)
> +		return 0; /* no asyncs started yet */
> +	return !pthread_equal(main_thread, pthread_self());
> +}
> +
>  #else
>  
>  static struct {
> @@ -653,6 +660,12 @@ int git_atexit(void (*handler)(void))
>  }
>  #define atexit git_atexit
>  
> +static int process_is_async;
> +int in_async(void)
> +{
> +	return process_is_async;
> +}
> +
>  #endif
>  
>  int start_async(struct async *async)
> @@ -712,6 +725,7 @@ int start_async(struct async *async)
>  		if (need_out)
>  			close(fdout[0]);
>  		git_atexit_clear();
> +		process_is_async = 1;
>  		exit(!!async->proc(proc_in, proc_out, async->data));
>  	}
>  
> diff --git a/run-command.h b/run-command.h
> index 5b4425a..629fab7 100644
> --- a/run-command.h
> +++ b/run-command.h
> @@ -118,5 +118,6 @@ struct async {
>  
>  int start_async(struct async *async);
>  int finish_async(struct async *async);
> +int in_async(void);
>  
>  #endif