Re: Bug report about symlinks

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 4, 2014, 16:34 UTC
Message-ID: <xmqqbns0tdqu.fsf@gitster.dls.corp.google.com>
In-Reply-To: <53DEBEE7.6070009@web.de>

René Scharfe <l.s.r@web.de> writes:

Show 12 quoted lines

> Am 03.08.2014 um 19:19 schrieb Junio C Hamano:
>
>>> And do we need to use the threaded_ variant of the function here?
>>
>> Hmmm, this is a tangent, but you comment made me wonder if we also
>> need to adjust preload_thread() in preload-index.c somehow, but we
>> do not touch CE_UPTODATE there, so it probably is not necessary.
>
> The function calls ce_mark_uptodate(), which does set CE_UPTODATE.  It
> calls threaded_has_symlink_leading_path() before lstat() already,
> however.  (Since f62ce3de: Make index preloading check the whole path
> to the file.)

Yeah, by "we do not touch", I meant "for paths that is beyond a symlink, we do not touch" (i.e. we have that "continue" before lstat-match-then-mark sequence).

Show 8 quoted lines

>> The caller of refresh_cache_ent() is walking an array of sorted
>> pathnames aka istate->cache[] in a single-threaded fashion, possibly
>> with a pathspec to limit the scan.
>
> There are two direct callers (refresh_index(), refresh_cache_entry())
> and several indirect ones.  Do we have a way to detect unsynchronized
> parallel access to the has_symlink_leading_path()-cache?  Checking the
> full callers-of-callers tree manually looks a bit scary to me.

The threaded variant is not used anybody outside preload-index, so currently we should be OK, I would think.