diff --git a/lib/pstore.rb b/lib/pstore.rb index 8c32d78..2978be0 100644 --- a/lib/pstore.rb +++ b/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 diff --git a/test/test_pstore.rb b/test/test_pstore.rb index 98d4459..f72378c 100644 --- a/test/test_pstore.rb +++ b/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