Re: [PATCH v2] grep: take the read-lock when adding a submodule

[Brandon Williams <bmwill@google.com>]

On 11/01, Martin Ågren wrote:
> With --recurse-submodules, we add each submodule that we encounter to
> the list of alternate object databases. With threading, our changes to
> the list are not protected against races. Indeed, ThreadSanitizer
> reports a race when we call `add_to_alternates_memory()` around the same
> time that another thread is reading in the list through
> `read_sha1_file()`.
> 
> Take the grep read-lock while adding the submodule. The lock is used to
> serialize uses of non-thread-safe parts of Git's API, including
> `read_sha1_file()`.
> 
> Helped-by: Brandon Williams <bmwill@google.com>
> Signed-off-by: Martin Ågren <martin.agren@gmail.com>
> ---
> Many thanks to Brandon for showing how this should have been done.

Of course! Happy to help :)

And this looks good, thanks for fixing my mistake!

> 
>  builtin/grep.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/builtin/grep.c b/builtin/grep.c
> index 2d65f27d0..5a6cfe6b4 100644
> --- a/builtin/grep.c
> +++ b/builtin/grep.c
> @@ -431,7 +431,9 @@ static int grep_submodule(struct grep_opt *opt, struct repository *superproject,
>  	 * store is no longer global and instead is a member of the repository
>  	 * object.
>  	 */
> +	grep_read_lock();
>  	add_to_alternates_memory(submodule.objectdir);
> +	grep_read_unlock();
>  
>  	if (oid) {
>  		struct object *object;
> -- 
> 2.15.0.415.gac1375d7e
> 

-- 
Brandon Williams