Skip to content

Commit c3a5bb3

Browse files
matheustavaresgitster
authored andcommitted
grep: fix race conditions on userdiff calls
git-grep uses an internal grep_read_mutex to protect object reading operations. Similarly, there's a grep_attr_mutex to protect calls to the gitattributes machinery. However, two of the three functions protected by the last mutex may also perform object reading, as seen below: - userdiff_get_textconv() > notes_cache_init() > notes_cache_match_validity() > lookup_commit_reference_gently() > parse_object() > repo_has_object_file() > repo_has_object_file_with_flags() > oid_object_info_extended() - userdiff_find_by_path() > git_check_attr() > collect_some_attrs() > prepare_attr_stack() > read_attr() > read_attr_from_index() > read_blob_data_from_index() > read_object_file() As these calls are not protected by grep_read_mutex, there might be race conditions with other threads performing object reading (e.g. threads calling fill_textconv() at grep.c:fill_textconv_grep()). To prevent that, let's make sure to acquire the lock before both of these calls. Note: this patch might slow down the threaded grep in worktree, for the sake of thread-safeness. However, in the following patches, we should regain performance by replacing grep_read_mutex for an internal object reading lock and allowing parallel inflation during object reading. Signed-off-by: Matheus Tavares <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d0654dc commit c3a5bb3

File tree

1 file changed

+6
-1
lines changed

1 file changed

+6
-1
lines changed

grep.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1816,7 +1816,9 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
18161816
* is not thread-safe.
18171817
*/
18181818
grep_attr_lock();
1819+
grep_read_lock();
18191820
textconv = userdiff_get_textconv(opt->repo, gs->driver);
1821+
grep_read_unlock();
18201822
grep_attr_unlock();
18211823
}
18221824

@@ -2184,8 +2186,11 @@ void grep_source_load_driver(struct grep_source *gs,
21842186
return;
21852187

21862188
grep_attr_lock();
2187-
if (gs->path)
2189+
if (gs->path) {
2190+
grep_read_lock();
21882191
gs->driver = userdiff_find_by_path(istate, gs->path);
2192+
grep_read_unlock();
2193+
}
21892194
if (!gs->driver)
21902195
gs->driver = userdiff_find_by_name("default");
21912196
grep_attr_unlock();

0 commit comments

Comments
 (0)