Re: Segfault in the attr stack

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2016, 22:46 UTC
Message-ID: <xmqqh9dcwmrr.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAPc5daXuQAeWvJAciRA_Kzyoxa=atEntGzKhqzjiN+ho6TnQyg@mail.gmail.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> Gaah, of course.
>
> This is coming from the cache preload codepath, where multiple threads
> try to run ce_path_match().
> It used to be OK because pathspec magic never looked at attributes,
> but now it does, and attribute system is not thread-safe.

The symlink check code has dealt with a similar issue in the past in the codepath. threaded_has_symlink_leading_path() is called with per-thread data structure called "cache", because the leading symbolic link check uses a cache that is shared across different invocations of the check.

We need something similar to duplicate the attribute stack per thread. git_check_attrs() currently uses the singleton instance of attr_stack chain, that is rooted at the file scope global "attr_stack", and no wonder that would be clobbered when multiple threads try to use it. The result of attribute collection is also accumulated in a file scope global, which should probably be moved to the "struct git_attr_check" introduced in the jc/attr topic.

We need to teach the callchain that includes prepare_attr_stack() and bootstrap_attr_stack() to take a pointer to the attr_stack root, give git_check_attr_threaded() that takes such root so that threaded code can use per-thread attr stack, and for non-threaded applications use &the_default_attr just like the file scope global "default_cache" is used in symlinks.c. Then a threaded attribute lookup can maintain its own attr_stack when running more than one instance of lookup.