diff --git a/lib/vagrant/util/file_mutex.rb b/lib/vagrant/util/file_mutex.rb index 0844726dd..9eababb15 100644 --- a/lib/vagrant/util/file_mutex.rb +++ b/lib/vagrant/util/file_mutex.rb @@ -1,10 +1,16 @@ module Vagrant module Util + # Utility to provide a simple mutex via file lock class FileMutex + # Create a new FileMutex instance + # + # @param mutex_path [String] path for file def initialize(mutex_path) @mutex_path = mutex_path end + # Execute provided block within lock and unlock + # when completed def with_lock(&block) lock begin @@ -16,16 +22,26 @@ module Vagrant end end + # Attempt to acquire the lock def lock - f = File.open(@mutex_path, "w+") - if f.flock(File::LOCK_EX|File::LOCK_NB) === false + if lock_file.flock(File::LOCK_EX|File::LOCK_NB) === false raise Errors::VagrantLocked, lock_file_path: @mutex_path end end + # Unlock the file def unlock + lock_file.flock(File::LOCK_UN) + lock_file.close File.delete(@mutex_path) if File.file?(@mutex_path) end + + protected + + def lock_file + return @lock_file if @lock_file && !@lock_file.closed? + @lock_file = File.open(@mutex_path, "w+") + end end end end diff --git a/test/unit/vagrant/util/file_mutex_test.rb b/test/unit/vagrant/util/file_mutex_test.rb index eacbd9ffd..131e08a12 100644 --- a/test/unit/vagrant/util/file_mutex_test.rb +++ b/test/unit/vagrant/util/file_mutex_test.rb @@ -5,77 +5,75 @@ describe Vagrant::Util::FileMutex do include_context "unit" let(:temp_dir) { Dir.mktmpdir("vagrant-test-util-mutex_test") } + let(:mutex_path) { File.join(temp_dir, "test.lock") } + let(:subject) { described_class.new(mutex_path) } after do FileUtils.rm_rf(temp_dir) end it "should create a lock file" do - mutex_path = temp_dir + "test.lock" - instance = described_class.new(mutex_path) - instance.lock + subject.lock expect(File).to exist(mutex_path) end it "should create and delete lock file" do - mutex_path = temp_dir + "test.lock" - instance = described_class.new(mutex_path) - instance.lock + subject.lock expect(File).to exist(mutex_path) - instance.unlock + subject.unlock expect(File).to_not exist(mutex_path) end it "should not raise an error if the lock file does not exist" do - mutex_path = temp_dir + "test.lock" - instance = described_class.new(mutex_path) - instance.unlock + subject.unlock expect(File).to_not exist(mutex_path) end it "should run a function with a lock" do - mutex_path = temp_dir + "test.lock" - instance = described_class.new(mutex_path) - instance.with_lock { true } + subject.with_lock { true } expect(File).to_not exist(mutex_path) end it "should fail running a function when locked" do - mutex_path = temp_dir + "test.lock" # create a lock - instance = described_class.new(mutex_path) - instance.lock + subject.lock # create a new lock that will run a function - instance2 = described_class.new(mutex_path) + instance = described_class.new(mutex_path) # lock should persist for multiple runs - expect {instance2.with_lock { true }}. + expect {instance.with_lock { true }}. to raise_error(Vagrant::Errors::VagrantLocked) - expect {instance2.with_lock { true }}. + expect {instance.with_lock { true }}. to raise_error(Vagrant::Errors::VagrantLocked) # mutex should exist until its unlocked expect(File).to exist(mutex_path) - instance.unlock + subject.unlock expect(File).to_not exist(mutex_path) end it "should fail running a function within a locked" do - mutex_path = temp_dir + "test.lock" - # create a lock - instance = described_class.new(mutex_path) # create a new lock that will run a function - instance2 = described_class.new(mutex_path) + instance = described_class.new(mutex_path) expect { - instance.with_lock { instance2.with_lock{true} } + subject.with_lock { instance.with_lock{true} } }.to raise_error(Vagrant::Errors::VagrantLocked) expect(File).to_not exist(mutex_path) end it "should delete the lock even when the function fails" do - mutex_path = temp_dir + "test.lock" - instance = described_class.new(mutex_path) expect { - instance.with_lock { raise Vagrant::Errors::VagrantError.new } + subject.with_lock { raise Vagrant::Errors::VagrantError.new } }.to raise_error(Vagrant::Errors::VagrantError) expect(File).to_not exist(mutex_path) end + + it "should unlock file before deletion" do + lock_file = double(:lock_file) + allow(subject).to receive(:lock_file).and_return(lock_file) + allow(lock_file).to receive(:flock).and_return(true) + + expect(lock_file).to receive(:flock).with(File::LOCK_UN) + expect(lock_file).to receive(:close) + + subject.with_lock { true } + end end