Proposal: Add new property for getting process path

[Giorgi]

Rationale and Usage

In order to get process path you need to use someProcess.MainModule.FileName property. However it suffers from a couple of issue:

• Accessing MainModule of 64 bit process from a 32 bit one throws an exception
• You can't access MainModule of elevated process from a non elevated one

As you can see many developers encounter these issues.

I suggest adding a new property which returns the path to executable of the Process. To overcome the issues that I described we can use QueryFullProcessImageName together with PROCESS_QUERY_LIMITED_INFORMATION access right.

Api Proposal

namespace System.Diagnostics
{
     public class  Process : Component
     {
           public string MainModuleFilePath { get; }
     }
}

Open Questions

Should the new property be called MainModuleFilePath or something else? From my own experience most of people use Path when asking question on stackoverflow but @Priya91 suggested calling it MainModuleFilePath as it is returning Path of the MainModule property.

Cross Platform Support

To implement this property on other platforms on Linux we can use readlink to read the value of "/proc/%d/exe" symlink and on OS X we can use proc_pidpath which returns process path given process pid.

[ellismg]

If we do this, on Linux, we could use /proc//exe to figure out where the on disk image is. There are some caveats listed in the man pages, but in general I think it would be OK. Not sure how (or if) we can do this on OS X.

[mikedn]

Possibly related discussion/work in progress: dotnet/roslyn#884 (diff)

[stephentoub]

on Linux, we could use /proc//exe to figure out where the on disk image is.

Yeah, we already use this in our Process implementation on Linux. In order to do path resolution for the file name passed to Start, we use /proc/self/exe to get the path of the exe and look in its directory.

[Giorgi]

This has two ways for getting process path on Mac:
Programatically retrieving the absolute path of an OS X command-line app

[xanather]

This is needed! Environment.ExecutingPath? Something like that. Ive always had to play with reflection, AppDomains (doesn't exist in .Net Core), or use System.Windows.Forms.Application.StartupPath (doesn't exist in .Net Core) to get such a simple thing.

[mikedn]

@xanather This issue is specific to the System.Diagnostics.Process class, you seem to be asking for something different.

AppDomains (doesn't exist in .Net Core),

There is AppContext.

[terrajobst]

If Process.MainModule will end up throwing on certain platforms, we should also expose a query API, like Process.CanQueryModuleInformation or something along those lines.

[Giorgi]

It already throws exception if the target process is elevated and the caller isn't

[Priya91]

@Giorgi Can you provide an api proposal, a good example here

[Giorgi]

The api is very simple:

 string Path { get; }

[Priya91]

@Giorgi The api proposal needs to be in the form of a speclet as mentioned in the link. You can edit your first comment on this issue, with the api speclet, mainly outlining on the usage scenarios to justify need of the API and the API structure like below. This will help during api review meeting to get to the necessary information quickly rather that perusing the whole issue.

Note: Path doesn't seem very obvious that it's indicating the main module file path, so renamed API below. Open to other suggestions.

namespace System.Diagnostics
{
     public class  Process : Component
     {
           public string MainModuleFilePath { get; }    
     }
}

[Giorgi]

@Priya91 Post updated.

[Priya91]

@Giorgi The proposal looks good, you have mentioned that it's possible to implement on windows, can you also update the post on similar investigations on Linux and OSX. .NETCore supports Linux and OSX, so we want to have data on whether this API returns meaningful value on other platforms as well.

[Giorgi]

@Priya91 Done!

[danmoseley]

@Priya91 I marked as ready for review per your note above, feel free to flip back if there is still more feedback.

[terrajobst]

We just reviewed this today:

• I understand that accessing the main module currently throws. Any reason we cannot make this API work and have other properties on ProcessModule throw if the bitness differs or the process is elevated?
• The concern being, that adding multiple properties to access the same information might make the API more confusing.

@karelz @Priya91 @Giorgi: could you answer the question?

[mellinoe]

I understand that accessing the main module currently throws.

I don't think that's true, barring the caveats in the original post (e.g. 64->32-bit or non-elevated->elevated processes on Windows). We just recently started using it in the corefx tests here, and I do believe that code is being run on all of our platforms.

EDIT: To clarify, I just mean that the API is working outside of Windows. There was some discussion above indicating that it might only work on Windows. There are probably still issues with Windows bitness and elevated-ness.

[Priya91]

Any reason we cannot make this API work and have other properties on ProcessModule throw if the bitness differs or the process is elevated?

We can defer throwing exceptions in the implementation to when the properties are accessed, and use a different win32 API for ProcessModule.FileName, but that will cause a behavior change from desktop. And there could be code, for, if MainModule is null assume querying process is elevated so no access, or if MainModule throws, assume different bitness, hence do something else, etc.

[stephentoub]

From an implementation standpoint, in what situations is it possible to provide MainModuleFilePath and not MainModule.FilePath? On macOS we simply construct a single ProcessModule from getting the file path (https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.OSX.cs#L113-L123), so if we can get the latter we can get the former. Similarly for Linux, we get the module information by parsing a virtual file from procfs (https://github.com/dotnet/corefx/blob/master/src/System.Diagnostics.Process/src/System/Diagnostics/ProcessManager.Linux.cs#L51), so if we can get the file path, we can get the rest of the data.

Or is this just an issue on Windows?

[Giorgi]

@stephentoub I haven't tried it on other platforms but I do have encountered this issue on Windows. Here is this bug on Microsoft connect with a small repro and workaround: https://connect.microsoft.com/VisualStudio/feedback/details/595717/add-path-property-to-process-class-so-that-it-works-with-elavated-processes-too

[wtgodbe]

@Giorgi @Priya91 @stephentoub do we still plan on taking this change/was a PR ever put up for this issue? Would like to get closure one way or another.

[stephentoub]

The API is still marked "api-needs-work", which suggests additional follow-up is necessary before it can be re-reviewed. And the milestone is set as future, which suggests there's no rush here. What kind of closure are you looking for, and why?

[Giorgi]

@wtgodbe I haven't sent a PR because the suggested api was rejected but I can't think of a better api for getting process path.

I think it would be very intuitive to have a Path property for accessing process path instead of using MainModule.FileName and it would also work in cases when accessing MainModule throws an exception.

[wtgodbe]

I will leave this open then, if you or someone else can come up with a proposal that addresses @terrajobst concerns then feel free to do so.

[Giorgi]

@wtgodbe As @Priya91 has said in her comment addressing @terrajobst concerns isn't possible without introducing breaking change so I don't think there is any other way.

[Giorgi]

@terrajobst @mellinoe @stephentoub What if we add an extension method in Windows Compatibility Pack ?

[eiriktsarpalis]

This seems like a reasonable request, however given there has been limited interest for producing an API proposal, perhaps we should close?

[Giorgi]

@eiriktsarpalis As I have already said I haven't sent a PR because the suggested api was rejected but I can't think of a better api for getting process path.

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.