Simplify lock breaking

Acquiring a .lock.lock file does not make lock breaking significantly
safer; there is still a race condition. Simplify it by simply deleting
the lockfile when breaking and try to acquire it again.

diff --git a/src/lockfile.cpp b/src/lockfile.cpp
index 1faef4f15e09eede93f52c06c690f6ce9340b6de..afd3f8f3d6d367ace62031ee04a6d05efbb9a505 100644 (file)
--- a/src/lockfile.cpp
+++ b/src/lockfile.cpp
@@ -101,16 +101,15 @@ lockfile_acquire(const char* path, unsigned staleness_limit)
     }
     if (slept > staleness_limit) {
       if (str_eq(content, initial_content)) {
-        // The lock seems to be stale -- break it.
+        // The lock seems to be stale -- break it and try again.
         cc_log("lockfile_acquire: breaking %s", lockfile);
-        // Try to acquire path.lock.lock:
-        if (lockfile_acquire(lockfile, staleness_limit)) {
-          lockfile_release(path);     // Remove path.lock
-          lockfile_release(lockfile); // Remove path.lock.lock
-          to_sleep = 1000;
-          slept = 0;
-          continue;
+        if (tmp_unlink(lockfile) != 0) {
+          cc_log("Failed to unlink %s: %s", lockfile, strerror(errno));
+          goto out;
         }
+        to_sleep = 1000;
+        slept = 0;
+        continue;
       }
       cc_log("lockfile_acquire: gave up acquiring %s", lockfile);
       goto out;
@@ -143,6 +142,9 @@ lockfile_release(const char* path)
 {
   char* lockfile = format("%s.lock", path);
   cc_log("Releasing lock %s", lockfile);
+  if (tmp_unlink(lockfile) != 0) {
+    cc_log("Failed to unlink %s: %s", lockfile, strerror(errno));
+  }
   tmp_unlink(lockfile);
   free(lockfile);
 }

diff --git a/unittest/test_lockfile.cpp b/unittest/test_lockfile.cpp
index 7ad4a2b7d379722608bf2fb9e23547e4cf620f0c..e66ab5aaf0bdebafac6f1fa96140b78cc4d6db6b 100644 (file)
--- a/unittest/test_lockfile.cpp
+++ b/unittest/test_lockfile.cpp
@@ -52,10 +52,8 @@ TEST(lock_breaking)
 
 #if defined(_WIN32) || defined(__CYGWIN__)
   create_file("test.lock", "foo");
-  create_file("test.lock.lock", "foo");
 #else
   CHECK_INT_EQ(0, symlink("foo", "test.lock"));
-  CHECK_INT_EQ(0, symlink("foo", "test.lock.lock"));
 #endif
   CHECK(lockfile_acquire("test", 1000));
 
@@ -66,7 +64,6 @@ TEST(lock_breaking)
 #endif
   CHECK(p);
   CHECK(!str_eq(p, "foo"));
-  CHECK(!path_exists("test.lock.lock"));
 
   free(p);
 }