@matheusbsbs / matheus-bagliano / commits / d7fcbe2c56

object-file: retry linking file into place when occluding file vanishes

Prior to 0ad3d65652 (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>

Patrick Steinhardt committed Jan 6, 2025 at 10:24 UTC d7fcbe2c56468ac780c689b02c6a9e056ce39c12

1 file changed +21 -4

object-file.c

+21 -4

@@ -1974,6 +1974,8 @@ static void write_object_file_prepare_literally(const struct git_hash_algo *algo

1974 hash_object_body(algo, &c, buf, len, oid, hdr, hdrlen);

1975 }

1976

1977 +#define CHECK_COLLISION_DEST_VANISHED -2

1978 +

1979 static int check_collision(const char *source, const char *dest)

1980 {

1981 char buf_source[4096], buf_dest[4096];

@@ -1990,6 +1992,8 @@ static int check_collision(const char *source, const char *dest)

1992 if (fd_dest < 0) {

1993 if (errno != ENOENT)

1994 ret = error_errno(_("unable to open %s"), dest);

1995 + else

1996 + ret = CHECK_COLLISION_DEST_VANISHED;

1997 goto out;

1998 }

1999

@@ -2037,8 +2041,11 @@ int finalize_object_file(const char *tmpfile, const char *filename)

2041 int finalize_object_file_flags(const char *tmpfile, const char *filename,

2042 enum finalize_object_file_flags flags)

2043 {

2040 - struct stat st;

2041 - int ret = 0;

2044 + unsigned retries = 0;

2045 + int ret;

2046 +

2047 +retry:

2048 + ret = 0;

2049

2050 if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)

2051 goto try_rename;

@@ -2059,6 +2066,8 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,

2066 * left to unlink.

2067 */

2068 if (ret && ret != EEXIST) {

2069 + struct stat st;

2070 +

2071 try_rename:

2072 if (!stat(filename, &st))

2073 ret = EEXIST;

@@ -2074,9 +2083,17 @@ int finalize_object_file_flags(const char *tmpfile, const char *filename,

2083 errno = saved_errno;

2084 return error_errno(_("unable to write file %s"), filename);

2085 }

2077 - if (!(flags & FOF_SKIP_COLLISION_CHECK) &&

2078 - check_collision(tmpfile, filename))

2086 + if (!(flags & FOF_SKIP_COLLISION_CHECK)) {

2087 + ret = check_collision(tmpfile, filename);

2088 + if (ret == CHECK_COLLISION_DEST_VANISHED) {

2089 + if (retries++ > 5)

2090 + return error(_("unable to write repeatedly vanishing file %s"),

2091 + filename);

2092 + goto retry;

2093 + }

2094 + else if (ret)

2095 return -1;

2096 + }

2097 unlink_or_warn(tmpfile);

2098 }

2099