Lock the current store file and close failed acquisition handles

lib/pstore.rb

@@ -621,23 +621,26 @@ def transaction(read_only = false)  # :yields:  pstore
   # All exceptions are propagated.
   #
   def open_and_lock_file(filename, read_only)
-    if read_only
-      begin
-        file = File.new(filename, **RD_ACCESS)
+    filename = File.path(filename)
+    loop do
+      if read_only
         begin
-          file.flock(File::LOCK_SH)
-          return file
-        rescue
-          file.close
-          raise
+          file = File.new(filename, **RD_ACCESS)
+        rescue Errno::ENOENT
+          return nil
         end
-      rescue Errno::ENOENT
-        return nil
+      else
+        file = File.new(filename, **RDWR_ACCESS)
+      end
+      current = false
+      begin
+        file.flock(read_only ? File::LOCK_SH : File::LOCK_EX)
+        # An atomic save may have replaced the file while this lock was pending.
+        current = File.identical?(file, filename)
+        return file if current
+      ensure
+        file.close unless current
       end
-    else
-      file = File.new(filename, **RDWR_ACCESS)
-      file.flock(File::LOCK_EX)
-      return file
     end
   end
 

test/test_pstore.rb

@@ -244,4 +244,33 @@ def default.==(_other)
 
     assert_same(default, @pstore.transaction(true) { @pstore.fetch(:missing, default) })
   end
+
+  def test_lock_retries_when_atomic_save_replaces_file
+    fake_file = Struct.new(:closed) do
+      def flock(_mode)
+        true
+      end
+
+      def close
+        self.closed = true
+      end
+    end
+    first = fake_file.new(false)
+    second = fake_file.new(false)
+    files = [first, second]
+    original_new = File.method(:new)
+    original_identical = File.method(:identical?)
+    File.define_singleton_method(:new) { |*_args, **_kwargs| files.shift }
+    File.define_singleton_method(:identical?) { |file, _path| file.equal?(second) }
+
+    result = @pstore.send(:open_and_lock_file, @pstore_file, false)
+
+    assert_same(second, result)
+    assert_equal(true, first.closed)
+    assert_equal(false, second.closed)
+  ensure
+    result.close if result && !result.closed
+    File.define_singleton_method(:new, original_new) if original_new
+    File.define_singleton_method(:identical?, original_identical) if original_identical
+  end
 end