Skip to content

G
git2
Commit 4ae41f9c by lhchavez
Options

Make the odb race-free 

This change adds all the necessary locking to the odb to avoid races in
the backends.

Part of: #5592
parent ef6de8d5
Showing with 235 additions and 67 deletions
script/thread-sanitizer.supp
......@@ -8,11 +8,6 @@ deadlock:attr_cache_lock
# data races.
called_from_lib:libc.so.6
# TODO(#5595): Remove these once the fixes land.
race:src/odb.c
race:git_repository_odb__weakptr
race:cache_store
# TODO(#5595): Investigate and fix this. It can be triggered by the `thread`
# TODO(#5592): Investigate and fix this. It can be triggered by the `thread`
# test suite.
race:git_filter_list__load_ext
src/config_cache.c
......@@ -111,17 +111,18 @@ int git_config__configmap_lookup(int *out, git_config *config, git_configmap_ite
int git_repository__configmap_lookup(int *out, git_repository *repo, git_configmap_item item)
{
*out = repo->configmap_cache[(int)item];
*out = (int)(intptr_t)git__load(repo->configmap_cache[(int)item]);
if (*out == GIT_CONFIGMAP_NOT_CACHED) {
int error;
int oldval = GIT_CONFIGMAP_NOT_CACHED;
git_config *config;
if ((error = git_repository_config__weakptr(&config, repo)) < 0 ||
(error = git_config__configmap_lookup(out, config, item)) < 0)
return error;
repo->configmap_cache[(int)item] = *out;
git__compare_and_swap(&repo->configmap_cache[(int)item], &oldval, out);
}
return 0;
......
src/odb.c
......@@ -450,12 +450,18 @@ int git_odb_new(git_odb **out)
git_odb *db = git__calloc(1, sizeof(*db));
GIT_ERROR_CHECK_ALLOC(db);
if (git_mutex_init(&db->lock) < 0) {
git__free(db);
return -1;
}
if (git_cache_init(&db->own_cache) < 0) {
git_mutex_free(&db->lock);
git__free(db);
return -1;
}
if (git_vector_init(&db->backends, 4, backend_sort_cmp) < 0) {
git_cache_dispose(&db->own_cache);
git_mutex_free(&db->lock);
git__free(db);
return -1;
}
......@@ -487,13 +493,18 @@ static int add_backend_internal(
internal->is_alternate = is_alternate;
internal->disk_inode = disk_inode;
if (git_mutex_lock(&odb->lock) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return -1;
}
if (git_vector_insert(&odb->backends, internal) < 0) {
git_mutex_unlock(&odb->lock);
git__free(internal);
return -1;
}
git_vector_sort(&odb->backends);
internal->backend->odb = odb;
git_mutex_unlock(&odb->lock);
return 0;
}
......@@ -509,8 +520,19 @@ int git_odb_add_alternate(git_odb *odb, git_odb_backend *backend, int priority)
size_t git_odb_num_backends(git_odb *odb)
{
size_t length;
bool locked = true;
GIT_ASSERT_ARG(odb);
return odb->backends.length;
if (git_mutex_lock(&odb->lock) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
locked = false;
}
length = odb->backends.length;
if (locked)
git_mutex_unlock(&odb->lock);
return length;
}
static int git_odb__error_unsupported_in_backend(const char *action)
......@@ -524,19 +546,28 @@ static int git_odb__error_unsupported_in_backend(const char *action)
int git_odb_get_backend(git_odb_backend **out, git_odb *odb, size_t pos)
{
backend_internal *internal;
int error;
GIT_ASSERT_ARG(out);
GIT_ASSERT_ARG(odb);
if ((error = git_mutex_lock(&odb->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
internal = git_vector_get(&odb->backends, pos);
if (internal && internal->backend) {
*out = internal->backend;
return 0;
if (!internal || !internal->backend) {
git_mutex_unlock(&odb->lock);
git_error_set(GIT_ERROR_ODB, "no ODB backend loaded at index %" PRIuZ, pos);
return GIT_ENOTFOUND;
}
*out = internal->backend;
git_mutex_unlock(&odb->lock);
git_error_set(GIT_ERROR_ODB, "no ODB backend loaded at index %" PRIuZ, pos);
return GIT_ENOTFOUND;
return 0;
}
int git_odb__add_default_backends(
......@@ -567,11 +598,18 @@ int git_odb__add_default_backends(
inode = st.st_ino;
if (git_mutex_lock(&db->lock) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return -1;
}
for (i = 0; i < db->backends.length; ++i) {
backend_internal *backend = git_vector_get(&db->backends, i);
if (backend->disk_inode == inode)
if (backend->disk_inode == inode) {
git_mutex_unlock(&db->lock);
return 0;
}
}
git_mutex_unlock(&db->lock);
#endif
/* add the loose object backend */
......@@ -687,7 +725,12 @@ int git_odb__set_caps(git_odb *odb, int caps)
static void odb_free(git_odb *db)
{
size_t i;
bool locked = true;
if (git_mutex_lock(&db->lock) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
locked = false;
}
for (i = 0; i < db->backends.length; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *backend = internal->backend;
......@@ -696,9 +739,12 @@ static void odb_free(git_odb *db)
git__free(internal);
}
if (locked)
git_mutex_unlock(&db->lock);
git_vector_free(&db->backends);
git_cache_dispose(&db->own_cache);
git_mutex_free(&db->lock);
git__memzero(db, sizeof(*db));
git__free(db);
......@@ -719,7 +765,12 @@ static int odb_exists_1(
{
size_t i;
bool found = false;
int error;
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0; i < db->backends.length && !found; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -730,6 +781,7 @@ static int odb_exists_1(
if (b->exists != NULL)
found = (bool)b->exists(b, id);
}
git_mutex_unlock(&db->lock);
return (int)found;
}
......@@ -741,7 +793,12 @@ static int odb_freshen_1(
{
size_t i;
bool found = false;
int error;
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0; i < db->backends.length && !found; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -754,6 +811,7 @@ static int odb_freshen_1(
else if (b->exists != NULL)
found = b->exists(b, id);
}
git_mutex_unlock(&db->lock);
return (int)found;
}
......@@ -805,6 +863,11 @@ static int odb_exists_prefix_1(git_oid *out, git_odb *db,
int error = GIT_ENOTFOUND, num_found = 0;
git_oid last_found = {{0}}, found;
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
error = GIT_ENOTFOUND;
for (i = 0; i < db->backends.length; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -818,18 +881,23 @@ static int odb_exists_prefix_1(git_oid *out, git_odb *db,
error = b->exists_prefix(&found, b, key, len);
if (error == GIT_ENOTFOUND || error == GIT_PASSTHROUGH)
continue;
if (error)
if (error) {
git_mutex_unlock(&db->lock);
return error;
}
/* make sure found item doesn't introduce ambiguity */
if (num_found) {
if (git_oid__cmp(&last_found, &found))
if (git_oid__cmp(&last_found, &found)) {
git_mutex_unlock(&db->lock);
return git_odb__error_ambiguous("multiple matches for prefix");
}
} else {
git_oid_cpy(&last_found, &found);
num_found++;
}
}
git_mutex_unlock(&db->lock);
if (!num_found)
return GIT_ENOTFOUND;
......@@ -971,6 +1039,10 @@ static int odb_read_header_1(
return 0;
}
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0; i < db->backends.length; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -992,9 +1064,11 @@ static int odb_read_header_1(
case GIT_ENOTFOUND:
break;
default:
git_mutex_unlock(&db->lock);
return error;
}
}
git_mutex_unlock(&db->lock);
return passthrough ? GIT_PASSTHROUGH : GIT_ENOTFOUND;
}
......@@ -1067,6 +1141,10 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
return error;
}
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0; i < db->backends.length && !found; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -1079,12 +1157,15 @@ static int odb_read_1(git_odb_object **out, git_odb *db, const git_oid *id,
if (error == GIT_PASSTHROUGH || error == GIT_ENOTFOUND)
continue;
if (error < 0)
if (error < 0) {
git_mutex_unlock(&db->lock);
return error;
}
found = true;
}
}
git_mutex_unlock(&db->lock);
if (!found)
return GIT_ENOTFOUND;
......@@ -1177,6 +1258,10 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
bool found = false;
git_odb_object *object;
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0; i < db->backends.length; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -1193,8 +1278,10 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
continue;
}
if (error)
if (error) {
git_mutex_unlock(&db->lock);
goto out;
}
git__free(data);
data = raw.data;
......@@ -1209,6 +1296,7 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
error = git_odb__error_ambiguous(buf.ptr);
git_buf_dispose(&buf);
git_mutex_unlock(&db->lock);
goto out;
}
......@@ -1216,6 +1304,7 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
found = true;
}
}
git_mutex_unlock(&db->lock);
if (!found)
return GIT_ENOTFOUND;
......@@ -1283,16 +1372,32 @@ int git_odb_read_prefix(
int git_odb_foreach(git_odb *db, git_odb_foreach_cb cb, void *payload)
{
unsigned int i;
git_vector backends = GIT_VECTOR_INIT;
backend_internal *internal;
int error = 0;
git_vector_foreach(&db->backends, i, internal) {
/* Make a copy of the backends vector to invoke the callback without holding the lock. */
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
goto cleanup;
}
error = git_vector_dup(&backends, &db->backends, NULL);
git_mutex_unlock(&db->lock);
if (error < 0)
goto cleanup;
git_vector_foreach(&backends, i, internal) {
git_odb_backend *b = internal->backend;
int error = b->foreach(b, cb, payload);
error = b->foreach(b, cb, payload);
if (error != 0)
return error;
goto cleanup;
}
return 0;
cleanup:
git_vector_free(&backends);
return error;
}
int git_odb_write(
......@@ -1314,6 +1419,10 @@ int git_odb_write(
if (git_odb__freshen(db, oid))
return 0;
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0, error = GIT_ERROR; i < db->backends.length && error < 0; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -1325,6 +1434,7 @@ int git_odb_write(
if (b->write != NULL)
error = b->write(b, oid, data, len, type);
}
git_mutex_unlock(&db->lock);
if (!error || error == GIT_PASSTHROUGH)
return 0;
......@@ -1366,6 +1476,11 @@ int git_odb_open_wstream(
GIT_ASSERT_ARG(stream);
GIT_ASSERT_ARG(db);
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
error = GIT_ERROR;
for (i = 0; i < db->backends.length && error < 0; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -1382,6 +1497,7 @@ int git_odb_open_wstream(
error = init_fake_wstream(stream, b, size, type);
}
}
git_mutex_unlock(&db->lock);
if (error < 0) {
if (error == GIT_PASSTHROUGH)
......@@ -1477,6 +1593,11 @@ int git_odb_open_rstream(
GIT_ASSERT_ARG(stream);
GIT_ASSERT_ARG(db);
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
error = GIT_ERROR;
for (i = 0; i < db->backends.length && error < 0; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -1486,6 +1607,7 @@ int git_odb_open_rstream(
error = b->readstream(stream, len, type, b, oid);
}
}
git_mutex_unlock(&db->lock);
if (error == GIT_PASSTHROUGH)
error = 0;
......@@ -1503,6 +1625,11 @@ int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_indexer_
GIT_ASSERT_ARG(out);
GIT_ASSERT_ARG(db);
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
error = GIT_ERROR;
for (i = 0; i < db->backends.length && error < 0; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
......@@ -1516,6 +1643,7 @@ int git_odb_write_pack(struct git_odb_writepack **out, git_odb *db, git_indexer_
error = b->writepack(out, b, db, progress_cb, progress_payload);
}
}
git_mutex_unlock(&db->lock);
if (error == GIT_PASSTHROUGH)
error = 0;
......@@ -1547,19 +1675,27 @@ void git_odb_backend_data_free(git_odb_backend *backend, void *data)
int git_odb_refresh(struct git_odb *db)
{
size_t i;
int error;
GIT_ASSERT_ARG(db);
if ((error = git_mutex_lock(&db->lock)) < 0) {
git_error_set(GIT_ERROR_ODB, "failed to acquire the odb lock");
return error;
}
for (i = 0; i < db->backends.length; ++i) {
backend_internal *internal = git_vector_get(&db->backends, i);
git_odb_backend *b = internal->backend;
if (b->refresh != NULL) {
int error = b->refresh(b);
if (error < 0)
if (error < 0) {
git_mutex_unlock(&db->lock);
return error;
}
}
}
git_mutex_unlock(&db->lock);
return 0;
}
......
src/odb.h
......@@ -40,6 +40,7 @@ struct git_odb_object {
/* EXPORT */
struct git_odb {
git_refcount rc;
git_mutex lock; /* protects backends */
git_vector backends;
git_cache own_cache;
unsigned int do_fsync :1;
......
src/pack.c
......@@ -12,6 +12,7 @@
#include "mwindow.h"
#include "odb.h"
#include "oid.h"
#include "oidarray.h"
/* Option to bypass checking existence of '.keep' files */
bool git_disable_pack_keep_file_checks = false;
......@@ -195,7 +196,8 @@ static void pack_index_free(struct git_pack_file *p)
}
}
static int pack_index_check(const char *path, struct git_pack_file *p)
/* Run with the packfile lock held */
static int pack_index_check_locked(const char *path, struct git_pack_file *p)
{
struct git_pack_idx_header *hdr;
uint32_t version, nr, i, *index;
......@@ -301,7 +303,8 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
return 0;
}
static int pack_index_open(struct git_pack_file *p)
/* Run with the packfile lock held */
static int pack_index_open_locked(struct git_pack_file *p)
{
int error = 0;
size_t name_len;
......@@ -324,18 +327,11 @@ static int pack_index_open(struct git_pack_file *p)
return -1;
}
if ((error = git_mutex_lock(&p->lock)) < 0) {
git_buf_dispose(&idx_name);
return error;
}
if (p->index_version == -1)
error = pack_index_check(idx_name.ptr, p);
error = pack_index_check_locked(idx_name.ptr, p);
git_buf_dispose(&idx_name);
git_mutex_unlock(&p->lock);
return error;
}
......@@ -1015,13 +1011,14 @@ static int packfile_open(struct git_pack_file *p)
git_oid sha1;
unsigned char *idx_sha1;
if (p->index_version == -1 && pack_index_open(p) < 0)
return git_odb__error_notfound("failed to open packfile", NULL, 0);
/* if mwf opened by another thread, return now */
if (git_mutex_lock(&p->lock) < 0)
return packfile_error("failed to get lock for open");
if (pack_index_open_locked(p) < 0) {
git_mutex_unlock(&p->lock);
return git_odb__error_notfound("failed to open packfile", NULL, 0);
}
if (p->mwf.fd >= 0) {
git_mutex_unlock(&p->lock);
return 0;
......@@ -1210,32 +1207,40 @@ int git_pack_foreach_entry(
git_odb_foreach_cb cb,
void *data)
{
const unsigned char *index = p->index_map.data, *current;
const unsigned char *index, *current;
uint32_t i;
int error = 0;
git_array_oid_t oids = GIT_ARRAY_INIT;
git_oid *oid;
if (index == NULL) {
if ((error = pack_index_open(p)) < 0)
return error;
if (git_mutex_lock(&p->lock) < 0)
return packfile_error("failed to get lock for git_pack_foreach_entry");
GIT_ASSERT(p->index_map.data);
index = p->index_map.data;
if ((error = pack_index_open_locked(p)) < 0) {
git_mutex_unlock(&p->lock);
return error;
}
if (p->index_version > 1) {
GIT_ASSERT(p->index_map.data);
index = p->index_map.data;
if (p->index_version > 1)
index += 8;
}
index += 4 * 256;
if (p->oids == NULL) {
git_vector offsets, oids;
if ((error = git_vector_init(&oids, p->num_objects, NULL)))
if ((error = git_vector_init(&oids, p->num_objects, NULL))) {
git_mutex_unlock(&p->lock);
return error;
}
if ((error = git_vector_init(&offsets, p->num_objects, git__memcmp4)))
if ((error = git_vector_init(&offsets, p->num_objects, git__memcmp4))) {
git_mutex_unlock(&p->lock);
return error;
}
if (p->index_version > 1) {
const unsigned char *off = index + 24 * p->num_objects;
......@@ -1256,10 +1261,33 @@ int git_pack_foreach_entry(
p->oids = (git_oid **)git_vector_detach(NULL, NULL, &oids);
}
for (i = 0; i < p->num_objects; i++)
if ((error = cb(p->oids[i], data)) != 0)
return git_error_set_after_callback(error);
/* We need to copy the OIDs to another array before we relinquish the lock to avoid races. */
git_array_init_to_size(oids, p->num_objects);
if (!oids.ptr) {
git_mutex_unlock(&p->lock);
git_array_clear(oids);
GIT_ERROR_CHECK_ARRAY(oids);
}
for (i = 0; i < p->num_objects; i++) {
oid = git_array_alloc(oids);
if (!oid) {
git_mutex_unlock(&p->lock);
git_array_clear(oids);
GIT_ERROR_CHECK_ALLOC(oid);
}
git_oid_cpy(oid, p->oids[i]);
}
git_mutex_unlock(&p->lock);
git_array_foreach(oids, i, oid) {
if ((error = cb(oid, data)) != 0) {
git_error_set_after_callback(error);
break;
}
}
git_array_clear(oids);
return error;
}
......@@ -1297,18 +1325,17 @@ static int pack_entry_find_offset(
int pos, found = 0;
off64_t offset;
const unsigned char *current = 0;
int error = 0;
*offset_out = 0;
if (p->index_version == -1) {
int error;
if ((error = pack_index_open(p)) < 0)
return error;
if (git_mutex_lock(&p->lock) < 0)
return packfile_error("failed to get lock for pack_entry_find_offset");
GIT_ASSERT(p->index_map.data);
}
if ((error = pack_index_open_locked(p)) < 0)
goto cleanup;
GIT_ASSERT(p->index_map.data);
index = p->index_map.data;
level1_ofs = p->index_map.data;
......@@ -1360,14 +1387,19 @@ static int pack_entry_find_offset(
}
}
if (!found)
return git_odb__error_notfound("failed to find offset for pack entry", short_oid, len);
if (found > 1)
return git_odb__error_ambiguous("found multiple offsets for pack entry");
if (!found) {
error = git_odb__error_notfound("failed to find offset for pack entry", short_oid, len);
goto cleanup;
}
if (found > 1) {
error = git_odb__error_ambiguous("found multiple offsets for pack entry");
goto cleanup;
}
if ((offset = nth_packed_object_offset(p, pos)) < 0) {
git_error_set(GIT_ERROR_ODB, "packfile index is corrupt");
return -1;
error = -1;
goto cleanup;
}
*offset_out = offset;
......@@ -1382,7 +1414,9 @@ static int pack_entry_find_offset(
}
#endif
return 0;
cleanup:
git_mutex_unlock(&p->lock);
return error;
}
int git_pack_entry_find(
......
src/repository.c
......@@ -1107,7 +1107,8 @@ int git_repository_odb__weakptr(git_odb **out, git_repository *repo)
GIT_ASSERT_ARG(repo);
GIT_ASSERT_ARG(out);
if (repo->_odb == NULL) {
*out = git__load(repo->_odb);
if (*out == NULL) {
git_buf odb_path = GIT_BUF_INIT;
git_odb *odb;
......@@ -1131,9 +1132,9 @@ int git_repository_odb__weakptr(git_odb **out, git_repository *repo)
}
git_buf_dispose(&odb_path);
*out = git__load(repo->_odb);
}
*out = repo->_odb;
return error;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment