From ce56da69b2df3d5fda1044887ff83345bbcd81f7 Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Tue, 14 Mar 2017 16:37:55 -0700 Subject: [PATCH] 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. --- lib/ansible/plugins/shell/powershell.py | 83 +++++++++++++++---- .../targets/win_async_wrapper/tasks/main.yml | 6 ++ 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) { @@ -617,6 +635,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 { @@ -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: