Lock example_status_persistence_file to prevent race conditions

This introduces file locking to ExampleStatusPersister#persist to
avoid a race condition where parallel or unrelated spec runs race to
update the same file:

- read 1
- read 2
- write 1
- write 2 - write 1 is lost

Because we need to read and write the file under the same lock,
I inline #write into #persist so that dumped_statuses is called
after the file is opened/locked.

Author: Empact

lib/rspec/core/example_status_persister.rb

@@ -21,16 +21,22 @@ def initialize(examples, file_name)
       end
 
       def persist
-        write dumped_statuses
+        RSpec::Support::DirectoryMaker.mkdir_p(File.dirname(@file_name))
+        File.open(@file_name, File::RDWR | File::CREAT) do |f|
+          # lock the file while reading / persisting to avoid a race
+          # condition where parallel or unrelated spec runs race to
+          # update the same file
+          f.flock(File::LOCK_EX)
+          @previous_runs = f.read
+          f.rewind
+          f.write(dumped_statuses)
+          f.flush
+          f.truncate(f.pos)
+        end
       end
 
     private
 
-      def write(statuses)
-        RSpec::Support::DirectoryMaker.mkdir_p(File.dirname(@file_name))
-        File.open(@file_name, "w") { |f| f.write(statuses) }
-      end
-
       def dumped_statuses
         ExampleStatusDumper.dump(merged_statuses)
       end
@@ -52,7 +58,7 @@ def statuses_from_this_run
       end
 
       def statuses_from_previous_runs
-        self.class.load_from(@file_name)
+        ExampleStatusParser.parse(@previous_runs)
       end
     end
 

spec/rspec/core/example_status_persister_spec.rb

@@ -58,6 +58,37 @@ def new_example(id, metadata = {})
         )
       end
 
+      it 'prevents simultaneous access to the file' do
+        # This tests whether a certain race condition is prevented:
+        #  - read 1
+        #  - read 2
+        #  - write 1
+        #  - write 2 - write 1 is lost
+        ex_1 = new_example("#{existing_spec_file}[1:1]", :status => :passed)
+        ex_2 = new_example("spec_1.rb[1:1]", :status => :failed)
+
+        persister_1 = ExampleStatusPersister.new([ex_1], file.path)
+        persister_2 = ExampleStatusPersister.new([ex_2], file.path)
+        persister_1_dumped_statuses = persister_1.method(:dumped_statuses)
+        persister_2_thread = nil
+
+        # dumped status is called after the file is locked but before
+        # the output is written
+        allow(persister_1).to receive(:dumped_statuses) do
+          persister_2_thread = Thread.new { persister_2.persist }
+          persister_1_dumped_statuses.call
+        end
+        persister_1.persist
+        persister_2_thread.join
+
+        loaded = ExampleStatusPersister.load_from(file.path)
+
+        expect(loaded).to contain_exactly(
+          a_hash_including(:example_id => ex_1.id, :status => "passed"),
+          a_hash_including(:example_id => ex_2.id, :status => "failed")
+        )
+      end
+
       it 'merges the example statuses with the existing records in the named file' do
         ex_1 = new_example("#{existing_spec_file}[1:1]", :status => :passed)
         ex_2 = new_example("spec_1.rb[1:1]", :status => :failed)