SafeChdir all the things for thread safety

lib/vagrant/action/general/package.rb

@@ -1,5 +1,6 @@
 require 'fileutils'
 
+require 'vagrant/util/safe_chdir'
 require 'vagrant/util/subprocess'
 
 module Vagrant
@@ -83,7 +84,7 @@ module Vagrant
           output_path = tar_path.to_s
 
           # Switch into that directory and package everything up
-          Dir.chdir(@env["package.directory"]) do
+          Util::SafeChdir.safe_chdir(@env["package.directory"]) do
             # Find all the files in our current directory and tar it up!
             files = Dir.glob(File.join(".", "**", "*"))
 

lib/vagrant/box.rb

@@ -4,6 +4,7 @@ require "json"
 require "log4r"
 
 require "vagrant/util/platform"
+require "vagrant/util/safe_chdir"
 require "vagrant/util/subprocess"
 
 module Vagrant
@@ -71,7 +72,7 @@ module Vagrant
     def repackage(path)
       @logger.debug("Repackaging box '#{@name}' to: #{path}")
 
-      Dir.chdir(@directory) do
+      Util::SafeChdir.safe_chdir(@directory) do
         # Find all the files in our current directory and tar it up!
         files = Dir.glob(File.join(".", "**", "*"))
 

lib/vagrant/util/safe_chdir.rb

@@ -0,0 +1,34 @@
+require 'thread'
+
+module Vagrant
+  module Util
+    class SafeChdir
+      @@chdir_lock  = Mutex.new
+      @@lock_holder = nil
+
+      # Safely changes directory of this process by putting a lock around
+      # it so that it is thread safe. This will yield a block and when the
+      # block exits it changes back to the original directory.
+      #
+      # @param [String] dir Dir to change to temporarily
+      def self.safe_chdir(dir)
+        lock = @@chdir_lock
+
+        begin
+          @@chdir_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
+          Dir.chdir(dir) do
+            return yield
+          end
+        end
+      end
+    end
+  end
+end
+

lib/vagrant/util/subprocess.rb

@@ -4,6 +4,7 @@ require 'childprocess'
 require 'log4r'
 
 require 'vagrant/util/platform'
+require 'vagrant/util/safe_chdir'
 
 module Vagrant
   module Util
@@ -14,8 +15,6 @@ module Vagrant
     # from the subprocess in real time, by simply passing a block to
     # the execute method.
     class Subprocess
-      @@chdir_lock = Mutex.new
-
       # The chunk size for reading from subprocess IO.
       READ_CHUNK_SIZE = 4096
 
@@ -76,10 +75,8 @@ module Vagrant
 
         # Start the process
         begin
-          @@chdir_lock.synchronize do
+          SafeChdir.safe_chdir(workdir) do
-            Dir.chdir(workdir) do
+            process.start
-              process.start
-            end
           end
         rescue ChildProcess::LaunchError => ex
           # Raise our own version of the error so that users of the class

test/unit/vagrant/util/safe_chdir_test.rb

@@ -0,0 +1,43 @@
+require 'tmpdir'
+
+require File.expand_path("../../../base", __FILE__)
+
+require 'vagrant/util/safe_chdir'
+
+describe Vagrant::Util::SafeChdir do
+  it "should change directories" do
+    expected = nil
+    result   = nil
+    temp_dir = Dir.mktmpdir
+
+    Dir.chdir(temp_dir) do
+      expected = Dir.pwd
+    end
+
+    described_class.safe_chdir(temp_dir) do
+      result = Dir.pwd
+    end
+
+    result.should == expected
+  end
+
+  it "should allow recursive chdir" do
+    expected  = nil
+    result    = nil
+    temp_path = Dir.mktmpdir
+
+    Dir.chdir(temp_path) do
+      expected = Dir.pwd
+    end
+
+    expect do
+      described_class.safe_chdir(Dir.mktmpdir) do
+        described_class.safe_chdir(temp_path) do
+          result = Dir.pwd
+        end
+      end
+    end.to_not raise_error
+
+    result.should == expected
+  end
+end