From 1834baaf9d44778764727a96ca892751a77fb24f Mon Sep 17 00:00:00 2001
From: Jason Frey <fryguy9@gmail.com>
Date: Fri, 11 Oct 2024 17:03:51 -0400
Subject: [PATCH] Fix sporadic ansible-runner macOS bug and remove extra
 listener thread

It was found that on macOS a sporadic failure would occur where the internal listener,
while started, wouldn't actually start listening right away, causing the listener to
miss the file we were waiting for.

The listen gem [creates an internal thread, `@run_thread`][1], which on most target systems
is where the actually listening is done. However, [on macOS, `@run_thread` creates a
second thread, `@worker_thread`][2], which does the actual listening. It's possible that
although the listener is started, the @worker_thread hasn't actually started yet.
This leaves a window where the target_path we are waiting on can actually be created
before the `@worker_thread` is started and we "miss" the creation of the target_path.
This commit ensures that we won't move on until that thread is ready, further ensuring
we can't miss the creation of the target_path.

Ansible::Runner#wait_for was also creating an extra thread when starting the listener,
but this thread is unnecessary as the listen gem creates its own internal thread under
the covers.

[1]: https://github.com/guard/listen/blob/f186b2fa159a2458f3ff7e8680c3a4fcbdc636d1/lib/listen/adapter/base.rb#L75
[2]: https://github.com/guard/listen/blob/f186b2fa159a2458f3ff7e8680c3a4fcbdc636d1/lib/listen/adapter/darwin.rb#L49
---
 lib/ansible/runner.rb                     | 25 ++++++++++++++---------
 spec/lib/ansible/runner_execution_spec.rb |  8 ++++++++
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/lib/ansible/runner.rb b/lib/ansible/runner.rb
index b305c93e721..f08ab02e8bd 100644
--- a/lib/ansible/runner.rb
+++ b/lib/ansible/runner.rb
@@ -378,15 +378,21 @@ def wait_for(base_dir, target_path, timeout: 10.seconds)
         listener = Listen.to(base_dir, :only => %r{\A#{target_path}\z}) do |modified, added, _removed|
           path_created.set if added.include?(base_dir.join(target_path).to_s) || modified.include?(base_dir.join(target_path).to_s)
         end
-
-        thread = Thread.new do
-          listener.start
-        rescue ArgumentError => err
-          # If the main thread raises an exception immediately it is possible
-          # for the ensure block to call `listener.stop` before this thread
-          # begins its execution resulting in an ArgumentError due to the state
-          # being `:stopped`
-          raise unless err.message.include?("cannot start from state :stopped")
+        listener.start
+
+        # The listen gem creates an internal thread, @run_thread, which on most target systems
+        # is where the actually listening is done. However, on macOS, @run_thread creates a
+        # second thread, @worker_thread, which does the actual listening. It's possible that
+        # although the listener is started, the @worker_thread hasn't actually started yet.
+        # This leaves a window where the target_path we are waiting on can actually be created
+        # before the @worker_thread is started and we "miss" the creation of the target_path.
+        # This line ensures that we won't move on until that thread is ready, further ensuring
+        # we can't miss the creation of the target_path.
+        if RbConfig::CONFIG['host_os'].include?("darwin")
+          listener_adapter = listener.instance_variable_get(:@backend).instance_variable_get(:@adapter)
+          until listener_adapter.instance_variable_get(:@worker_thread)&.alive?
+            sleep(0.01) # yield to other threads to allow them to start
+          end
         end
 
         begin
@@ -394,7 +400,6 @@ def wait_for(base_dir, target_path, timeout: 10.seconds)
           raise "Timed out waiting for #{target_path}" unless path_created.wait(timeout)
         ensure
           listener.stop
-          thread.join
         end
 
         res
diff --git a/spec/lib/ansible/runner_execution_spec.rb b/spec/lib/ansible/runner_execution_spec.rb
index 85c97bdec74..71bdb18a33c 100644
--- a/spec/lib/ansible/runner_execution_spec.rb
+++ b/spec/lib/ansible/runner_execution_spec.rb
@@ -72,6 +72,14 @@ def expect_ansible_runner_success(response)
         expect(response.human_stdout).to include('"msg": "Hello World! example_var=\'example var value\'"')
       end
     end
+
+    it "with a payload that fails before running even starts" do
+      playbook = data_directory.join("hello_world.yml")
+
+      expect(AwesomeSpawn).to receive(:run).and_raise(RuntimeError.new("Some failure"))
+
+      expect { Ansible::Runner.public_send(method_under_test, env_vars, extra_vars, playbook) }.to raise_error(RuntimeError, "Some failure")
+    end
   end
 
   describe ".run" do