Re: Segfault in the attr stack

[Junio C Hamano <gitster@pobox.com>]

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

>>> 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.

I'll look into teaching a threadble interface to the attribute
subsystem, but for now, this should get you unstuck, I think.

One of the ways preload codepath assures that multiple threads do
not stomp on the shared datastructure is by copying things that are
involved in the operation, and there is copy_pathspec() call.  I
think the "pathspec magic that limits with the attributes" topic,
which adds to the pathspec structure, needs to update the function
so that its shared data structure is properly copied.  Before the
series, I think the pathspec structure only has either pointers to
borrowed strings (e.g. raw) or scalars, and it was sufficient to do
a shallow copy with memcpy().  With the change, that is no longer
the case.

Which would mean we'd need to make sure that any codepath that calls
copy_pathspec() needs to think about how to clean it up.  Before the
change, preload-index codepath didn't have to, but with the change,
it will.  We'd need pathspec_clear() or something like that.

 pathspec.c      | 12 ++++++++++++
 pathspec.h      |  2 ++
 preload-index.c |  4 ++++
 3 files changed, 18 insertions(+)

diff --git a/pathspec.c b/pathspec.c
index 0a02255..326863a 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -208,6 +208,18 @@ static void eat_long_magic(struct pathspec_item *item, const char *elt,
 	*copyfrom_ = copyfrom;
 }
 
+int pathspec_is_non_threadable(const struct pathspec *pathspec)
+{
+	int i;
+
+	for (i = 0; i < pathspec->nr; i++) {
+		const struct pathspec_item *item = &pathspec->items[i];
+		if (item->attr_check)
+			return 1;
+	}
+	return 0;
+}
+
 /*
  * Take an element of a pathspec and check for magic signatures.
  * Append the result to the prefix. Return the magic bitmap.
diff --git a/pathspec.h b/pathspec.h
index 5308137..07d21f0 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -115,4 +115,6 @@ extern void add_pathspec_matches_against_index(const struct pathspec *pathspec,
 extern const char *check_path_for_gitlink(const char *path);
 extern void die_if_path_beyond_symlink(const char *path, const char *prefix);
 
+extern int pathspec_is_non_threadable(const struct pathspec *);
+
 #endif /* PATHSPEC_H */
diff --git a/preload-index.c b/preload-index.c
index c1fe3a3..af46878 100644
--- a/preload-index.c
+++ b/preload-index.c
@@ -76,6 +76,10 @@ static void preload_index(struct index_state *index,
 	if (!core_preload_index)
 		return;
 
+	/* Do not preload when pathspec uses non-threadable subsystems */
+	if (pathspec && pathspec_is_non_threadable(pathspec))
+		return; /* for now ... */
+
 	threads = index->cache_nr / THREAD_COST;
 	if (threads < 2)
 		return;