Improve the thread safety of BoxCollection

This commit is contained in:
Mitchell Hashimoto 2013-04-19 23:48:05 -06:00
parent 9dd582be3a
commit 04d9872674
2 changed files with 162 additions and 134 deletions

View File

@ -1,4 +1,5 @@
require "digest/sha1" require "digest/sha1"
require "thread"
require "tmpdir" require "tmpdir"
require "log4r" require "log4r"
@ -43,6 +44,7 @@ module Vagrant
options ||= {} options ||= {}
@directory = directory @directory = directory
@lock = Mutex.new
@temp_root = options[:temp_dir_root] @temp_root = options[:temp_dir_root]
@logger = Log4r::Logger.new("vagrant::box_collection") @logger = Log4r::Logger.new("vagrant::box_collection")
end end
@ -69,97 +71,99 @@ module Vagrant
# @param [Boolean] force If true, any existing box with the same name # @param [Boolean] force If true, any existing box with the same name
# and provider will be replaced. # and provider will be replaced.
def add(path, name, provider=nil, force=false) def add(path, name, provider=nil, force=false)
# A helper to check if a box exists. We store this in a variable with_collection_lock do
# since we call it multiple times. # A helper to check if a box exists. We store this in a variable
check_box_exists = lambda do |box_provider| # since we call it multiple times.
box = find(name, box_provider) check_box_exists = lambda do |box_provider|
next if !box box = find(name, box_provider)
next if !box
if !force if !force
@logger.error("Box already exists, can't add: #{name} #{box_provider}") @logger.error("Box already exists, can't add: #{name} #{box_provider}")
raise Errors::BoxAlreadyExists, :name => name, :provider => box_provider raise Errors::BoxAlreadyExists, :name => name, :provider => box_provider
end
# We're forcing, so just delete the old box
@logger.info("Box already exists, but forcing so removing: #{name} #{box_provider}")
box.destroy!
end end
# We're forcing, so just delete the old box log_provider = provider ? provider : "any provider"
@logger.info("Box already exists, but forcing so removing: #{name} #{box_provider}") @logger.debug("Adding box: #{name} (#{log_provider}) from #{path}")
box.destroy!
end
log_provider = provider ? provider : "any provider" # Verify the box doesn't exist early if we're given a provider. This
@logger.debug("Adding box: #{name} (#{log_provider}) from #{path}") # can potentially speed things up considerably since we don't need
# to unpack any files.
check_box_exists.call(provider) if provider
# Verify the box doesn't exist early if we're given a provider. This # Verify that a V1 box doesn't exist. If it does, then we signal
# can potentially speed things up considerably since we don't need # to the user that we need an upgrade.
# to unpack any files. raise Errors::BoxUpgradeRequired, :name => name if v1_box?(@directory.join(name))
check_box_exists.call(provider) if provider
# Verify that a V1 box doesn't exist. If it does, then we signal # Create a temporary directory since we're not sure at this point if
# to the user that we need an upgrade. # the box we're unpackaging already exists (if no provider was given)
raise Errors::BoxUpgradeRequired, :name => name if v1_box?(@directory.join(name)) with_temp_dir do |temp_dir|
# Extract the box into a temporary directory.
@logger.debug("Unpacking box into temporary directory: #{temp_dir}")
result = Util::Subprocess.execute(
"bsdtar", "-v", "-x", "-m", "-C", temp_dir.to_s, "-f", path.to_s)
raise Errors::BoxUnpackageFailure, :output => result.stderr.to_s if result.exit_code != 0
# Create a temporary directory since we're not sure at this point if # If we get a V1 box, we want to update it in place
# the box we're unpackaging already exists (if no provider was given) if v1_box?(temp_dir)
with_temp_dir do |temp_dir| @logger.debug("Added box is a V1 box. Upgrading in place.")
# Extract the box into a temporary directory. temp_dir = v1_upgrade(temp_dir)
@logger.debug("Unpacking box into temporary directory: #{temp_dir}") end
result = Util::Subprocess.execute(
"bsdtar", "-v", "-x", "-m", "-C", temp_dir.to_s, "-f", path.to_s)
raise Errors::BoxUnpackageFailure, :output => result.stderr.to_s if result.exit_code != 0
# If we get a V1 box, we want to update it in place # We re-wrap ourselves in the safety net in case we upgraded.
if v1_box?(temp_dir) # If we didn't upgrade, then this is still safe because the
@logger.debug("Added box is a V1 box. Upgrading in place.") # helper will only delete the directory if it exists
temp_dir = v1_upgrade(temp_dir) with_temp_dir(temp_dir) do |final_temp_dir|
end # Get an instance of the box we just added before it is finalized
# in the system so we can inspect and use its metadata.
box = Box.new(name, provider, final_temp_dir)
# We re-wrap ourselves in the safety net in case we upgraded. # Get the provider, since we'll need that to at the least add it
# If we didn't upgrade, then this is still safe because the # to the system or check that it matches what is given to us.
# helper will only delete the directory if it exists box_provider = box.metadata["provider"]
with_temp_dir(temp_dir) do |final_temp_dir|
# Get an instance of the box we just added before it is finalized
# in the system so we can inspect and use its metadata.
box = Box.new(name, provider, final_temp_dir)
# Get the provider, since we'll need that to at the least add it if provider
# to the system or check that it matches what is given to us. # Verify that the given provider matches what the box has.
box_provider = box.metadata["provider"] if box_provider.to_sym != provider
@logger.error("Added box provider doesnt match expected: #{box_provider}")
raise Errors::BoxProviderDoesntMatch, :expected => provider, :actual => box_provider
end
else
# We weren't given a provider, so store this one.
provider = box_provider.to_sym
if provider # Verify the box doesn't already exist
# Verify that the given provider matches what the box has. check_box_exists.call(provider)
if box_provider.to_sym != provider
@logger.error("Added box provider doesnt match expected: #{box_provider}")
raise Errors::BoxProviderDoesntMatch, :expected => provider, :actual => box_provider
end end
else
# We weren't given a provider, so store this one.
provider = box_provider.to_sym
# Verify the box doesn't already exist # Create the directory for this box, not including the provider
check_box_exists.call(provider) box_dir = @directory.join(name)
end box_dir.mkpath
@logger.debug("Box directory: #{box_dir}")
# Create the directory for this box, not including the provider # This is the final directory we'll move it to
box_dir = @directory.join(name) final_dir = box_dir.join(provider.to_s)
box_dir.mkpath if final_dir.exist?
@logger.debug("Box directory: #{box_dir}") @logger.debug("Removing existing provider directory...")
final_dir.rmtree
end
# This is the final directory we'll move it to # Move to final destination
final_dir = box_dir.join(provider.to_s) final_dir.mkpath
if final_dir.exist?
@logger.debug("Removing existing provider directory...")
final_dir.rmtree
end
# Move to final destination # Go through each child and copy them one-by-one. This avoids
final_dir.mkpath # an issue where on Windows cross-device directory copies are
# failing for some reason. [GH-1424]
# Go through each child and copy them one-by-one. This avoids final_temp_dir.children(true).each do |f|
# an issue where on Windows cross-device directory copies are destination = final_dir.join(f.basename)
# failing for some reason. [GH-1424] @logger.debug("Moving: #{f} => #{destination}")
final_temp_dir.children(true).each do |f| FileUtils.mv(f, destination)
destination = final_dir.join(f.basename) end
@logger.debug("Moving: #{f} => #{destination}")
FileUtils.mv(f, destination)
end end
end end
end end
@ -177,33 +181,35 @@ module Vagrant
def all def all
results = [] results = []
@logger.debug("Finding all boxes in: #{@directory}") with_collection_lock do
@directory.children(true).each do |child| @logger.debug("Finding all boxes in: #{@directory}")
# Ignore non-directories, since files are not interesting to @directory.children(true).each do |child|
# us in our folder structure. # Ignore non-directories, since files are not interesting to
next if !child.directory? # us in our folder structure.
next if !child.directory?
box_name = child.basename.to_s box_name = child.basename.to_s
# If this is a V1 box, we still return that name, but specify # If this is a V1 box, we still return that name, but specify
# that the box is a V1 box. # that the box is a V1 box.
if v1_box?(child) if v1_box?(child)
@logger.debug("V1 box found: #{box_name}") @logger.debug("V1 box found: #{box_name}")
results << [box_name, :virtualbox, :v1] results << [box_name, :virtualbox, :v1]
next next
end end
# Otherwise, traverse the subdirectories and see what providers # Otherwise, traverse the subdirectories and see what providers
# we have. # we have.
child.children(true).each do |provider| child.children(true).each do |provider|
# Verify this is a potentially valid box. If it looks # Verify this is a potentially valid box. If it looks
# correct enough then include it. # correct enough then include it.
if provider.directory? && provider.join("metadata.json").file? if provider.directory? && provider.join("metadata.json").file?
provider_name = provider.basename.to_s.to_sym provider_name = provider.basename.to_s.to_sym
@logger.debug("Box: #{box_name} (#{provider_name})") @logger.debug("Box: #{box_name} (#{provider_name})")
results << [box_name, provider_name] results << [box_name, provider_name]
else else
@logger.debug("Invalid box, ignoring: #{provider}") @logger.debug("Invalid box, ignoring: #{provider}")
end
end end
end end
end end
@ -217,29 +223,31 @@ module Vagrant
# @Param [String] provider Provider that the box implements. # @Param [String] provider Provider that the box implements.
# @return [Box] The box found, or `nil` if not found. # @return [Box] The box found, or `nil` if not found.
def find(name, provider) def find(name, provider)
# First look directly for the box we're asking for. with_collection_lock do
box_directory = @directory.join(name, provider.to_s, "metadata.json") # First look directly for the box we're asking for.
@logger.info("Searching for box: #{name} (#{provider}) in #{box_directory}") box_directory = @directory.join(name, provider.to_s, "metadata.json")
if box_directory.file? @logger.info("Searching for box: #{name} (#{provider}) in #{box_directory}")
@logger.info("Box found: #{name} (#{provider})") if box_directory.file?
return Box.new(name, provider, box_directory.dirname) @logger.info("Box found: #{name} (#{provider})")
end return Box.new(name, provider, box_directory.dirname)
end
# If we're looking for a VirtualBox box, then we check if there is # If we're looking for a VirtualBox box, then we check if there is
# a V1 box. # a V1 box.
if provider == :virtualbox if provider == :virtualbox
# Check if a V1 version of this box exists, and if so, raise an # Check if a V1 version of this box exists, and if so, raise an
# exception notifying the caller that the box exists but needs # exception notifying the caller that the box exists but needs
# to be upgraded. We don't do the upgrade here because it can be # to be upgraded. We don't do the upgrade here because it can be
# a fairly intensive activity and don't want to immediately degrade # a fairly intensive activity and don't want to immediately degrade
# user performance on a find. # user performance on a find.
# #
# To determine if it is a V1 box we just do a simple heuristic # To determine if it is a V1 box we just do a simple heuristic
# based approach. # based approach.
@logger.info("Searching for V1 box: #{name}") @logger.info("Searching for V1 box: #{name}")
if v1_box?(@directory.join(name)) if v1_box?(@directory.join(name))
@logger.warn("V1 box found: #{name}") @logger.warn("V1 box found: #{name}")
raise Errors::BoxUpgradeRequired, :name => name raise Errors::BoxUpgradeRequired, :name => name
end
end end
end end
@ -255,21 +263,23 @@ module Vagrant
# #
# @return [Boolean] `true` otherwise an exception is raised. # @return [Boolean] `true` otherwise an exception is raised.
def upgrade(name) def upgrade(name)
@logger.debug("Upgrade request for box: #{name}") with_collection_lock do
box_dir = @directory.join(name) @logger.debug("Upgrade request for box: #{name}")
box_dir = @directory.join(name)
# If the box doesn't exist at all, raise an exception # If the box doesn't exist at all, raise an exception
raise Errors::BoxNotFound, :name => name if !box_dir.directory? raise Errors::BoxNotFound, :name => name if !box_dir.directory?
if v1_box?(box_dir) if v1_box?(box_dir)
@logger.debug("V1 box #{name} found. Upgrading!") @logger.debug("V1 box #{name} found. Upgrading!")
# First we actually perform the upgrade # First we actually perform the upgrade
temp_dir = v1_upgrade(box_dir) temp_dir = v1_upgrade(box_dir)
# Rename the temporary directory to the provider. # Rename the temporary directory to the provider.
FileUtils.mv(temp_dir.to_s, box_dir.join("virtualbox").to_s) FileUtils.mv(temp_dir.to_s, box_dir.join("virtualbox").to_s)
@logger.info("Box '#{name}' upgraded from V1 to V2.") @logger.info("Box '#{name}' upgraded from V1 to V2.")
end
end end
# We did it! Or the v1 box didn't exist so it doesn't matter. # We did it! Or the v1 box didn't exist so it doesn't matter.
@ -330,6 +340,24 @@ module Vagrant
temp_dir temp_dir
end end
# This locks the region given by the block with a lock on this
# collection.
def with_collection_lock
lock = @lock
begin
lock.synchronize {}
rescue ThreadError
# If we already hold the lock, just create a new lock so
# we definitely don't block and don't get an error.
lock = Mutex.new
end
lock.synchronize do
return yield
end
end
# This is a helper that makes sure that our temporary directories # This is a helper that makes sure that our temporary directories
# are cleaned up no matter what. # are cleaned up no matter what.
# #

View File

@ -331,7 +331,6 @@ module Vagrant
# Determine the possible box formats for any boxes and find the box # Determine the possible box formats for any boxes and find the box
box_formats = provider_options[:box_format] || provider box_formats = provider_options[:box_format] || provider
box = nil box = nil
box = find_box(config.vm.box, box_formats) if config.vm.box
# Set this variable in order to keep track of if the box changes # Set this variable in order to keep track of if the box changes
# too many times. # too many times.
@ -339,6 +338,8 @@ module Vagrant
box_changed = false box_changed = false
load_box_and_overrides = lambda do load_box_and_overrides = lambda do
box = find_box(config.vm.box, box_formats) if config.vm.box
# If a box was found, then we attempt to load the Vagrantfile for # If a box was found, then we attempt to load the Vagrantfile for
# that box. We don't require a box since we allow providers to download # that box. We don't require a box since we allow providers to download
# boxes and so on. # boxes and so on.
@ -378,7 +379,6 @@ module Vagrant
@logger.info("Box changed to: #{config.vm.box}. Reloading configurations.") @logger.info("Box changed to: #{config.vm.box}. Reloading configurations.")
original_box = config.vm.box original_box = config.vm.box
box_changed = true box_changed = true
box = find_box(config.vm.box, box_formats)
# Recurse so that we reload all the configurations # Recurse so that we reload all the configurations
load_box_and_overrides.call load_box_and_overrides.call