From de410701811f4142315ce89183256a969a08ff9d Mon Sep 17 00:00:00 2001
From: JINNOUCHI Yasushi <me@delphinus.dev>
Date: Sun, 3 Dec 2023 12:55:37 +0900
Subject: [PATCH] fix: get file lock forcibly (#153)

* fix: get file lock forcibly

When Neovim ends without removing file locks, it will fail to open
frecency ever because file locks remain. This fix makes it remove file
locks when all attempts fail forcibly.

* test: avoid error in Windows

In Windows, unclosed file cannot be removed, so close it before
unlinking.

* feat: retry to unlink when it fails
---
 lua/frecency/file_lock.lua            | 18 ++++++--
 lua/frecency/tests/file_lock_spec.lua | 59 ++++++++++++++++++++-------
 2 files changed, 59 insertions(+), 18 deletions(-)

diff --git a/lua/frecency/file_lock.lua b/lua/frecency/file_lock.lua
index ed4cb05..2294f4b 100644
--- a/lua/frecency/file_lock.lua
+++ b/lua/frecency/file_lock.lua
@@ -9,13 +9,14 @@ local FileLock = {}
 
 ---@class FrecencyFileLockConfig
 ---@field retry integer default: 5
+---@field unlink_retry integer default: 5
 ---@field interval integer default: 500
 
 ---@param path string
 ---@param opts FrecencyFileLockConfig?
 ---@return FrecencyFileLock
 FileLock.new = function(path, opts)
-  local config = vim.tbl_extend("force", { retry = 5, interval = 500 }, opts or {})
+  local config = vim.tbl_extend("force", { retry = 5, unlink_retry = 5, interval = 500 }, opts or {})
   local self = setmetatable({ config = config }, { __index = FileLock })
   self.filename = path .. ".lock"
   return self
@@ -25,6 +26,7 @@ end
 ---@return string? err
 function FileLock:get()
   local count = 0
+  local unlink_count = 0
   local err, fd
   while true do
     count = count + 1
@@ -33,9 +35,17 @@ function FileLock:get()
       break
     end
     async.util.sleep(self.config.interval)
-    if count == self.config.retry then
-      log.debug(("file_lock get() failed: retry count reached: %d"):format(count))
-      return "failed to get lock"
+    if count >= self.config.retry then
+      log.debug(("file_lock get(): retry count reached. try to delete the lock file: %d"):format(count))
+      err = async.uv.fs_unlink(self.filename)
+      if err then
+        log.debug("file_lock get() failed: " .. err)
+        unlink_count = unlink_count + 1
+        if unlink_count >= self.config.unlink_retry then
+          log.error("file_lock get(): failed to unlink the lock file: " .. err)
+          return "failed to get lock"
+        end
+      end
     end
     log.debug(("file_lock get() retry: %d"):format(count))
   end
diff --git a/lua/frecency/tests/file_lock_spec.lua b/lua/frecency/tests/file_lock_spec.lua
index c6eba26..34c0d24 100644
--- a/lua/frecency/tests/file_lock_spec.lua
+++ b/lua/frecency/tests/file_lock_spec.lua
@@ -1,3 +1,4 @@
+---@diagnostic disable: undefined-field
 local FileLock = require "frecency.file_lock"
 local util = require "frecency.tests.util"
 local async = require "plenary.async" --[[@as PlenaryAsync]]
@@ -10,6 +11,18 @@ local function with_dir(f)
   close()
 end
 
+local function with_unlink_fails(f)
+  return function()
+    local original = async.uv.fs_unlink
+    ---@diagnostic disable-next-line: duplicate-set-field
+    async.uv.fs_unlink = function()
+      return "overwritten"
+    end
+    f()
+    async.uv.fs_unlink = original
+  end
+end
+
 a.describe("file_lock", function()
   a.describe("get()", function()
     a.describe("when no lock file", function()
@@ -24,9 +37,11 @@ a.describe("file_lock", function()
     a.describe("when with a lock file", function()
       with_dir(function(filename)
         local fl = FileLock.new(filename, { retry = 1, interval = 10 })
-        a.it("fails to get", function()
-          assert.is.Nil(async.uv.fs_open(fl.filename, "wx", tonumber("600", 8)))
-          assert.are.same("failed to get lock", fl:get())
+        a.it("gets successfully", function()
+          local err, fd = async.uv.fs_open(fl.filename, "wx", tonumber("600", 8))
+          assert.is.Nil(err)
+          assert.is.Nil(async.uv.fs_close(fd))
+          assert.is.Nil(fl:get())
         end)
       end)
     end)
@@ -34,12 +49,25 @@ a.describe("file_lock", function()
     a.describe("when getting twice", function()
       with_dir(function(filename)
         local fl = FileLock.new(filename, { retry = 1, interval = 10 })
-        a.it("fails to get", function()
+        a.it("gets successfully", function()
+          assert.is.Nil(fl:get())
           assert.is.Nil(fl:get())
-          assert.are.same("failed to get lock", fl:get())
         end)
       end)
     end)
+
+    a.describe("when getting twice but unlink fails", function()
+      with_dir(function(filename)
+        local fl = FileLock.new(filename, { retry = 1, interval = 10 })
+        a.it(
+          "fails to get",
+          with_unlink_fails(function()
+            assert.is.Nil(fl:get())
+            assert.are.same("failed to get lock", fl:get())
+          end)
+        )
+      end)
+    end)
   end)
 
   a.describe("release()", function()
@@ -78,15 +106,18 @@ a.describe("file_lock", function()
     a.describe("when get() fails", function()
       with_dir(function(filename)
         local fl = FileLock.new(filename, { retry = 1, interval = 10 })
-        a.it("fails with a valid err", function()
-          assert.is.Nil(fl:get())
-          assert.are.same(
-            "failed to get lock",
-            fl:with(function()
-              return nil
-            end)
-          )
-        end)
+        a.it(
+          "fails with a valid err",
+          with_unlink_fails(function()
+            assert.is.Nil(fl:get())
+            assert.are.same(
+              "failed to get lock",
+              fl:with(function()
+                return nil
+              end)
+            )
+          end)
+        )
       end)
     end)