Do not rely on pids being unique

Based on a patch by Mike Frysinger <vapier@gentoo.org>:

"Linux supports creating pid namespaces cheaply and running processes
inside of them. When you try to share a single cache among multiple such
runs, the fact that the code relies on pid numbers as globally unique
values quickly fails. Instead, switch to standard mkstemp to generate temp
files for us."

diff --git a/ccache.c b/ccache.c
index f0dca22c7fc76b76ba716c3268241b328c6de5ef..7b9fd0b0435b6dc5e46c90c22f61b3df420f3b7c 100644 (file)
--- a/ccache.c
+++ b/ccache.c
@@ -613,8 +613,11 @@ to_cache(struct args *args)
        unsigned added_files = 0;
 
        tmp_stdout = format("%s.tmp.stdout.%s", cached_obj, tmp_string());
+       create_empty_file(tmp_stdout);
        tmp_stderr = format("%s.tmp.stderr.%s", cached_obj, tmp_string());
+       create_empty_file(tmp_stderr);
        tmp_obj = format("%s.tmp.%s", cached_obj, tmp_string());
+       create_empty_file(tmp_obj);
 
        args_add(args, "-o");
        args_add(args, tmp_obj);
@@ -666,7 +669,7 @@ to_cache(struct args *args)
                int fd_result;
                char *tmp_stderr2;
 
-               tmp_stderr2 = format("%s.tmp.stderr2.%s", cached_obj, tmp_string());
+               tmp_stderr2 = format("%s.2", tmp_stderr);
                if (x_rename(tmp_stderr, tmp_stderr2)) {
                        cc_log("Failed to rename %s to %s: %s", tmp_stderr, tmp_stderr2,
                               strerror(errno));
@@ -808,13 +811,21 @@ get_object_name_from_cpp(struct args *args, struct mdfour *hash)
        }
 
        path_stderr = format("%s/tmp.cpp_stderr.%s", temp_dir, tmp_string());
+       create_empty_file(path_stderr);
        add_pending_tmp_file(path_stderr);
 
        time_of_compilation = time(NULL);
 
        if (!direct_i_file) {
-               path_stdout = format("%s/%s.tmp.%s.%s",
-                                    temp_dir, input_base, tmp_string(), i_extension);
+               /* The temporary file needs the proper i_extension for the compiler to do
+                * its thing. However, create_empty_file requires the tmp_string() part to
+                * be last, which is why the temporary file is created in two steps. */
+               char *path_stdout_tmp =
+                       format("%s/%s.tmp.%s", temp_dir, input_base, tmp_string());
+               create_empty_file(path_stdout_tmp);
+               path_stdout = format("%s.%s", path_stdout_tmp, i_extension);
+               x_rename(path_stdout_tmp, path_stdout);
+               free(path_stdout_tmp);
                add_pending_tmp_file(path_stdout);
 
                /* run cpp on the input file to obtain the .i */
@@ -827,11 +838,6 @@ get_object_name_from_cpp(struct args *args, struct mdfour *hash)
                   can skip the cpp stage and directly form the
                   correct i_tmpfile */
                path_stdout = input_file;
-               if (create_empty_file(path_stderr) != 0) {
-                       cc_log("Failed to create %s: %s", path_stderr, strerror(errno));
-                       stats_update(STATS_ERROR);
-                       failed();
-               }
                status = 0;
        }
 

diff --git a/ccache.h b/ccache.h
index ce4d1cf5779c749a2235c7be7677c0e90bdbba3a..adcc381609556f9f496b8f6e67fb190e21f622ef 100644 (file)
--- a/ccache.h
+++ b/ccache.h
@@ -133,7 +133,7 @@ size_t file_size(struct stat *st);
 int safe_open(const char *fname);
 char *x_realpath(const char *path);
 char *gnu_getcwd(void);
-int create_empty_file(const char *fname);
+int create_empty_file(char *fname);
 const char *get_home_directory(void);
 char *get_cwd();
 bool same_executable_name(const char *s1, const char *s2);

diff --git a/manifest.c b/manifest.c
index 7977418b0e3eee9c2d7f2504f7b485bdec014f01..660d437f6e515d1dec82eb235bee1f24dc018f74 100644 (file)
--- a/manifest.c
+++ b/manifest.c
@@ -653,7 +653,7 @@ manifest_put(const char *manifest_path, struct file_hash *object_hash,
        }
 
        tmp_file = format("%s.tmp.%s", manifest_path, tmp_string());
-       fd2 = safe_open(tmp_file);
+       fd2 = mkstemp(tmp_file);
        if (fd2 == -1) {
                cc_log("Failed to open %s", tmp_file);
                goto out;

diff --git a/stats.c b/stats.c
index d7321265ce6952fc08196cbf9746a17ac010e92e..76cea8355a9aa3192bd1a7c862528a35f0bad5b0 100644 (file)
--- a/stats.c
+++ b/stats.c
@@ -126,9 +126,15 @@ stats_write(const char *path, struct counters *counters)
        size_t i;
        char *tmp_file;
        FILE *f;
+       int fd;
 
        tmp_file = format("%s.tmp.%s", path, tmp_string());
-       f = fopen(tmp_file, "wb");
+       fd = mkstemp(tmp_file);
+       if (fd == -1) {
+               cc_log("Failed to open %s", tmp_file);
+               goto end;
+       }
+       f = fdopen(fd, "wb");
        if (!f) {
                cc_log("Failed to open %s", tmp_file);
                goto end;
@@ -138,7 +144,7 @@ stats_write(const char *path, struct counters *counters)
                        fatal("Failed to write to %s", tmp_file);
                }
        }
-       fclose(f);
+       fclose(f); /* This also implicitly closes the fd. */
        x_rename(tmp_file, path);
 
 end:

diff --git a/util.c b/util.c
index ed470d2c3d376b1a46bf0eaafcf52f1f88039974..4c18e1576b20f78eab6c3f198e7cf7f191a3276e 100644 (file)
--- a/util.c
+++ b/util.c
@@ -195,7 +195,7 @@ copy_file(const char *src, const char *dest, int compress_dest)
        struct stat st;
        int errnum;
 
-       tmp_name = format("%s.%s.XXXXXX", dest, tmp_string());
+       tmp_name = format("%s.%s", dest, tmp_string());
        cc_log("Copying %s to %s via %s (%s)",
               src, dest, tmp_name, compress_dest ? "compressed": "uncompressed");
 
@@ -418,8 +418,8 @@ get_hostname(void)
 }
 
 /*
- * Return a string to be used to distinguish temporary files. Also tries to
- * cope with NFS by adding the local hostname.
+ * Return a string to be passed to mkstemp to create a temporary file. Also
+ * tries to cope with NFS by adding the local hostname.
  */
 const char *
 tmp_string(void)
@@ -427,7 +427,7 @@ tmp_string(void)
        static char *ret;
 
        if (!ret) {
-               ret = format("%s.%u", get_hostname(), (unsigned)getpid());
+               ret = format("%s.%u.XXXXXX", get_hostname(), (unsigned)getpid());
        }
 
        return ret;
@@ -886,12 +886,13 @@ gnu_getcwd(void)
 
 /* create an empty file */
 int
-create_empty_file(const char *fname)
+create_empty_file(char *fname)
 {
        int fd;
 
-       fd = open(fname, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL|O_BINARY, 0666);
+       fd = mkstemp(fname);
        if (fd == -1) {
+               cc_log("Failed to create %s: %s", fname, strerror(errno));
                return -1;
        }
        close(fd);
@@ -1126,7 +1127,10 @@ x_unlink(const char *path)
                goto out;
        }
        if (unlink(tmp_name) == -1) {
-               result = -1;
+               /* If it was released in a race, that's OK. */
+               if (errno != ENOENT) {
+                       result = -1;
+               }
        }
 out:
        free(tmp_name);