Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Canceling a build can fail if a process running on the computer is protected #6477

Open
DamienKochanek opened this issue May 24, 2021 · 3 comments
Labels
bug help wanted Issues that the core team doesn't plan to work on, but would accept a PR for. Comment to claim. triaged
Milestone

Comments

@DamienKochanek
Copy link

DamienKochanek commented May 24, 2021

Issue Description

Canceling a NMake project (not relevant here but it's Unreal Engine 4) through CTRL+BREAK in Visual Studio doesn't work when some (unrelated) processes are running on the computer.

Steps to Reproduce

Prerequisites: have a process running which will deny access to its start time (for instance, OpenVPN, see Analysis section).

  1. Build Project in Visual Studio
  2. Cancel build (CTRL+BREAK)

Notice an MSBuild error (see Actual Behavior section).

Expected Behavior

We should be able to cancel a build without an error. (Or at least just #5508 😄)

Actual Behavior

Cancelling a build doesn't work and provokes the following error:

2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): warning MSB5021: Terminating the task executable "cmd" and its child processes because the build was canceled.
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003: The specified task executable "cmd.exe" could not be run. System.ComponentModel.Win32Exception (0x80004005): Access is denied
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at System.Diagnostics.ProcessManager.OpenProcess(Int32 processId, Int32 access, Boolean throwIfExited)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at System.Diagnostics.Process.GetProcessHandle(Int32 access, Boolean throwIfExited)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at System.Diagnostics.Process.GetProcessTimes()
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at System.Diagnostics.Process.get_StartTime()
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Shared.NativeMethodsShared.GetChildProcessIds(Int32 parentProcessId, DateTime parentStartTime)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Shared.NativeMethodsShared.KillTree(Int32 processIdToKill)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Utilities.ProcessExtensions.KillTree(Process process, Int32 timeout)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.KillToolProcessOnTimeout(Process proc, Boolean isBeingCancelled)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.TerminateToolProcess(Process proc, Boolean isBeingCancelled)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.HandleToolNotifications(Process proc)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Tasks.Exec.ExecuteTool(String pathToTool, String responseFileCommands, String commandLineCommands)
2>C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB6003:    at Microsoft.Build.Utilities.ToolTask.Execute()

Analysis

Looking at the code, I think the issue is with Shared.NativeMethodsShared.GetChildProcessIds which fails if, while iterating through the processes currently running, one of them is protected.

In my case, I managed to identify the faulty process: OpenVPN 2.5.1. When connected to the VPN, it creates a process which makes GetChildProcessIds() fail.

More precisely, Process.StartTime throws Win32Exception 0x80004005 "Access is denied" and it's not caught.

I also managed to reproduce the error by copying the following code from msbuild in a simple project and calling it:

		[SuppressMessage("Microsoft.Design", "CA1060:MovePInvokesToNativeMethodsClass", Justification = "Class name is NativeMethodsShared for increased clarity")]
		[DllImport("KERNEL32.DLL")]
		private static extern SafeProcessHandle OpenProcess(eDesiredAccess dwDesiredAccess, [MarshalAs(UnmanagedType.Bool)] bool bInheritHandle, int dwProcessId);

		internal static List<KeyValuePair<int, SafeProcessHandle>> GetChildProcessIds(int parentProcessId, DateTime parentStartTime)
		{
			List<KeyValuePair<int, SafeProcessHandle>> myChildren = new List<KeyValuePair<int, SafeProcessHandle>>();

			foreach (Process possibleChildProcess in Process.GetProcesses())
			{
				using (possibleChildProcess)
				{
					Log.TraceInformation("Process:");
					Log.TraceInformation(possibleChildProcess.ToString());
					// Hold the child process handle open so that children cannot die and restart with a different parent after we've started looking at it.
					// This way, any handle we pass back is guaranteed to be one of our actual children.
					SafeProcessHandle childHandle = OpenProcess(eDesiredAccess.PROCESS_QUERY_INFORMATION, false, possibleChildProcess.Id);
					if (childHandle.IsInvalid)
					{
						continue;
					}

					bool keepHandle = false;
					try
					{
						if (possibleChildProcess.StartTime > parentStartTime)
						{
						}
					}
					finally
					{
						if (!keepHandle)
						{
							childHandle.Dispose();
						}
					}
				}
			}

			return myChildren;
		}

Versions & Configurations

16.9.0.16703

Attach a binlog

@DamienKochanek DamienKochanek added bug needs-triage Have yet to determine what bucket this goes in. labels May 24, 2021
@benvillalobos benvillalobos added help wanted Issues that the core team doesn't plan to work on, but would accept a PR for. Comment to claim. and removed needs-triage Have yet to determine what bucket this goes in. labels May 26, 2021
@benvillalobos benvillalobos added this to the Backlog milestone May 26, 2021
@benvillalobos
Copy link
Member

benvillalobos commented May 26, 2021

Team Triage: We agree, that shouldn't be fatal. The solution here would be to add a catch for this

@JaynieBai JaynieBai self-assigned this Sep 19, 2022
@benvillalobos
Copy link
Member

I tried your repro (the code you posted) with a few admin level processes, but I can't repro your scenario. Is OpenVPN the only way to repro your scenario?

@JaynieBai
Copy link
Member

@DamienKochanek I can't repro this issue either. Could you repro this issue now?

@JaynieBai JaynieBai removed their assignment Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug help wanted Issues that the core team doesn't plan to work on, but would accept a PR for. Comment to claim. triaged
Projects
None yet
Development

No branches or pull requests

3 participants