make windows async ... async (#22624)

Fixes #22575 - issue under new exec wrapper where unconstrained handle inheritance (for stdin) caused WinRM to block on breakaway processes. Uses explicit handle inheritance to ensure that only stdin read handle gets inherited. Adds test to ensure that async is actually async.

2 files changed, 75 insertions, 14 deletions

diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py
index 24f11e21eb..4a81a594cc 100644
--- a/lib/ansible/plugins/shell/powershell.py
+++ b/lib/ansible/plugins/shell/powershell.py
@@ -364,7 +364,13 @@ Function Run($payload) {
 
         [Ansible.Shell.ProcessUtil]::GrantAccessToWindowStationAndDesktop($username)
 
-        $proc.Start() | Out-Null # will always return $true for non shell-exec cases
+        Try {
+            $proc.Start() | Out-Null # will always return $true for non shell-exec cases
+        }
+        Catch {
+            Write-Output $_.Exception.InnerException
+            return
+        }
 
         $payload_string = $payload | ConvertTo-Json -Depth 99 -Compress
 
@@ -494,7 +500,7 @@ Function Run($payload) {
                     IntPtr lpEnvironment,
                     [MarshalAs(UnmanagedType.LPTStr)]
                     string lpCurrentDirectory,
-                    STARTUPINFO lpStartupInfo,
+                    STARTUPINFOEX lpStartupInfo,
                     out PROCESS_INFORMATION lpProcessInformation);
 
                 [DllImport("kernel32.dll", SetLastError=true, CharSet=CharSet.Unicode)]
@@ -516,6 +522,18 @@ Function Run($payload) {
                 [DllImport("kernel32.dll", SetLastError=true)]
                 public static extern bool SetHandleInformation(IntPtr hObject, HandleFlags dwMask, int dwFlags);
 
+                [DllImport("kernel32.dll", SetLastError=true)]
+                public static extern bool InitializeProcThreadAttributeList(IntPtr lpAttributeList, int dwAttributeCount, int dwFlags, ref int lpSize);
+
+                [DllImport("kernel32.dll", SetLastError=true)]
+                public static extern bool UpdateProcThreadAttribute(
+                     IntPtr lpAttributeList,
+                     uint dwFlags,
+                     IntPtr Attribute,
+                     IntPtr lpValue,
+                     IntPtr cbSize,
+                     IntPtr lpPreviousValue,
+                     IntPtr lpReturnSize);
 
                 public static string SearchPath(string findThis)
                 {
@@ -618,6 +636,17 @@ Function Run($payload) {
             }
 
             [StructLayout(LayoutKind.Sequential)]
+            public class STARTUPINFOEX {
+                public STARTUPINFO startupInfo;
+                public IntPtr lpAttributeList;
+
+                public STARTUPINFOEX() {
+                    startupInfo = new STARTUPINFO();
+                    startupInfo.cb = Marshal.SizeOf(this);
+                }
+            }
+
+            [StructLayout(LayoutKind.Sequential)]
             public struct PROCESS_INFORMATION
             {
                 public IntPtr hProcess;
@@ -669,22 +698,24 @@ Function Run($payload) {
 
     # FUTURE: create under new job to ensure all children die on exit?
 
-    # FUTURE: move these flags into C# enum
+    # FUTURE: move these flags into C# enum?
     # start process suspended + breakaway so we can record the watchdog pid without worrying about a completion race
     Set-Variable CREATE_BREAKAWAY_FROM_JOB -Value ([uint32]0x01000000) -Option Constant
     Set-Variable CREATE_SUSPENDED -Value ([uint32]0x00000004) -Option Constant
     Set-Variable CREATE_UNICODE_ENVIRONMENT -Value ([uint32]0x000000400) -Option Constant
     Set-Variable CREATE_NEW_CONSOLE -Value ([uint32]0x00000010) -Option Constant
+    Set-Variable EXTENDED_STARTUPINFO_PRESENT -Value ([uint32]0x00080000) -Option Constant
 
-    $pstartup_flags = $CREATE_BREAKAWAY_FROM_JOB -bor $CREATE_UNICODE_ENVIRONMENT -bor $CREATE_NEW_CONSOLE -bor $CREATE_SUSPENDED
+    $pstartup_flags = $CREATE_BREAKAWAY_FROM_JOB -bor $CREATE_UNICODE_ENVIRONMENT -bor $CREATE_NEW_CONSOLE `
+        -bor $CREATE_SUSPENDED -bor $EXTENDED_STARTUPINFO_PRESENT
 
-    # execute the dynamic watchdog as a breakway process, which will in turn exec the module
-    $si = New-Object Ansible.Async.STARTUPINFO
+    # execute the dynamic watchdog as a breakway process to free us from the WinRM job, which will in turn exec the module
+    $si = New-Object Ansible.Async.STARTUPINFOEX
 
     # setup stdin redirection, we'll leave stdout/stderr as normal
-    $si.dwFlags = [Ansible.Async.StartupInfoFlags]::USESTDHANDLES
-    $si.hStdOutput = [Ansible.Async.NativeProcessUtil]::GetStdHandle([Ansible.Async.StandardHandleValues]::STD_OUTPUT_HANDLE)
-    $si.hStdError = [Ansible.Async.NativeProcessUtil]::GetStdHandle([Ansible.Async.StandardHandleValues]::STD_ERROR_HANDLE)
+    $si.startupInfo.dwFlags = [Ansible.Async.StartupInfoFlags]::USESTDHANDLES
+    $si.startupInfo.hStdOutput = [Ansible.Async.NativeProcessUtil]::GetStdHandle([Ansible.Async.StandardHandleValues]::STD_OUTPUT_HANDLE)
+    $si.startupInfo.hStdError = [Ansible.Async.NativeProcessUtil]::GetStdHandle([Ansible.Async.StandardHandleValues]::STD_ERROR_HANDLE)
 
     $stdin_read = $stdin_write = 0
 
@@ -697,7 +728,35 @@ Function Run($payload) {
     If(-not [Ansible.Async.NativeProcessUtil]::SetHandleInformation($stdin_write, [Ansible.Async.HandleFlags]::INHERIT, 0)) {
         throw "Stdin handle setup failed, Win32Error: $([System.Runtime.InteropServices.Marshal]::GetLastWin32Error())"
     }
-    $si.hStdInput = $stdin_read
+    $si.startupInfo.hStdInput = $stdin_read
+
+    # create an attribute list with our explicit handle inheritance list to pass to CreateProcess
+    [int]$buf_sz = 0
+
+    # determine the buffer size necessary for our attribute list
+    If(-not [Ansible.Async.NativeProcessUtil]::InitializeProcThreadAttributeList([IntPtr]::Zero, 1, 0, [ref]$buf_sz)) {
+        $last_err = [System.Runtime.InteropServices.Marshal]::GetLastWin32Error()
+        If($last_err -ne 122) { # ERROR_INSUFFICIENT_BUFFER
+            throw "Attribute list size query failed, Win32Error: $last_err"
+        }
+    }
+
+    $si.lpAttributeList = [System.Runtime.InteropServices.Marshal]::AllocHGlobal($buf_sz)
+
+    # initialize the attribute list
+    If(-not [Ansible.Async.NativeProcessUtil]::InitializeProcThreadAttributeList($si.lpAttributeList, 1, 0, [ref]$buf_sz)) {
+        throw "Attribute list init failed, Win32Error: $([System.Runtime.InteropServices.Marshal]::GetLastWin32Error())"
+    }
+
+    $handles_to_inherit = [IntPtr[]]@($stdin_read)
+    $pinned_handles = [System.Runtime.InteropServices.GCHandle]::Alloc($handles_to_inherit, [System.Runtime.InteropServices.GCHandleType]::Pinned)
+
+    # update the attribute list with the handles we want to inherit
+    If(-not [Ansible.Async.NativeProcessUtil]::UpdateProcThreadAttribute($si.lpAttributeList, 0, 0x20002 <# PROC_THREAD_ATTRIBUTE_HANDLE_LIST #>, `
+        $pinned_handles.AddrOfPinnedObject(), [System.Runtime.InteropServices.Marshal]::SizeOf([type][IntPtr]) * $handles_to_inherit.Length, `
+        [System.IntPtr]::Zero, [System.IntPtr]::Zero)) {
+        throw "Attribute list update failed, Win32Error: $([System.Runtime.InteropServices.Marshal]::GetLastWin32Error())"
+    }
 
     # need to use a preamble-free version of UTF8Encoding
     $utf8_encoding = New-Object System.Text.UTF8Encoding @($false)
@@ -894,12 +953,8 @@ class ShellModule(object):
 
     # TODO: implement module transfer
     # TODO: implement #Requires -Modules parser/locator
-    # TODO: add raw failure + errcode preservation (all success right now)
     # TODO: add KEEP_REMOTE_FILES support + debug wrapper dump
-    # TODO: add become support
     # TODO: add binary module support
-    # TODO: figure out non-pipelined path (or force pipelining)
-
 
     def assert_safe_env_key(self, key):
         if not self.safe_envkey.match(key):
diff --git a/test/integration/targets/win_async_wrapper/tasks/main.yml b/test/integration/targets/win_async_wrapper/tasks/main.yml
index 8a2fd351ab..fc8f154f14 100644
--- a/test/integration/targets/win_async_wrapper/tasks/main.yml
+++ b/test/integration/targets/win_async_wrapper/tasks/main.yml
@@ -1,3 +1,7 @@
+- name: capture timestamp before fire and forget
+  set_fact:
+    start_timestamp: "{{ lookup('pipe', 'date +%s') }}"
+
 - name: async fire and forget
   async_test:
     sleep_delay_sec: 5
@@ -12,6 +16,8 @@
     - asyncresult.started == 1
     - asyncresult.finished == 0
     - asyncresult.results_file is search('\.ansible_async.+\d+\.\d+')
+     # ensure that async is actually async- this test will fail if # hosts > forks or if the target host is VERY slow
+    - (lookup('pipe', 'date +%s') | int) - (start_timestamp | int) < 5
 
 - name: async poll immediate success
   async_test: