Commit 9d2f841a by Russell Belfer

Add extra locking around packfile open

We were still seeing a few issues in threaded access to packs.
This adds extra locks around the opening of the mwindow to
avoid a different race.
parent 69c50f4c
...@@ -205,13 +205,18 @@ static int pack_index_check(const char *path, struct git_pack_file *p) ...@@ -205,13 +205,18 @@ static int pack_index_check(const char *path, struct git_pack_file *p)
if (fd < 0) if (fd < 0)
return fd; return fd;
if (p_fstat(fd, &st) < 0 || if (p_fstat(fd, &st) < 0) {
!S_ISREG(st.st_mode) || p_close(fd);
giterr_set(GITERR_OS, "Unable to stat pack index '%s'", path);
return -1;
}
if (!S_ISREG(st.st_mode) ||
!git__is_sizet(st.st_size) || !git__is_sizet(st.st_size) ||
(idx_size = (size_t)st.st_size) < 4 * 256 + 20 + 20) (idx_size = (size_t)st.st_size) < 4 * 256 + 20 + 20)
{ {
p_close(fd); p_close(fd);
giterr_set(GITERR_OS, "Failed to check pack index."); giterr_set(GITERR_ODB, "Invalid pack index '%s'", path);
return -1; return -1;
} }
...@@ -402,7 +407,7 @@ int git_packfile_unpack_header( ...@@ -402,7 +407,7 @@ int git_packfile_unpack_header(
if (base == NULL) if (base == NULL)
return GIT_EBUFS; return GIT_EBUFS;
ret = packfile_unpack_header1(&used, size_p, type_p, base, left); ret = packfile_unpack_header1(&used, size_p, type_p, base, left);
git_mwindow_close(w_curs); git_mwindow_close(w_curs);
if (ret == GIT_EBUFS) if (ret == GIT_EBUFS)
return ret; return ret;
...@@ -799,9 +804,6 @@ void git_packfile_free(struct git_pack_file *p) ...@@ -799,9 +804,6 @@ void git_packfile_free(struct git_pack_file *p)
if (!p) if (!p)
return; return;
if (git_mutex_lock(&p->lock) < 0)
return;
cache_free(&p->bases); cache_free(&p->bases);
git_mwindow_free_all(&p->mwf); git_mwindow_free_all(&p->mwf);
...@@ -813,8 +815,6 @@ void git_packfile_free(struct git_pack_file *p) ...@@ -813,8 +815,6 @@ void git_packfile_free(struct git_pack_file *p)
git__free(p->bad_object_sha1); git__free(p->bad_object_sha1);
git_mutex_unlock(&p->lock);
git_mutex_free(&p->lock); git_mutex_free(&p->lock);
git__free(p); git__free(p);
} }
...@@ -829,12 +829,19 @@ static int packfile_open(struct git_pack_file *p) ...@@ -829,12 +829,19 @@ static int packfile_open(struct git_pack_file *p)
if (!p->index_map.data && pack_index_open(p) < 0) if (!p->index_map.data && pack_index_open(p) < 0)
return git_odb__error_notfound("failed to open packfile", NULL); return git_odb__error_notfound("failed to open packfile", NULL);
/* 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 (p->mwf.fd >= 0) {
git_mutex_unlock(&p->lock);
return 0;
}
/* TODO: open with noatime */ /* TODO: open with noatime */
p->mwf.fd = git_futils_open_ro(p->pack_name); p->mwf.fd = git_futils_open_ro(p->pack_name);
if (p->mwf.fd < 0) { if (p->mwf.fd < 0)
p->mwf.fd = -1; goto cleanup;
return -1;
}
if (p_fstat(p->mwf.fd, &st) < 0 || if (p_fstat(p->mwf.fd, &st) < 0 ||
git_mwindow_file_register(&p->mwf) < 0) git_mwindow_file_register(&p->mwf) < 0)
...@@ -875,13 +882,20 @@ static int packfile_open(struct git_pack_file *p) ...@@ -875,13 +882,20 @@ static int packfile_open(struct git_pack_file *p)
idx_sha1 = ((unsigned char *)p->index_map.data) + p->index_map.len - 40; idx_sha1 = ((unsigned char *)p->index_map.data) + p->index_map.len - 40;
if (git_oid__cmp(&sha1, (git_oid *)idx_sha1) == 0) if (git_oid__cmp(&sha1, (git_oid *)idx_sha1) != 0)
return 0; goto cleanup;
git_mutex_unlock(&p->lock);
return 0;
cleanup: cleanup:
giterr_set(GITERR_OS, "Invalid packfile '%s'", p->pack_name); giterr_set(GITERR_OS, "Invalid packfile '%s'", p->pack_name);
p_close(p->mwf.fd); p_close(p->mwf.fd);
p->mwf.fd = -1; p->mwf.fd = -1;
git_mutex_unlock(&p->lock);
return -1; return -1;
} }
......
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