Subject: Re: What's cooking in git.git (Nov 2016, #06; Mon, 28)

From: Brandon Williams <[email-masked]>
Date: Wed, 30 Nov 2016 11:54:27 -0800

On 11/29, Jeff King wrote:
> On Tue, Nov 29, 2016 at 01:37:59AM -0500, Jeff King wrote:
> 
> >   2. Grep threads doing more complicated stuff that needs to take a
> >      lock. You might try building with -fsanitize=thread to see if it
> >      turns up anything.
> 
> I tried this and it didn't find anything useful. It complains about
> multiple threads calling want_color() at the same time, which you can
> silence with something like:
> 
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2c727ef49..d48846f40 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -207,6 +207,12 @@ static void start_threads(struct grep_opt *opt)
>  {
>  	int i;
>  
> +	/*
> +	 * trigger want_color() for its side effect of caching the result;
> +	 * otherwise the threads will fight over setting the cache
> +	 */
> +	want_color(GIT_COLOR_AUTO);
> +
>  	pthread_mutex_init(&grep_mutex, NULL);
>  	pthread_mutex_init(&grep_read_mutex, NULL);
>  	pthread_mutex_init(&grep_attr_mutex, NULL);
> 
> But the problem persists even with that patch, so it is something else.
> It may still be a threading problem; -fsanitize=thread isn't perfect. I
> also couldn't get the stress-test to fail when compiled with it. But
> that may simply mean that the timing of the resulting binary is changed
> enough not to trigger the issue.
> 
> -Peff

With you're stress script I'm able to see the failures.  The interesting
thing is that the entry missing is always from the non-submodule file.

-- 
Brandon Williams