summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--builtin/grep.c46
-rw-r--r--grep.c39
-rw-r--r--grep.h13
3 files changed, 35 insertions, 63 deletions
diff --git a/builtin/grep.c b/builtin/grep.c
index 91fc032..4a436d6 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -200,12 +200,12 @@ static void start_threads(struct grep_opt *opt)
int i;
pthread_mutex_init(&grep_mutex, NULL);
- pthread_mutex_init(&grep_read_mutex, NULL);
pthread_mutex_init(&grep_attr_mutex, NULL);
pthread_cond_init(&cond_add, NULL);
pthread_cond_init(&cond_write, NULL);
pthread_cond_init(&cond_result, NULL);
grep_use_locks = 1;
+ enable_obj_read_lock();
for (i = 0; i < ARRAY_SIZE(todo); i++) {
strbuf_init(&todo[i].out, 0);
@@ -257,12 +257,12 @@ static int wait_all(void)
free(threads);
pthread_mutex_destroy(&grep_mutex);
- pthread_mutex_destroy(&grep_read_mutex);
pthread_mutex_destroy(&grep_attr_mutex);
pthread_cond_destroy(&cond_add);
pthread_cond_destroy(&cond_write);
pthread_cond_destroy(&cond_result);
grep_use_locks = 0;
+ disable_obj_read_lock();
return hit;
}
@@ -295,16 +295,6 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
return st;
}
-static void *lock_and_read_oid_file(const struct object_id *oid, enum object_type *type, unsigned long *size)
-{
- void *data;
-
- grep_read_lock();
- data = read_object_file(oid, type, size);
- grep_read_unlock();
- return data;
-}
-
static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
const char *filename, int tree_name_len,
const char *path)
@@ -413,20 +403,20 @@ static int grep_submodule(struct grep_opt *opt,
/*
* NEEDSWORK: submodules functions need to be protected because they
- * access the object store via config_from_gitmodules(): the latter
- * uses get_oid() which, for now, relies on the global the_repository
- * object.
+ * call config_from_gitmodules(): the latter contains in its call stack
+ * many thread-unsafe operations that are racy with object reading, such
+ * as parse_object() and is_promisor_object().
*/
- grep_read_lock();
+ obj_read_lock();
sub = submodule_from_path(superproject, &null_oid, path);
if (!is_submodule_active(superproject, path)) {
- grep_read_unlock();
+ obj_read_unlock();
return 0;
}
if (repo_submodule_init(&subrepo, superproject, sub)) {
- grep_read_unlock();
+ obj_read_unlock();
return 0;
}
@@ -443,7 +433,7 @@ static int grep_submodule(struct grep_opt *opt,
* object.
*/
add_to_alternates_memory(subrepo.objects->odb->path);
- grep_read_unlock();
+ obj_read_unlock();
memcpy(&subopt, opt, sizeof(subopt));
subopt.repo = &subrepo;
@@ -455,13 +445,12 @@ static int grep_submodule(struct grep_opt *opt,
unsigned long size;
struct strbuf base = STRBUF_INIT;
- grep_read_lock();
+ obj_read_lock();
object = parse_object_or_die(oid, oid_to_hex(oid));
+ obj_read_unlock();
data = read_object_with_reference(&subrepo,
&object->oid, tree_type,
&size, NULL);
- grep_read_unlock();
-
if (!data)
die(_("unable to read tree (%s)"), oid_to_hex(&object->oid));
@@ -586,7 +575,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
void *data;
unsigned long size;
- data = lock_and_read_oid_file(&entry.oid, &type, &size);
+ data = read_object_file(&entry.oid, &type, &size);
if (!data)
die(_("unable to read tree (%s)"),
oid_to_hex(&entry.oid));
@@ -624,12 +613,9 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
struct strbuf base;
int hit, len;
- grep_read_lock();
data = read_object_with_reference(opt->repo,
&obj->oid, tree_type,
&size, NULL);
- grep_read_unlock();
-
if (!data)
die(_("unable to read tree (%s)"), oid_to_hex(&obj->oid));
@@ -659,17 +645,17 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,
for (i = 0; i < nr; i++) {
struct object *real_obj;
- grep_read_lock();
+ obj_read_lock();
real_obj = deref_tag(opt->repo, list->objects[i].item,
NULL, 0);
- grep_read_unlock();
+ obj_read_unlock();
/* load the gitmodules file for this rev */
if (recurse_submodules) {
submodule_free(opt->repo);
- grep_read_lock();
+ obj_read_lock();
gitmodules_config_oid(&real_obj->oid);
- grep_read_unlock();
+ obj_read_unlock();
}
if (grep_object(opt, pathspec, real_obj, list->objects[i].name,
list->objects[i].path)) {
diff --git a/grep.c b/grep.c
index c028f70..13232a9 100644
--- a/grep.c
+++ b/grep.c
@@ -1540,11 +1540,6 @@ static inline void grep_attr_unlock(void)
pthread_mutex_unlock(&grep_attr_mutex);
}
-/*
- * Same as git_attr_mutex, but protecting the thread-unsafe object db access.
- */
-pthread_mutex_t grep_read_mutex;
-
static int match_funcname(struct grep_opt *opt, struct grep_source *gs, char *bol, char *eol)
{
xdemitconf_t *xecfg = opt->priv;
@@ -1741,13 +1736,20 @@ static int fill_textconv_grep(struct repository *r,
}
/*
- * fill_textconv is not remotely thread-safe; it may load objects
- * behind the scenes, and it modifies the global diff tempfile
- * structure.
+ * fill_textconv is not remotely thread-safe; it modifies the global
+ * diff tempfile structure, writes to the_repo's odb and might
+ * internally call thread-unsafe functions such as the
+ * prepare_packed_git() lazy-initializator. Because of the last two, we
+ * must ensure mutual exclusion between this call and the object reading
+ * API, thus we use obj_read_lock() here.
+ *
+ * TODO: allowing text conversion to run in parallel with object
+ * reading operations might increase performance in the multithreaded
+ * non-worktreee git-grep with --textconv.
*/
- grep_read_lock();
+ obj_read_lock();
size = fill_textconv(r, driver, df, &buf);
- grep_read_unlock();
+ obj_read_unlock();
free_filespec(df);
/*
@@ -1813,12 +1815,15 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
grep_source_load_driver(gs, opt->repo->index);
/*
* We might set up the shared textconv cache data here, which
- * is not thread-safe.
+ * is not thread-safe. Also, get_oid_with_context() and
+ * parse_object() might be internally called. As they are not
+ * currenty thread-safe and might be racy with object reading,
+ * obj_read_lock() must be called.
*/
grep_attr_lock();
- grep_read_lock();
+ obj_read_lock();
textconv = userdiff_get_textconv(opt->repo, gs->driver);
- grep_read_unlock();
+ obj_read_unlock();
grep_attr_unlock();
}
@@ -2118,10 +2123,7 @@ static int grep_source_load_oid(struct grep_source *gs)
{
enum object_type type;
- grep_read_lock();
gs->buf = read_object_file(gs->identifier, &type, &gs->size);
- grep_read_unlock();
-
if (!gs->buf)
return error(_("'%s': unable to read %s"),
gs->name,
@@ -2186,11 +2188,8 @@ void grep_source_load_driver(struct grep_source *gs,
return;
grep_attr_lock();
- if (gs->path) {
- grep_read_lock();
+ if (gs->path)
gs->driver = userdiff_find_by_path(istate, gs->path);
- grep_read_unlock();
- }
if (!gs->driver)
gs->driver = userdiff_find_by_name("default");
grep_attr_unlock();
diff --git a/grep.h b/grep.h
index 811fd27..9115db8 100644
--- a/grep.h
+++ b/grep.h
@@ -220,18 +220,5 @@ int grep_threads_ok(const struct grep_opt *opt);
*/
extern int grep_use_locks;
extern pthread_mutex_t grep_attr_mutex;
-extern pthread_mutex_t grep_read_mutex;
-
-static inline void grep_read_lock(void)
-{
- if (grep_use_locks)
- pthread_mutex_lock(&grep_read_mutex);
-}
-
-static inline void grep_read_unlock(void)
-{
- if (grep_use_locks)
- pthread_mutex_unlock(&grep_read_mutex);
-}
#endif