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.

diff --git a/ccache.c b/ccache.c
index db793edb1eb0fccc960e3fd09907ae8b65922a74..3f9be5da60a8dc8a3101f51bf0566be814eff844 100644 (file)
--- 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 f4413fccce6f93ce54f10762f82143cddc223519..54919f5c95d64b20c124606584447cbacb92e7a8 100644 (file)
--- 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 3bf9df90d19d9da79e0e0bc0f3803bb8699bac13..c5c93433a943b259b6bb1b5dade5bf83694dc29a 100644 (file)
--- 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<STATS_END;i++) {
-               len += snprintf(buf+len, sizeof(buf)-(len+1), "%u ", counters[i]);
-               if (len >= (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 3bc5542266c64b15fcb121ef9d02f7efc819500a..e69a615e5798295c4a4d2759fdf9c2381646292a 100644 (file)
--- 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)
 {