Windows support for Chef::Runlock.

This ensures that if someone does a manual chef-client run, it doesn't fail if the chef is configured as a service on windows and if there is a chef-client run happening right now. The newly started run will wait for the old run to finish and continue after it.

5 files changed, 295 insertions, 14 deletions

diff --git a/lib/chef/run_lock.rb b/lib/chef/run_lock.rb
index 2ddf02973a..09f6100bee 100644
--- a/lib/chef/run_lock.rb
+++ b/lib/chef/run_lock.rb
@@ -17,6 +17,9 @@
 
 require 'chef/mixin/create_path'
 require 'fcntl'
+if Chef::Platform.windows?
+  require 'chef/win32/mutex'
+end
 
 class Chef
 
@@ -30,6 +33,7 @@ class Chef
     include Chef::Mixin::CreatePath
 
     attr_reader :runlock
+    attr_reader :mutex
     attr_reader :runlock_file
 
     # Create a new instance of RunLock
@@ -38,6 +42,7 @@ class Chef
     def initialize(lockfile)
       @runlock_file = lockfile
       @runlock = nil
+      @mutex = nil
     end
 
     # Acquire the system-wide lock. Will block indefinitely if another process
@@ -58,17 +63,32 @@ class Chef
       # ensure the runlock_file path exists
       create_path(File.dirname(runlock_file))
       @runlock = File.open(runlock_file,'a+')
-      # if we support FD_CLOEXEC (linux, !windows), then use it.
-      # NB: ruby-2.0.0-p195 sets FD_CLOEXEC by default, but not ruby-1.8.7/1.9.3
-      if Fcntl.const_defined?('F_SETFD') && Fcntl.const_defined?('FD_CLOEXEC')
-        runlock.fcntl(Fcntl::F_SETFD, runlock.fcntl(Fcntl::F_GETFD, 0) | Fcntl::FD_CLOEXEC)
-      end
-      # Flock will return 0 if it can acquire the lock otherwise it
-      # will return false
-      if runlock.flock(File::LOCK_NB|File::LOCK_EX) == 0
-        true
+
+      if Chef::Platform.windows?
+        # Since flock mechanism doesn't exist on windows we are using
+        # platform Mutex.
+        # We are creating a "Global" mutex here so that non-admin
+        # users can not DoS chef-client by creating the same named
+        # mutex we are using locally.
+        # Mutex name is case-sensitive contrary to other things in
+        # windows. "\" is the only invalid character.
+        # @mutex = Chef::ReservedNames::Win32::Mutex.new("Global\\serdar:/_-running.pid")
+        @mutex = Chef::ReservedNames::Win32::Mutex.new("Global\\#{runlock_file.gsub(/[\\]/, "/").downcase}")
+        mutex.test
       else
-        false
+        # If we support FD_CLOEXEC, then use it.
+        # NB: ruby-2.0.0-p195 sets FD_CLOEXEC by default, but not
+        # ruby-1.8.7/1.9.3
+        if Fcntl.const_defined?('F_SETFD') && Fcntl.const_defined?('FD_CLOEXEC')
+          runlock.fcntl(Fcntl::F_SETFD, runlock.fcntl(Fcntl::F_GETFD, 0) | Fcntl::FD_CLOEXEC)
+        end
+        # Flock will return 0 if it can acquire the lock otherwise it
+        # will return false
+        if runlock.flock(File::LOCK_NB|File::LOCK_EX) == 0
+          true
+        else
+          false
+        end
       end
     end
 
@@ -78,7 +98,11 @@ class Chef
     def wait
       runpid = runlock.read.strip.chomp
       Chef::Log.warn("Chef client #{runpid} is running, will wait for it to finish and then run.")
-      runlock.flock(File::LOCK_EX)
+      if Chef::Platform.windows?
+        mutex.wait
+      else
+        runlock.flock(File::LOCK_EX)
+      end
     end
 
     def save_pid
@@ -93,7 +117,11 @@ class Chef
     # Release the system-wide lock.
     def release
       if runlock
-        runlock.flock(File::LOCK_UN)
+        if Chef::Platform.windows?
+          mutex.release
+        else
+          runlock.flock(File::LOCK_UN)
+        end
         runlock.close
         # Don't unlink the pid file, if another chef-client was waiting, it
         # won't be recreated. Better to leave a "dead" pid file than not have
@@ -106,6 +134,7 @@ class Chef
 
     def reset
       @runlock = nil
+      @mutex = nil
     end
 
   end
diff --git a/lib/chef/win32/api/synchronization.rb b/lib/chef/win32/api/synchronization.rb
new file mode 100644
index 0000000000..9c148d7e2b
--- /dev/null
+++ b/lib/chef/win32/api/synchronization.rb
@@ -0,0 +1,89 @@
+#
+# Author:: Serdar Sutay (<serdar@opscode.com>)
+# Copyright:: Copyright 2011 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/win32/api'
+
+class Chef
+  module ReservedNames::Win32
+    module API
+      module Synchronization
+        extend Chef::ReservedNames::Win32::API
+
+        ffi_lib 'kernel32'
+
+        # Constant synchronization functions use to indicate wait
+        # forever.
+        INFINITE = 0xFFFFFFFF
+
+        # Return codes
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/ms687032(v=vs.85).aspx
+        WAIT_FAILED = 0xFFFFFFFF
+        WAIT_TIMEOUT = 0x00000102
+        WAIT_OBJECT_0 = 0x00000000
+        WAIT_ABANDONED = 0x00000080
+
+        # Security and access rights for synchronization objects
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/ms686670(v=vs.85).aspx
+        DELETE = 0x00010000
+        READ_CONTROL = 0x00020000
+        SYNCHRONIZE = 0x00100000
+        WRITE_DAC = 0x00040000
+        WRITE_OWNER = 0x00080000
+
+        # Mutex specific rights
+        MUTEX_ALL_ACCESS = 0x001F0001
+        MUTEX_MODIFY_STATE = 0x00000001
+
+=begin
+HANDLE WINAPI CreateMutex(
+  _In_opt_  LPSECURITY_ATTRIBUTES lpMutexAttributes,
+  _In_      BOOL bInitialOwner,
+  _In_opt_  LPCTSTR lpName
+);
+=end
+        safe_attach_function :CreateMutexW, [ :LPSECURITY_ATTRIBUTES, :BOOL, :LPCTSTR ], :HANDLE
+        safe_attach_function :CreateMutexA, [ :LPSECURITY_ATTRIBUTES, :BOOL, :LPCTSTR ], :HANDLE
+
+=begin
+DWORD WINAPI WaitForSingleObject(
+  _In_  HANDLE hHandle,
+  _In_  DWORD dwMilliseconds
+);
+=end
+        safe_attach_function :WaitForSingleObject, [ :HANDLE, :DWORD ], :DWORD
+
+=begin
+BOOL WINAPI ReleaseMutex(
+  _In_  HANDLE hMutex
+);
+=end
+        safe_attach_function :ReleaseMutex, [ :HANDLE ], :BOOL
+
+=begin
+HANDLE WINAPI OpenMutex(
+  _In_  DWORD dwDesiredAccess,
+  _In_  BOOL bInheritHandle,
+  _In_  LPCTSTR lpName
+);
+=end
+        safe_attach_function :OpenMutexW, [ :DWORD, :BOOL, :LPCTSTR ], :HANDLE
+        safe_attach_function :OpenMutexA, [ :DWORD, :BOOL, :LPCTSTR ], :HANDLE
+      end
+    end
+  end
+end
diff --git a/lib/chef/win32/mutex.rb b/lib/chef/win32/mutex.rb
new file mode 100644
index 0000000000..b0a9ba210e
--- /dev/null
+++ b/lib/chef/win32/mutex.rb
@@ -0,0 +1,94 @@
+#
+# Author:: Serdar Sutay (<serdar@opscode.com>)
+# Copyright:: Copyright 2013 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'chef/win32/api/synchronization'
+
+class Chef
+  module ReservedNames::Win32
+    class Mutex
+      include Chef::ReservedNames::Win32::API::Synchronization
+      extend Chef::ReservedNames::Win32::API::Synchronization
+
+      def initialize(name)
+        @name = name
+        # First check if there exists a mutex in the system with the
+        # given name.
+
+        # In the initial creation of the mutex initial_owner is set to
+        # false so that mutex will not be acquired until someone calls
+        # acquire.
+        # In order to call "*W" windows apis, strings needs to be
+        # encoded as wide strings.
+        @handle = CreateMutexW(nil, false, name.to_wstring)
+
+        # Fail early if we can't get a handle to the named mutex
+        if @handle == 0
+          Chef::Log.error("Failed to create system mutex with name'#{name}'")
+          Chef::ReservedNames::Win32::Error.raise!
+        end
+      end
+
+      attr_reader :handle
+      attr_reader :name
+
+      #####################################################
+      # Attempts to grab the mutex.
+      # Returns true if the mutex is grabbed or if it's already
+      # owned; false otherwise.
+      def test
+        WaitForSingleObject(handle, 0) == WAIT_OBJECT_0
+      end
+
+      #####################################################
+      # Attempts to grab the mutex and waits until it is acquired.
+      def wait
+        wait_result = WaitForSingleObject(handle, INFINITE)
+        case wait_result
+        when WAIT_ABANDONED
+          # Previous owner of the mutex died before it can release the
+          # mutex. Log a warning and continue.
+          Chef::Log.debug "Existing owner of the mutex exited prematurely."
+        when WAIT_OBJECT_0
+          # Mutex is successfully acquired.
+        else
+          Chef::Log.error("Failed to acquire system mutex '#{name}'. Return code: #{wait_result}")
+          Chef::ReservedNames::Win32::Error.raise!
+        end
+      end
+
+      #####################################################
+      # Releaes the mutex
+      def release
+        # http://msdn.microsoft.com/en-us/library/windows/desktop/ms685066(v=vs.85).aspx
+        # Note that release method needs to be called more than once
+        # if mutex is acquired more than once.
+        unless ReleaseMutex(handle)
+          # Don't fail things in here if we can't release the mutex.
+          # Because it will be automatically released when the owner
+          # of the process goes away and this class is only being used
+          # to synchronize chef-clients runs on a node.
+          Chef::Log.error("Can not release mutex '#{name}'. This might cause issues \
+if the mutex is attempted to be acquired by other threads.")
+          Chef::ReservedNames::Win32::Error.raise!
+        end
+      end
+    end
+  end
+end
+
+
diff --git a/spec/functional/run_lock_spec.rb b/spec/functional/run_lock_spec.rb
index ab0eb237ec..51645c0ae3 100644
--- a/spec/functional/run_lock_spec.rb
+++ b/spec/functional/run_lock_spec.rb
@@ -20,7 +20,7 @@ require 'chef/client'
 
 describe Chef::RunLock do
 
-  # This behavior is believed to work on windows, but the tests use UNIX APIs.
+  # This behavior works on windows, but the tests use fork :(
   describe "when locking the chef-client run", :unix_only => true do
 
     ##
diff --git a/spec/integration/solo/solo_spec.rb b/spec/integration/solo/solo_spec.rb
index 4ffb618311..c5341064fc 100644
--- a/spec/integration/solo/solo_spec.rb
+++ b/spec/integration/solo/solo_spec.rb
@@ -1,5 +1,9 @@
 require 'support/shared/integration/integration_helper'
 require 'chef/mixin/shell_out'
+require 'chef/run_lock'
+require 'chef/config'
+require 'timeout'
+require 'fileutils'
 
 describe "chef-solo" do
   extend IntegrationSupport
@@ -14,11 +18,76 @@ describe "chef-solo" do
 cookbook_path "#{path_to('cookbooks')}"
 file_cache_path "#{path_to('config/cache')}"
 EOM
-
       chef_dir = File.join(File.dirname(__FILE__), "..", "..", "..", "bin")
       result = shell_out("chef-solo -c \"#{path_to('config/solo.rb')}\" -o 'x::default' -l debug", :cwd => chef_dir)
       result.error!
     end
+  end
+
+  when_the_repository "has a cookbook with a recipe with sleep" do
+    directory 'logs'
+    file 'logs/runs.log', ''
+    file 'cookbooks/x/metadata.rb', 'version "1.0.0"'
+    file 'cookbooks/x/recipes/default.rb', <<EOM
+ruby_block "sleeping" do
+  block do
+    sleep 3
+  end
+end
+EOM
+    it "while running solo concurrently" do
+      file 'config/solo.rb', <<EOM
+cookbook_path "#{path_to('cookbooks')}"
+file_cache_path "#{path_to('config/cache')}"
+EOM
+      # We have a timeout protection here so that if due to some bug
+      # run_lock gets stuck we can discover it.
+      lambda {
+        Timeout.timeout(120) do
+          chef_dir = File.join(File.dirname(__FILE__), "..", "..", "..", "bin")
+
+          # Instantiate the first chef-solo run
+          s1 = Process.spawn("chef-solo -c \"#{path_to('config/solo.rb')}\" -o 'x::default' \
+-l debug -L #{path_to('logs/runs.log')}", :chdir => chef_dir)
+
+          # Give it some time to progress
+          sleep 3
+
+          # Instantiate the second chef-solo run
+          s2 = Process.spawn("chef-solo -c \"#{path_to('config/solo.rb')}\" -o 'x::default' \
+-l debug -L #{path_to('logs/runs.log')}", :chdir => chef_dir)
+
+          Process.waitpid(s1)
+          Process.waitpid(s2)
+        end
+      }.should_not raise_error(Timeout::Error)
+
+      # Unfortunately file / directory helpers in integration tests
+      # are implemented using before(:each) so we need to do all below
+      # checks in one example.
+      run_log = File.read(path_to('logs/runs.log'))
+
+      # both of the runs should succeed
+      run_log.lines.reject {|l| !l.include? "INFO: Chef Run complete in"}.length.should == 2
+
+      # second run should have a message which indicates it's waiting for the first run
+      pid_lines = run_log.lines.reject {|l| !l.include? "Chef-client pid:"}
+      pid_lines.length.should == 2
+      pids = pid_lines.map {|l| l.split(" ").last}
+      run_log.should include("Chef client #{pids[0]} is running, will wait for it to finish and then run.")
+
+      # second run should start after first run ends
+      starts = [ ]
+      ends = [ ]
+      run_log.lines.each_with_index do |line, index|
+        if line.include? "Chef-client pid:"
+          starts << index
+        elsif line.include? "INFO: Chef Run complete in"
+          ends << index
+        end
+      end
+      starts[1].should > ends[0]
+    end
 
   end
 end