Re: Protecting old temporary objects being reused from concurrent "git gc"?
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 15, 2016, 20:01 UTC
- Message-ID
- <xmqqk2c4tsv4.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <20161115174028.zvohfcw4jse3jrmm@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 5 quoted lines
> I suspect the issue is that read-tree populates the cache-tree index > extension, and then write-tree omits the object write before it even > gets to write_sha1_file(). The solution is that it should probably be > calling one of the freshen() functions (possibly just replacing > has_sha1_file() with check_and_freshen(), but I haven't looked).
I think the final writing always happens via write_sha1_file(), but an earlier cache-tree update that says "if we have a tree object already, then use it, otherwise even though we know the object name for this subtree, do not record it in the cache-tree" codepath may decide to record the subtree's sha1 without refreshing the referent.
A fix may look like this.
cache-tree.c | 2 +- cache.h | 1 + sha1_file.c | 9 +++++++-- 3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/cache-tree.c b/cache-tree.c index 345ea35963..3ae6d056b4 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -401,7 +401,7 @@ static int update_one(struct cache_tree *it, if (repair) { unsigned char sha1[20]; hash_sha1_file(buffer.buf, buffer.len, tree_type, sha1); - if (has_sha1_file(sha1)) + if (freshen_object(sha1)) hashcpy(it->sha1, sha1); else to_invalidate = 1; diff --git a/cache.h b/cache.h index 5cdea6833e..1f5694f308 100644 --- a/cache.h +++ b/cache.h @@ -1126,6 +1126,7 @@ extern int sha1_object_info(const unsigned char *, unsigned long *); extern int hash_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1); extern int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *return_sha1); extern int hash_sha1_file_literally(const void *buf, unsigned long len, const char *type, unsigned char *sha1, unsigned flags); +extern int freshen_object(const unsigned char *); extern int pretend_sha1_file(void *, unsigned long, enum object_type, unsigned char *); extern int force_object_loose(const unsigned char *sha1, time_t mtime); extern int git_open_cloexec(const char *name, int flags); diff --git a/sha1_file.c b/sha1_file.c index e030805497..1daeb05dcd 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -3275,6 +3275,11 @@ static int freshen_packed_object(const unsigned char *sha1) return 1; } +int freshen_object(const unsigned char *sha1) +{ + return freshen_packed_object(sha1) || freshen_loose_object(sha1); +} + int write_sha1_file(const void *buf, unsigned long len, const char *type, unsigned char *sha1) { char hdr[32]; @@ -3284,7 +3289,7 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign * it out into .git/objects/??/?{38} file. */ write_sha1_file_prepare(buf, len, type, sha1, hdr, &hdrlen); - if (freshen_packed_object(sha1) || freshen_loose_object(sha1)) + if (freshen_object(sha1)) return 0; return write_loose_object(sha1, hdr, hdrlen, buf, len, 0); } @@ -3302,7 +3307,7 @@ int hash_sha1_file_literally(const void *buf, unsigned long len, const char *typ if (!(flags & HASH_WRITE_OBJECT)) goto cleanup; - if (freshen_packed_object(sha1) || freshen_loose_object(sha1)) + if (freshen_object(sha1)) goto cleanup; status = write_loose_object(sha1, header, hdrlen, buf, len, 0);