From f7b6d8599242a97fe23cb1534fdecb1327c6fcaf Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Fri, 28 Aug 2026 02:49:30 +0300 Subject: [PATCH 1/3] Close file descriptors when lock acquisition fails or is interrupted --- lib/pstore.rb | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/pstore.rb b/lib/pstore.rb index e730be2..a159118 100644 --- a/lib/pstore.rb +++ b/lib/pstore.rb @@ -624,21 +624,19 @@ def open_and_lock_file(filename, read_only) if read_only begin file = File.new(filename, **RD_ACCESS) - begin - file.flock(File::LOCK_SH) - return file - rescue - file.close - raise - end rescue Errno::ENOENT return nil end else file = File.new(filename, **RDWR_ACCESS) - file.flock(File::LOCK_EX) - return file end + begin + file.flock(read_only ? File::LOCK_SH : File::LOCK_EX) + rescue Exception + file.close + raise + end + file end # Load the given PStore file. From c1aa054df123804afff9e12495a8614a2f85f32a Mon Sep 17 00:00:00 2001 From: Oskar Eichler Date: Fri, 28 Aug 2026 02:52:12 +0300 Subject: [PATCH 2/3] Retry file acquisition after an atomic replacement --- lib/pstore.rb | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/pstore.rb b/lib/pstore.rb index a159118..6b6747b 100644 --- a/lib/pstore.rb +++ b/lib/pstore.rb @@ -621,22 +621,27 @@ def transaction(read_only = false) # :yields: pstore # All exceptions are propagated. # def open_and_lock_file(filename, read_only) - if read_only + filename = File.path(filename) + loop do + if read_only + begin + file = File.new(filename, **RD_ACCESS) + rescue Errno::ENOENT + return nil + end + else + file = File.new(filename, **RDWR_ACCESS) + end + current = false begin - file = File.new(filename, **RD_ACCESS) - rescue Errno::ENOENT - return nil + 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) - end - begin - file.flock(read_only ? File::LOCK_SH : File::LOCK_EX) - rescue Exception - file.close - raise end - file end # Load the given PStore file. From 864875e4abea42515eebb1c31e617b1785306a99 Mon Sep 17 00:00:00 2001 From: Oskar Eichler <62393985+OskarEichler@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:59:57 +0300 Subject: [PATCH 3/3] test: cover PStore regression --- test/test_pstore.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/test_pstore.rb b/test/test_pstore.rb index 4a65e4f..1cef149 100644 --- a/test/test_pstore.rb +++ b/test/test_pstore.rb @@ -190,4 +190,33 @@ def clear_store def second_file File.join(Dir.tmpdir, "pstore.tmp2.#{Process.pid}") 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