From 7daab1e4c204de630675a95d8c05dfc4d5a106ea Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Sun, 1 Aug 2010 16:06:09 +0200 Subject: [PATCH] Use lock files instead of POSIX locks when writing stats files When writing a stats file, locking is now done by atomically creating a symlink with lockfile_acquire() instead of locking with fcntl(). Updating of the stats file while holding the lock is done using the rename-into-place idiom since the lock may be intentionally broken by another process, and if that happens, there should be no file corruption, only lost information. The major reason for the change is to be more robust against badly implemented or configured network filesystems (e.g. NFS) where POSIX locks may be broken in different ways. --- ccache.c | 3 ++ ccache.h | 2 - stats.c | 136 ++++++++++++++++++++++++++++++++----------------------- util.c | 34 -------------- 4 files changed, 83 insertions(+), 92 deletions(-) diff --git a/ccache.c b/ccache.c index db793edb1..3f9be5da6 100644 --- a/ccache.c +++ b/ccache.c @@ -188,6 +188,9 @@ static int nlevels = 2; */ static int compile_preprocessed_source_code; +/* How long (in microseconds) to wait before breaking a stale lock. */ +unsigned lock_staleness_limit = 2000000; + /* * Supported file extensions and corresponding languages (as in parameter to * the -x option). diff --git a/ccache.h b/ccache.h index f4413fccc..54919f5c9 100644 --- a/ccache.h +++ b/ccache.h @@ -105,8 +105,6 @@ char *basename(const char *s); char *dirname(char *s); const char *get_extension(const char *path); char *remove_extension(const char *path); -int read_lock_fd(int fd); -int write_lock_fd(int fd); size_t file_size(struct stat *st); int safe_open(const char *fname); char *x_realpath(const char *path); diff --git a/stats.c b/stats.c index 3bf9df90d..c5c93433a 100644 --- a/stats.c +++ b/stats.c @@ -35,6 +35,7 @@ extern char *stats_file; extern char *cache_dir; +extern unsigned lock_staleness_limit; static unsigned counter_updates[STATS_END]; @@ -105,24 +106,29 @@ static void parse_stats(unsigned counters[STATS_END], char *buf) } /* write out a stats file */ -static void write_stats(int fd, unsigned counters[STATS_END]) +static void +write_stats(const char *path, unsigned counters[STATS_END]) { int i; - int len = 0; - char buf[1024]; + char *tmp_file; + FILE *f; - for (i=0;i= (int)sizeof(buf)-1) fatal("stats too long"); + tmp_file = format("%s.tmp.%s", path, tmp_string()); + f = fopen(tmp_file, "wb"); + if (!f) { + cc_log("Failed to open %s", tmp_file); + return; } - len += snprintf(buf+len, sizeof(buf)-(len+1), "\n"); - if (len >= (int)sizeof(buf)-1) fatal("stats too long"); - - lseek(fd, 0, SEEK_SET); - if (write(fd, buf, len) == -1) fatal("Could not write stats"); + for (i = 0; i < STATS_END; i++) { + if (fprintf(f, "%u ", counters[i]) < 0) { + fatal("Failed to write to %s", tmp_file); + } + } + fputs("\n", f); + fclose(f); + x_rename(tmp_file, path); } - /* fill in some default stats values */ static void stats_default(unsigned counters[STATS_END]) { @@ -191,19 +197,23 @@ void stats_flush(void) free(stats_dir); } - fd = safe_open(stats_file); - if (fd == -1) return; - if (write_lock_fd(fd) != 0) return; - memset(counters, 0, sizeof(counters)); - stats_read_fd(fd, counters); + if (!lockfile_acquire(stats_file, lock_staleness_limit)) { + return; + } + fd = open(stats_file, O_RDONLY|O_BINARY); + if (fd == -1) { + stats_default(counters); + } else { + stats_read_fd(fd, counters); + close(fd); + } for (i = 0; i < STATS_END; ++i) { counters[i] += counter_updates[i]; } - - write_stats(fd, counters); - close(fd); + write_stats(stats_file, counters); + lockfile_release(stats_file); if (counters[STATS_MAXFILES] != 0 && counters[STATS_NUMFILES] > counters[STATS_MAXFILES]) { @@ -234,18 +244,17 @@ unsigned stats_get_pending(enum stats stat) } /* read in the stats from one dir and add to the counters */ -void stats_read(const char *stats_file, unsigned counters[STATS_END]) +void stats_read(const char *path, unsigned counters[STATS_END]) { int fd; - fd = open(stats_file, O_RDONLY|O_BINARY); + fd = open(path, O_RDONLY|O_BINARY); if (fd == -1) { stats_default(counters); - return; + } else { + stats_read_fd(fd, counters); + close(fd); } - read_lock_fd(fd); - stats_read_fd(fd, counters); - close(fd); } /* sum and display the total stats for all cache dirs */ @@ -311,26 +320,29 @@ void stats_zero(void) for (dir=0;dir<=0xF;dir++) { fname = format("%s/%1x/stats", cache_dir, dir); - fd = safe_open(fname); - if (fd == -1) { + memset(counters, 0, sizeof(counters)); + if (!lockfile_acquire(fname, lock_staleness_limit)) { free(fname); continue; } - memset(counters, 0, sizeof(counters)); - write_lock_fd(fd); - stats_read_fd(fd, counters); - for (i=0;stats_info[i].message;i++) { + fd = open(fname, O_RDONLY|O_BINARY); + if (fd == -1) { + stats_default(counters); + } else { + stats_read_fd(fd, counters); + close(fd); + } + for (i = 0; stats_info[i].message; i++) { if (!(stats_info[i].flags & FLAG_NOZERO)) { counters[stats_info[i].stat] = 0; } } - write_stats(fd, counters); - close(fd); + write_stats(fname, counters); + lockfile_release(fname); free(fname); } } - /* set the per directory limits */ int stats_set_limits(long maxfiles, long maxsize) { @@ -349,7 +361,7 @@ int stats_set_limits(long maxfiles, long maxsize) } /* set the limits in each directory */ - for (dir=0;dir<=0xF;dir++) { + for (dir = 0; dir <= 0xF; dir++) { char *fname, *cdir; int fd; @@ -360,20 +372,26 @@ int stats_set_limits(long maxfiles, long maxsize) fname = format("%s/stats", cdir); free(cdir); + if (!lockfile_acquire(fname, lock_staleness_limit)) { + free(fname); + continue; + } + fd = open(fname, O_RDONLY|O_BINARY); memset(counters, 0, sizeof(counters)); - fd = safe_open(fname); - if (fd != -1) { - write_lock_fd(fd); + if (fd == -1) { + stats_default(counters); + } else { stats_read_fd(fd, counters); - if (maxfiles != -1) { - counters[STATS_MAXFILES] = maxfiles; - } - if (maxsize != -1) { - counters[STATS_MAXSIZE] = maxsize; - } - write_stats(fd, counters); close(fd); } + if (maxfiles != -1) { + counters[STATS_MAXFILES] = maxfiles; + } + if (maxsize != -1) { + counters[STATS_MAXSIZE] = maxsize; + } + write_stats(fname, counters); + lockfile_release(fname); free(fname); } @@ -385,22 +403,28 @@ void stats_set_sizes(const char *dir, size_t num_files, size_t total_size) { int fd; unsigned counters[STATS_END]; - char *stats_file; + char *statsfile; create_dir(dir); - stats_file = format("%s/stats", dir); + statsfile = format("%s/stats", dir); memset(counters, 0, sizeof(counters)); - fd = safe_open(stats_file); - if (fd != -1) { - write_lock_fd(fd); + if (!lockfile_acquire(statsfile, lock_staleness_limit)) { + free(statsfile); + return; + } + fd = safe_open(statsfile); + if (fd == -1) { + stats_default(counters); + } else { stats_read_fd(fd, counters); - counters[STATS_NUMFILES] = num_files; - counters[STATS_TOTALSIZE] = total_size; - write_stats(fd, counters); close(fd); } - - free(stats_file); + counters[STATS_NUMFILES] = num_files; + counters[STATS_TOTALSIZE] = total_size; + close(fd); + write_stats(statsfile, counters); + lockfile_release(statsfile); + free(statsfile); } diff --git a/util.c b/util.c index 3bc554226..e69a615e5 100644 --- a/util.c +++ b/util.c @@ -681,40 +681,6 @@ char *remove_extension(const char *path) return x_strndup(path, strlen(path) - strlen(get_extension(path))); } -static int lock_fd(int fd, short type) -{ -#ifdef _WIN32 - (void) type; - return _locking(fd, _LK_NBLCK, 1); -#else - struct flock fl; - int ret; - - fl.l_type = type; - fl.l_whence = SEEK_SET; - fl.l_start = 0; - fl.l_len = 1; - fl.l_pid = 0; - - /* not sure why we would be getting a signal here, - but one user claimed it is possible */ - do { - ret = fcntl(fd, F_SETLKW, &fl); - } while (ret == -1 && errno == EINTR); - return ret; -#endif -} - -int read_lock_fd(int fd) -{ - return lock_fd(fd, F_RDLCK); -} - -int write_lock_fd(int fd) -{ - return lock_fd(fd, F_WRLCK); -} - /* return size on disk of a file */ size_t file_size(struct stat *st) { -- 2.47.3