Skip to content

Commit 6c30762

Browse files
matheustavaresgitster
authored andcommitted
grep: protect packed_git [re-]initialization
Some fields in struct raw_object_store are lazy initialized by the thread-unsafe packfile.c:prepare_packed_git(). Although this function is present in the call stack of git-grep threads, all paths to it are currently protected by obj_read_lock() (and the main thread usually indirectly calls it before firing the worker threads, anyway). However, it's possible that future modifications add new unprotected paths to it, introducing a race condition. Because errors derived from it wouldn't happen often, it could be hard to detect. So to prevent future headaches, let's force eager initialization of packed_git when setting git-grep up. There'll be a small overhead in the cases where we didn't really need to prepare packed_git during execution but this shouldn't be very noticeable. Also, packed_git may be re-initialized by packfile.c:reprepare_packed_git(). Again, all paths to it in git-grep are already protected by obj_read_lock() but it may suffer from the same problem in the future. So let's also internally protect it with obj_read_lock() (which is a recursive mutex). Signed-off-by: Matheus Tavares <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent c441ea4 commit 6c30762

File tree

2 files changed

+8
-2
lines changed

2 files changed

+8
-2
lines changed

builtin/grep.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "submodule.h"
2525
#include "submodule-config.h"
2626
#include "object-store.h"
27+
#include "packfile.h"
2728

2829
static char const * const grep_usage[] = {
2930
N_("git grep [<options>] [-e] <pattern> [<rev>...] [[--] <path>...]"),
@@ -1074,11 +1075,14 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
10741075
skip_first_line = 1;
10751076

10761077
/*
1077-
* Pre-read gitmodules (if not read already) to prevent racy
1078-
* lazy reading in worker threads.
1078+
* Pre-read gitmodules (if not read already) and force eager
1079+
* initialization of packed_git to prevent racy lazy
1080+
* reading/initialization once worker threads are started.
10791081
*/
10801082
if (recurse_submodules)
10811083
repo_read_gitmodules(the_repository, 1);
1084+
if (startup_info->have_repository)
1085+
(void)get_packed_git(the_repository);
10821086

10831087
start_threads(&opt);
10841088
} else {

packfile.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1004,12 +1004,14 @@ void reprepare_packed_git(struct repository *r)
10041004
{
10051005
struct object_directory *odb;
10061006

1007+
obj_read_lock();
10071008
for (odb = r->objects->odb; odb; odb = odb->next)
10081009
odb_clear_loose_cache(odb);
10091010

10101011
r->objects->approximate_object_count_valid = 0;
10111012
r->objects->packed_git_initialized = 0;
10121013
prepare_packed_git(r);
1014+
obj_read_unlock();
10131015
}
10141016

10151017
struct packed_git *get_packed_git(struct repository *r)

0 commit comments

Comments
 (0)