From 2cdcc29902212fe386407f7c8be252bf0c3476f2 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Wed, 6 Aug 2014 10:24:05 -0700
Subject: [PATCH] provisioners/chef: put global lock around knife exec

---
 CHANGELOG.md                                  |  1 +
 lib/vagrant/shared_helpers.rb                 | 12 ++++++++++
 .../chef/provisioner/chef_client.rb           | 22 +++++++++++--------
 test/unit/vagrant/shared_helpers_test.rb      |  7 ++++++
 4 files changed, 33 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7b7c850d0..a3a1932dc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -23,6 +23,7 @@ BUG FIXES:
   - hosts/arch: NFS works with latest versions. [GH-4224]
   - hosts/windows: RDP command works without crash. [GH-3962]
   - providers/virtualbox: Increase network device limit to 36. [GH-4206]
+  - provisioners/chef: Chef client cleanup should work. [GH-4099]
   - provisioners/puppet: Manifest file can be a directory. [GH-4169]
   - provisioners/puppet: Properly escape facter variables for PowerShell
       on Windows guests. [GH-3959]
diff --git a/lib/vagrant/shared_helpers.rb b/lib/vagrant/shared_helpers.rb
index a93dbbe8a..7c33533ef 100644
--- a/lib/vagrant/shared_helpers.rb
+++ b/lib/vagrant/shared_helpers.rb
@@ -1,7 +1,10 @@
 require "pathname"
 require "tempfile"
+require "thread"
 
 module Vagrant
+  @@global_lock = Mutex.new
+
   # This is the default endpoint of the Vagrant Cloud in
   # use. API calls will be made to this for various functions
   # of Vagrant that may require remote access.
@@ -9,6 +12,15 @@ module Vagrant
   # @return [String]
   DEFAULT_SERVER_URL = "https://vagrantcloud.com"
 
+  # This holds a global lock for the duration of the block. This should
+  # be invoked around anything that is modifying process state (such as
+  # environmental variables).
+  def self.global_lock
+    @@global_lock.synchronize do
+      return yield
+    end
+  end
+
   # This returns a true/false showing whether we're running from the
   # environment setup by the Vagrant installers.
   #
diff --git a/plugins/provisioners/chef/provisioner/chef_client.rb b/plugins/provisioners/chef/provisioner/chef_client.rb
index fa02d9ecf..d91b3c22c 100644
--- a/plugins/provisioners/chef/provisioner/chef_client.rb
+++ b/plugins/provisioners/chef/provisioner/chef_client.rb
@@ -1,4 +1,6 @@
 require 'pathname'
+
+require 'vagrant'
 require 'vagrant/util/subprocess'
 
 require File.expand_path("../base", __FILE__)
@@ -106,15 +108,17 @@ module VagrantPlugins
 
           # Knife is not part of the current Vagrant bundle, so it needs to run
           # in the context of the system.
-          Bundler.with_clean_env do
-            command = ["knife", deletable, "delete", "--yes", node_name]
-            r = Vagrant::Util::Subprocess.execute(*command)
-            if r.exit_code != 0
-              @machine.ui.error(I18n.t(
-                "vagrant.chef_client_cleanup_failed",
-                deletable: deletable,
-                stdout: r.stdout,
-                stderr: r.stderr))
+          Vagrant.global_lock do
+            Bundler.with_clean_env do
+              command = ["knife", deletable, "delete", "--yes", node_name]
+              r = Vagrant::Util::Subprocess.execute(*command)
+              if r.exit_code != 0
+                @machine.ui.error(I18n.t(
+                  "vagrant.chef_client_cleanup_failed",
+                  deletable: deletable,
+                  stdout: r.stdout,
+                  stderr: r.stderr))
+              end
             end
           end
         end
diff --git a/test/unit/vagrant/shared_helpers_test.rb b/test/unit/vagrant/shared_helpers_test.rb
index 7333ea1f5..c64a26001 100644
--- a/test/unit/vagrant/shared_helpers_test.rb
+++ b/test/unit/vagrant/shared_helpers_test.rb
@@ -8,6 +8,13 @@ describe Vagrant do
 
   subject { described_class }
 
+  describe "#global_lock" do
+    it "yields to the block" do
+      result = subject.global_lock { 42 }
+      expect(result).to eq(42)
+    end
+  end
+
   describe "#in_installer?" do
     it "is not if env is not set" do
       with_temp_env("VAGRANT_INSTALLER_ENV" => nil) do