From 14d70776cad167a56e4c23f1fdf8c73613e13886 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Sat, 20 Apr 2013 14:31:29 -0600 Subject: [PATCH] Better locking within handle_box_url This improves locking in the face of parallel providers and handling box_url parameters. This avoids downloading a box multiple times. --- lib/vagrant/action/builtin/handle_box_url.rb | 58 +++++++++++++++----- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/lib/vagrant/action/builtin/handle_box_url.rb b/lib/vagrant/action/builtin/handle_box_url.rb index e586494f5..7cbac6570 100644 --- a/lib/vagrant/action/builtin/handle_box_url.rb +++ b/lib/vagrant/action/builtin/handle_box_url.rb @@ -1,3 +1,5 @@ +require "thread" + module Vagrant module Action module Builtin @@ -6,34 +8,60 @@ module Vagrant # sequence for a provider after configuration validation but before # you attempt to use any box. class HandleBoxUrl + @@big_lock = Mutex.new + @@handle_box_url_locks = Hash.new { |h,k| h[k] = Mutex.new } + def initialize(app, env) @app = app end def call(env) if !env[:machine].box + # Get a "big lock" to make sure that our more fine grained + # lock access is thread safe. + lock = nil + @@big_lock.synchronize do + lock = @@handle_box_url_locks[env[:machine].config.vm.box] + end + # We can assume a box URL is set because the Vagrantfile # validation should do this for us. If not, though, we do # raise a terrible runtime error. box_name = env[:machine].config.vm.box box_url = env[:machine].config.vm.box_url - # Add the box then reload the box collection so that it becomes - # aware of it. - env[:ui].info I18n.t( - "vagrant.actions.vm.check_box.not_found", - :name => box_name, - :provider => env[:machine].provider_name) + lock.synchronize do + # First see if we actually have the box now. + has_box = false - begin - env[:action_runner].run(Vagrant::Action.action_box_add, { - :box_name => box_name, - :box_provider => env[:machine].provider_name, - :box_url => box_url - }) - rescue Errors::BoxAlreadyExists - # Just ignore this, since it means the next part will succeed! - # This can happen in a multi-threaded environment. + formats = env[:machine].provider_options[:box_format] || + env[:machine].provider_name + [formats].flatten.each do |format| + if env[:box_collection].find(box_name, format) + has_box = true + break + end + end + + if !has_box + # Add the box then reload the box collection so that it becomes + # aware of it. + env[:ui].info I18n.t( + "vagrant.actions.vm.check_box.not_found", + :name => box_name, + :provider => env[:machine].provider_name) + + begin + env[:action_runner].run(Vagrant::Action.action_box_add, { + :box_name => box_name, + :box_provider => env[:machine].provider_name, + :box_url => box_url + }) + rescue Errors::BoxAlreadyExists + # Just ignore this, since it means the next part will succeed! + # This can happen in a multi-threaded environment. + end + end end # Reload the environment and set the VM to be the new loaded VM.