[PATCH 2/2] object-file: retry linking file into place when occluding file vanishes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



In a preceding commit, we have adapted `check_collision()` to ignore
the case where either of the colliding files vanishes. This should be
safe in general when we assume that the contents of these two files were
the same. But the check is all about detecting collisions, so that
assumption may be too optimistic.

Adapt the code to retry linking the object into place 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.

Furthermore, stop treating `ENOENT` specially for the source file. It
shouldn't happen that the source vanishes as we're using a fresh
temporary file for it, so if it does vanish it indicates an actual
error.

Suggested-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
---
 object-file.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/object-file.c b/object-file.c
index e1989236ca87e565dea4d003f57882f257889ecf..88432cc9c07c3c56ce31a298a0ee90e5b5acbaff 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1970,6 +1970,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];
@@ -1978,8 +1980,7 @@ static int check_collision(const char *source, const char *dest)
 
 	fd_source = open(source, O_RDONLY);
 	if (fd_source < 0) {
-		if (errno != ENOENT)
-			ret = error_errno(_("unable to open %s"), source);
+		ret = error_errno(_("unable to open %s"), source);
 		goto out;
 	}
 
@@ -1987,6 +1988,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;
 	}
 
@@ -2034,8 +2037,10 @@ 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;
+	int ret;
+
+retry:
+	ret = 0;
 
 	if (object_creation_mode == OBJECT_CREATION_USES_RENAMES)
 		goto try_rename;
@@ -2056,6 +2061,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;
@@ -2071,9 +2078,13 @@ 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)
+				goto retry;
+			else if (ret)
 				return -1;
+		}
 		unlink_or_warn(tmpfile);
 	}
 

-- 
2.48.0.rc1.241.g6c04ab211c.dirty





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux