Skip to content

Commit

Permalink
object-file: retry linking file into place when occluding file vanishes
Browse files Browse the repository at this point in the history
Prior to 0ad3d65 (object-file: fix race in object collision check,
2024-12-30), callers could expect that a successful return from
`finalize_object_file()` means that either the file was moved into
place, or the identical bytes were already present. If neither of those
happens, we'd return an error.

Since that commit, if the destination file disappears between our
link(3p) call and the collision check, we'd return success without
actually checking the contents, and without retrying the link. This
solves the common case that the files were indeed the same, but it means
that we may corrupt the repository if they weren't (this implies a hash
collision, but the whole point of this function is protecting against
hash collisions).

We can't be pessimistic and assume they're different; that hurts the
common case that the mentioned commit was trying to fix. But after
seeing that the destination file went away, we can retry linking again.
Adapt the code to do so when we see that the destination file has racily
vanished. This should generally succeed as we have just observed that
the destination file does not exist anymore, except in the very unlikely
event that it gets recreated by another concurrent process again.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
pks-t authored and gitster committed Jan 6, 2025
1 parent cfae50e commit d7fcbe2
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions object-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -1974,6 +1974,8 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo
hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);
}

#define CHECK_COLLISION_DEST_VANISHED -2

static int check_collision(const char *source, const char *dest)
{
char buf_source[4096], buf_dest[4096];
Expand All @@ -1990,6 +1992,8 @@ static int check_collision(const char *source, const char *dest)
if (fd_dest < 0) {
if (errno != ENOENT)
ret = error_errno(_("unable to open %s"), dest);
else
ret = CHECK_COLLISION_DEST_VANISHED;
goto out;
}

Expand Down Expand Up @@ -2037,8 +2041,11 @@ int finalize_object_file(const char *tmpfile, const char *filename)
int finalize_object_file_flags(const char *tmpfile, const char *filename,
enum finalize_object_file_flags flags)
{
struct stat st;
int ret = 0;
unsigned retries = 0;
int ret;

retry:
ret = 0;

if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
goto try_rename;
Expand All @@ -2059,6 +2066,8 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,
* left to unlink.
*/
if (ret && ret != EEXIST) {
struct stat st;

try_rename:
if (!stat(filename, &st))
ret = EEXIST;
Expand All @@ -2074,9 +2083,17 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,
errno = saved_errno;
return error_errno(_("unable to write file %s"), filename);
}
if (!(flags & FOF_SKIP_COLLISION_CHECK) &&
check_collision(tmpfile, filename))
if (!(flags & FOF_SKIP_COLLISION_CHECK)) {
ret = check_collision(tmpfile, filename);
if (ret == CHECK_COLLISION_DEST_VANISHED) {
if (retries++ > 5)
return error(_("unable to write repeatedly vanishing file %s"),
filename);
goto retry;
}
else if (ret)
return -1;
}
unlink_or_warn(tmpfile);
}

Expand Down

0 comments on commit d7fcbe2

Please sign in to comment.