[ccache] Corrupt objects from three colliding compiles

Wilson Snyder wsnyder at wsnyder.org
Thu May 6 14:22:40 MDT 2010


Two followups.

First, though not the problem I'm seeing, as I understand
it, mkstemp isn't guaranteed to work on NFS v2 mounted
drives.  Thus I suggest including the hostname; there's
already tmp_string() which is perfect.  Patch below.

Second, I'm still not sure of the cause, but if I count
bytes copied, and fail if zero bytes are moved this works
around the issue.  Patch below, but this not be the right
solution.  I thought of calling stat() after the copy, and
fail if the size doesn't match what stat says, alas
compression makes that difficult.

One thing I wonder is the unlink() before the rename.  I
presume that is needed to prevent permission issues, but I
wonder if it contributes to the race involving
host1:unlink(), host1:rename(), host2:unlink(),
host2:rename(), host3:open().

-Wilson


diff --git a/util.c b/util.c
index ae48cf0..df522f1 100644
--- a/util.c
+++ b/util.c
@@ -128,7 +128,7 @@ int copy_file(const char *src, const char *dest, int compress_dest)
   mode_t mask;
   struct stat st;
 
-	x_asprintf(&tmp_name, "%s.XXXXXX", dest);
+	x_asprintf(&tmp_name, "%s.%s.XXXXXX", dest, tmp_string());
 
	/* open source file */
 	fd_in = open(src, O_RDONLY);


diff --git a/util.c b/util.c
index ae48cf0..df522f1 100644
--- a/util.c
+++ b/util.c
@@ -177,7 +177,9 @@ int copy_file(const char *src, const char *dest, int compress_dest)
   	  }
 	  }
 
+	int bytecnt = 0;
 	while ((n = gzread(gz_in, buf, sizeof(buf))) > 0) {
+	      bytecnt += n;
 	      	      if (compress_dest) {
 		      	 		 ret = gzwrite(gz_out, buf, n);
 					     } else {
@@ -212,14 +214,25 @@ int copy_file(const char *src, const char *dest, int compress_dest)
   	   return -1;
 	   }
 
+	/* Empty objects are bad - created by race in NFS between 2 writers and 1+ readers */
+	if (bytecnt == 0) {
+	    unlink(tmp_name);
+	        free(tmp_name);
+		    cc_log("Refusing to copy zero sized file %s", src);
+		        return -1;
+			}
+
	unlink(dest);
 
	if (rename(tmp_name, dest) == -1) {
 	   unlink(tmp_name);
		free(tmp_name);
+			cc_log("Rename failed");
 				       return -1;
 				       }
 
+	cc_log("Copied %d bytes from %s via %s to %s", bytecnt, src, tmp_name, dest);
+
	free(tmp_name);
 
	return 0;


More information about the ccache mailing list