]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib: file_lock_set_unlink_on_free() - Avoid unlink() if another process is waiting...
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Tue, 12 Sep 2017 11:54:57 +0000 (14:54 +0300)
committerAki Tuomi <aki.tuomi@dovecot.fi>
Wed, 13 Sep 2017 06:52:56 +0000 (09:52 +0300)
src/lib/file-create-locked.h
src/lib/file-lock.c
src/lib/file-lock.h

index cba44767074cc3402594c87ac90ff410611f7a16..7d21040347a96ead03e9c2d81761f1b2bfd4a618 100644 (file)
@@ -33,7 +33,12 @@ struct file_create_settings {
    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);
index 445caf8c7fdd98e2333fc7d2ff454657c5466d97..e86153ef335331bdc7c957f0cf67dc769352deec 100644 (file)
@@ -352,10 +352,20 @@ void file_lock_set_close_on_free(struct file_lock *lock, bool set)
        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,15 +374,42 @@ void file_unlock(struct file_lock **_lock)
           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);
+       }
+       if (temp_lock != NULL)
+               file_lock_free(&temp_lock);
+}
+
 void file_lock_free(struct file_lock **_lock)
 {
        struct file_lock *lock = *_lock;
@@ -380,7 +417,7 @@ void file_lock_free(struct file_lock **_lock)
        *_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);
 
index 022210600a0b0c7f7948792adbc2674c415a9936..e4d6eb19fe6ff13a2aeec80d407b81cfd17ce277 100644 (file)
@@ -45,8 +45,9 @@ int file_wait_lock_error(int fd, const char *path, int lock_type,
 /* 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. */