Merge pull request #9386 from tvpartytonight/PA-6394

(PA-6394) Stop using FFI threads and use ruby threads for the windows puppet daemon

Author: joshcooper

Diff for: acceptance/tests/windows/service_manager_integration.rb

@@ -0,0 +1,51 @@
+test_name 'Test agent state via service control manager' do
+
+  tag 'audit:integration'
+
+  confine :to, platform: 'windows'
+
+  teardown do
+    agents.each do |agent|
+      state = query_agent_state(agent)
+      if state != "STOPPED"
+        stop_puppet_windows_daemon(agent)
+        ensure_agent_state(agent, "STOPPED")
+      end
+    end
+  end
+
+  def query_agent_state(host)
+    on(host, 'sc query puppet').stdout.match(/STATE.+\s{1}(\w+)/)[1]
+  end
+
+  def start_puppet_windows_daemon(host)
+    on(host, 'sc start puppet')
+  end
+
+  def stop_puppet_windows_daemon(host)
+    on(host, 'sc stop puppet')
+  end
+
+  def ensure_agent_state(host, state)
+    retry_attempts = 0
+    while retry_attempts < 5
+      return if state == query_agent_state(host)
+      retry_attempts += 1
+      sleep 1
+    end
+    fail_test "State not #{state} after 5 tries"
+  end
+
+  step 'store initial state' do
+
+    agents.each do |agent|
+      initial_state = query_agent_state(agent)
+      assert_match("STOPPED", initial_state, "agent daemon should initially be stopped")
+
+      start_puppet_windows_daemon(agent)
+      ensure_agent_state(agent, "RUNNING")
+      stop_puppet_windows_daemon(agent)
+      ensure_agent_state(agent, "STOPPED")
+    end
+  end
+end

Diff for: lib/puppet/util/windows/daemon.rb

@@ -187,25 +187,6 @@ class Daemon
       SetTheServiceStatus.call(SERVICE_STOPPED, NO_ERROR, 0, 0)
     end
 
-    ThreadProc = FFI::Function.new(:ulong, [:pointer]) do |lpParameter|
-      ste = FFI::MemoryPointer.new(SERVICE_TABLE_ENTRYW, 2)
-
-      s = SERVICE_TABLE_ENTRYW.new(ste[0])
-      s[:lpServiceName] = FFI::MemoryPointer.from_string('')
-      s[:lpServiceProc] = lpParameter
-
-      s = SERVICE_TABLE_ENTRYW.new(ste[1])
-      s[:lpServiceName] = nil
-      s[:lpServiceProc] = nil
-
-      # No service to step, no service handle, no ruby exceptions, just terminate the thread..
-      unless StartServiceCtrlDispatcherW(ste)
-        return 1
-      end
-
-      return 0
-    end
-
     # This is a shortcut for Daemon.new + Daemon#mainloop.
     #
     def self.mainloop
@@ -255,26 +236,28 @@ def mainloop
         raise SystemCallError.new('CreateEvent', FFI.errno)
       end
 
-      hThread = CreateThread(nil, 0, ThreadProc, Service_Main, 0, nil)
+      hThread = Thread.new do
+        ste = FFI::MemoryPointer.new(SERVICE_TABLE_ENTRYW, 2)
 
-      if hThread == 0
-        raise SystemCallError.new('CreateThread', FFI.errno)
-      end
+        s = SERVICE_TABLE_ENTRYW.new(ste[0])
+        s[:lpServiceName] = FFI::MemoryPointer.from_string("")
+        s[:lpServiceProc] = Service_Main
 
-      events = FFI::MemoryPointer.new(:pointer, 2)
-      events.put_pointer(0, FFI::Pointer.new(hThread))
-      events.put_pointer(FFI::Pointer.size, FFI::Pointer.new(@@hStartEvent))
+        s = SERVICE_TABLE_ENTRYW.new(ste[1])
+        s[:lpServiceName] = nil
+        s[:lpServiceProc] = nil
 
-      while (index = WaitForMultipleObjects(2, events, 0, 1000)) == WAIT_TIMEOUT
+        # No service to step, no service handle, no ruby exceptions, just terminate the thread..
+        StartServiceCtrlDispatcherW(ste)
       end
 
-      if index == WAIT_FAILED
-        raise SystemCallError.new('WaitForMultipleObjects', FFI.errno)
+      while (index = WaitForSingleObject(@@hStartEvent, 1000)) == WAIT_TIMEOUT
+        # The thread exited, so the show is off.
+        raise "Service_Main thread exited abnormally" unless hThread.alive?
       end
 
-      # The thread exited, so the show is off.
-      if index == WAIT_OBJECT_0
-        raise "Service_Main thread exited abnormally"
+      if index == WAIT_FAILED
+        raise SystemCallError.new("WaitForSingleObject", FFI.errno)
       end
 
       thr = Thread.new do