changeset 22501:99a2b945045f

lib: file_lock_set_unlink_on_free() - Avoid unlink() if another process is waiting on the lock
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Tue, 12 Sep 2017 14:54:57 +0300
parents bf1220873408
children fc335480acb3
files src/lib/file-create-locked.h src/lib/file-lock.c src/lib/file-lock.h
diffstat 3 files changed, 54 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib/file-create-locked.h	Wed Aug 30 20:49:17 2017 -0400
+++ b/src/lib/file-create-locked.h	Tue Sep 12 14:54:57 2017 +0300
@@ -33,7 +33,12 @@
    If link() fails, opening is retried again. Returns fd on success,
    -1 on error. errno is preserved for the last failed syscall, so most
    importantly ENOENT could mean that the directory doesn't exist and EAGAIN
-   means locking timed out. */
+   means locking timed out.
+
+   If this function is used to create lock files, file_lock_set_unlink_on_free()
+   should be used for the resulting lock. It attempts to avoid unlinking the
+   file if there are already other processes using the lock. That can help to
+   avoid "Creating a locked file ... keeps failing" errors */
 int file_create_locked(const char *path, const struct file_create_settings *set,
 		       struct file_lock **lock_r, bool *created_r,
 		       const char **error_r);
--- a/src/lib/file-lock.c	Wed Aug 30 20:49:17 2017 -0400
+++ b/src/lib/file-lock.c	Tue Sep 12 14:54:57 2017 +0300
@@ -352,10 +352,20 @@
 	lock->close_on_free = set;
 }
 
+static void file_unlock_real(struct file_lock *lock)
+{
+	const char *error;
+
+	if (file_lock_do(lock->fd, lock->path, F_UNLCK,
+			 lock->lock_method, 0, &error) == 0) {
+		/* this shouldn't happen */
+		i_error("file_unlock(%s) failed: %m", lock->path);
+	}
+}
+
 void file_unlock(struct file_lock **_lock)
 {
 	struct file_lock *lock = *_lock;
-	const char *error;
 
 	*_lock = NULL;
 
@@ -364,13 +374,40 @@
 	   could be deleting the new lock. */
 	i_assert(!lock->unlink_on_free);
 
-	if (file_lock_do(lock->fd, lock->path, F_UNLCK,
-			 lock->lock_method, 0, &error) == 0) {
-		/* this shouldn't happen */
-		i_error("file_unlock(%s) failed: %m", lock->path);
+	file_unlock_real(lock);
+	file_lock_free(&lock);
+}
+
+static void file_try_unlink_locked(struct file_lock *lock)
+{
+	struct file_lock *temp_lock = NULL;
+	struct stat st1, st2;
+	const char *error;
+	int ret;
+
+	file_unlock_real(lock);
+	ret = file_try_lock_error(lock->fd, lock->path, F_WRLCK,
+				  lock->lock_method, &temp_lock, &error);
+	if (ret < 0) {
+		i_error("file_lock_free(): Unexpectedly failed to retry locking %s: %s",
+			lock->path, error);
+	} else if (ret == 0) {
+		/* already locked by someone else */
+	} else if (fstat(lock->fd, &st1) < 0) {
+		/* not expected to happen */
+		i_error("file_lock_free(): fstat(%s) failed: %m", lock->path);
+	} else if (stat(lock->path, &st2) < 0) {
+		if (errno != ENOENT)
+			i_error("file_lock_free(): stat(%s) failed: %m", lock->path);
+	} else if (st1.st_ino != st2.st_ino ||
+		   !CMP_DEV_T(st1.st_dev, st2.st_dev)) {
+		/* lock file was recreated already - don't delete it */
+	} else {
+		/* nobody was waiting on the lock - unlink it */
+		i_unlink(lock->path);
 	}
-
-	file_lock_free(&lock);
+	if (temp_lock != NULL)
+		file_lock_free(&temp_lock);
 }
 
 void file_lock_free(struct file_lock **_lock)
@@ -380,7 +417,7 @@
 	*_lock = NULL;
 
 	if (lock->unlink_on_free)
-		i_unlink(lock->path);
+		file_try_unlink_locked(lock);
 	if (lock->close_on_free)
 		i_close_fd(&lock->fd);
 
--- a/src/lib/file-lock.h	Wed Aug 30 20:49:17 2017 -0400
+++ b/src/lib/file-lock.h	Tue Sep 12 14:54:57 2017 +0300
@@ -45,8 +45,9 @@
 /* Change the lock type. WARNING: This isn't an atomic operation!
    The result is the same as file_unlock() + file_try_lock(). */
 int file_lock_try_update(struct file_lock *lock, int lock_type);
-/* When the lock is freed, unlink() the file automatically. This can be useful
-   for files that are only created to exist as lock files. */
+/* When the lock is freed, unlink() the file automatically, unless other
+   processes are already waiting on the lock. This can be useful for files that
+   are only created to exist as lock files. */
 void file_lock_set_unlink_on_free(struct file_lock *lock, bool set);
 /* When the lock is freed, close the fd automatically. This can
    be useful for files that are only created to exist as lock files. */