Skip to content

Commit

Permalink
Do not rely on pids being unique
Browse files Browse the repository at this point in the history
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."
  • Loading branch information
jrosdahl committed Nov 13, 2014
1 parent 1df9109 commit a07f46a
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 19 deletions.
22 changes: 14 additions & 8 deletions ccache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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 */
Expand All @@ -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;
}

Expand Down
2 changes: 1 addition & 1 deletion ccache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion manifest.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
10 changes: 8 additions & 2 deletions stats.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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:
Expand Down
18 changes: 11 additions & 7 deletions util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -418,16 +418,16 @@ 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)
{
static char *ret;

if (!ret) {
ret = format("%s.%u", get_hostname(), (unsigned)getpid());
ret = format("%s.%u.XXXXXX", get_hostname(), (unsigned)getpid());
}

return ret;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit a07f46a

Please sign in to comment.