From 874a9763f5e6b674d7f408b67c45b1a881549d88 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 9 Jul 2011 15:57:24 -0700 Subject: [PATCH] Only one copy of Vagrant can run at any given time. [closes GH-364] This is to protect against issues with VirtualBox overwriting each other. --- CHANGELOG.md | 1 + lib/vagrant/action.rb | 5 ++- lib/vagrant/environment.rb | 37 ++++++++++++++++++- lib/vagrant/errors.rb | 5 +++ templates/locales/en.yml | 5 +++ test/vagrant/environment_test.rb | 61 ++++++++++++++++++++++++++++++++ 6 files changed, 112 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 247677849..5b803fdb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Multiple Chef provisioners no longer overwrite cookbook folders. [GH-407] - `package` won't delete previously existing file. [GH-408] - Vagrantfile can be lowercase now. [GH-399] + - Only one copy of Vagrant may be running at any given time. [GH-364] ## 0.7.6 (July 2, 2011) diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb index 6854e97ec..032199895 100644 --- a/lib/vagrant/action.rb +++ b/lib/vagrant/action.rb @@ -128,7 +128,10 @@ module Vagrant @@reported_interrupt = true end - Busy.busy(int_callback) { callable.call(action_environment) } + # We place a process lock around every action that is called + env.lock do + Busy.busy(int_callback) { callable.call(action_environment) } + end end end end diff --git a/lib/vagrant/environment.rb b/lib/vagrant/environment.rb index 0adf83fc0..b3126a269 100644 --- a/lib/vagrant/environment.rb +++ b/lib/vagrant/environment.rb @@ -60,7 +60,8 @@ module Vagrant :parent => nil, :vm => nil, :cwd => nil, - :vagrantfile_name => nil + :vagrantfile_name => nil, + :lock_path => nil }.merge(opts || {}) # Set the default working directory to look for the vagrantfile @@ -77,6 +78,7 @@ module Vagrant end @loaded = false + @lock_acquired = false end #--------------------------------------------------------------- @@ -281,6 +283,39 @@ module Vagrant @root_path = root_finder.call(cwd) end + # This returns the path which Vagrant uses to determine the location + # of the file lock. This is specific to each operating system. + def lock_path + @lock_path || tmp_path.join("vagrant.lock") + end + + # This locks Vagrant for the duration of the block passed to this + # method. During this time, any other environment which attempts + # to lock which points to the same lock file will fail. + def lock + # This allows multiple locks in the same process to be nested + return yield if @lock_acquired + + File.open(lock_path, "w+") do |f| + # The file locking fails only if it returns "false." If it + # succeeds it returns a 0, so we must explicitly check for + # the proper error case. + raise Errors::EnvironmentLockedError if f.flock(File::LOCK_EX | File::LOCK_NB) === false + + begin + # Mark that we have a lock + @lock_acquired = true + + yield + ensure + # We need to make sure that no matter what this is always + # reset to false so we don't think we have a lock when we + # actually don't. + @lock_acquired = false + end + end + end + #--------------------------------------------------------------- # Config Methods #--------------------------------------------------------------- diff --git a/lib/vagrant/errors.rb b/lib/vagrant/errors.rb index 3d28a3a1a..decc6d1af 100644 --- a/lib/vagrant/errors.rb +++ b/lib/vagrant/errors.rb @@ -153,6 +153,11 @@ module Vagrant error_key(:status_error, "vagrant.downloaders.http") end + class EnvironmentLockedError < VagrantError + status_code(52) + error_key(:environment_locked) + end + class ForwardPortAutolistEmpty < VagrantError status_code(27) error_key(:auto_empty, "vagrant.actions.vm.forward_ports") diff --git a/templates/locales/en.yml b/templates/locales/en.yml index 208faf51c..6407cb73a 100644 --- a/templates/locales/en.yml +++ b/templates/locales/en.yml @@ -31,6 +31,11 @@ en: this command in another directory. If you aren't in a home directory, then please rename ".vagrant" to something else, or configure Vagrant to use another filename by modifying `config.vagrant.dotfile_name`. + environment_locked: |- + An instance of Vagrant is already running. Only one instance of Vagrant + may run at any given time to avoid problems with VirtualBox inconsistencies + occurring. Please wait for the other instance of Vagrant to end and then + try again. interrupted: "Vagrant exited after cleanup due to external interrupt." multi_vm_required: "A multi-vm environment is required for name specification to this command." multi_vm_target_required: "`vagrant %{command}` requires a specific VM name to target in a multi-VM environment." diff --git a/test/vagrant/environment_test.rb b/test/vagrant/environment_test.rb index babc5a715..1cef71619 100644 --- a/test/vagrant/environment_test.rb +++ b/test/vagrant/environment_test.rb @@ -1,5 +1,6 @@ require "test_helper" require "pathname" +require "tempfile" class EnvironmentTest < Test::Unit::TestCase setup do @@ -291,6 +292,66 @@ class EnvironmentTest < Test::Unit::TestCase end end + context "locking" do + setup do + @instance = @klass.new(:lock_path => Tempfile.new('vagrant-test').path) + end + + should "allow nesting locks" do + assert_nothing_raised do + @instance.lock do + @instance.lock do + # Nothing + end + end + end + end + + should "raise an exception if an environment already has a lock" do + @another = @klass.new(:lock_path => @instance.lock_path) + + # Create first locked thread which should succeed + first = Thread.new do + begin + @instance.lock do + Thread.current[:locked] = true + loop { sleep 1000 } + end + rescue Vagrant::Errors::EnvironmentLockedError + Thread.current[:locked] = false + end + end + + # Wait until the first thread has acquired the lock + loop do + break if first[:locked] || !first.alive? + Thread.pass + end + + # Verify that the fist got the lock + assert first[:locked] + + # Create second locked thread which should fail + second = Thread.new do + begin + @another.lock do + Thread.current[:error] = false + end + rescue Vagrant::Errors::EnvironmentLockedError + Thread.current[:error] = true + end + end + + # Wait for the second to end and verify it failed + second.join + assert second[:error] + + # Make sure both threads are killed + first.kill + second.kill + end + end + context "accessing the configuration" do should "load the environment if its not already loaded" do env = @klass.new(:cwd => vagrantfile)