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

[API Proposal]: ProcessStartInfo.CreationFlags (for Windows only) #71515

Open
alexrp opened this issue Jul 1, 2022 · 29 comments
Open

[API Proposal]: ProcessStartInfo.CreationFlags (for Windows only) #71515

alexrp opened this issue Jul 1, 2022 · 29 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Diagnostics.Process
Milestone

Comments

@alexrp
Copy link
Contributor

alexrp commented Jul 1, 2022

Background and motivation

The Process API currently provides no way to control the process creation flags used on Windows. This is unfortunate as there are a number of flags that are quite useful, such as CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, and DEBUG_PROCESS.

Of course, you can P/Invoke CreateProcessW to use these flags and this works for extremely trivial use cases. But as soon as you need anything more than "just run an executable with these flags", things get hairy: You have to reimplement environment block construction, string[] -> string command line pasting, and standard I/O piping. Not a fun experience at all.

I propose that ProcessStartInfo simply gets a Windows-specific CreationFlags property to set the process creation flags. These would just get OR'd with whichever flags Process itself needs to set based on other properties (e.g. CREATE_NO_WINDOW).

The implementation effort here should be extremely minimal while providing huge convenience to people who need to set process creation flags on Windows.

API Proposal

namespace System.Diagnostics.Process;

public sealed partial class ProcessStartInfo
{
    [SupportedOSPlatform("windows")]
    public uint CreationFlags { get; set; }
}
  • I don't think there needs to be any validation of the flags beyond what CreateProcessW might do; the assumption would be that anyone using this property knows what they're doing.
  • I'm agnostic on whether the property should throw PlatformNotSupportedException on non-Windows platforms vs just being ignored.
  • CreationFlags could be strongly typed as an enum, but I'm not sure the BCL should be in the business of exposing such an enum and keeping it up to date with the Win32 API.

API Usage

var info = new ProcessStartInfo(fileName);

foreach (var arg in args)
    info.ArgumentList.Add(arg);

info.CreationFlags = 0x4 /* CREATE_SUSPENDED */;

using var proc = new Process
{
    StartInfo = info,
};

proc.Start();

// Do some Win32-specific stuff that ultimately causes the process to be woken up.

proc.WaitForExit();

Alternative Designs

Individual properties for each flag is also an option.

Risks

None.

@alexrp alexrp added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jul 1, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jul 1, 2022
@ghost
Copy link

ghost commented Jul 1, 2022

Tagging subscribers to this area: @dotnet/area-system-diagnostics-process
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The Process API currently provides no way to control the process creation flags used on Windows. This is unfortunate as there are a number of flags that are quite useful, such as CREATE_NEW_PROCESS_GROUP, CREATE_SUSPENDED, and DEBUG_PROCESS.

Of course, you can P/Invoke CreateProcessW to use these flags and this works for extremely trivial use cases. But as soon as you need anything more than "just run an executable with these flags", things get hairy: You have to reimplement environment block construction, string[] -> string command line pasting, and standard I/O piping. Not a fun experience at all.

I propose that ProcessStartInfo simply gets a Windows-specific CreationFlags property to set the process creation flags. These would just get OR'd with whichever flags Process itself needs to set based on other properties (e.g. CREATE_NO_WINDOW).

The implementation effort here should be extremely minimal while providing huge convenience to people who need to set process creation flags on Windows.

API Proposal

namespace System.Diagnostics.Process;

public sealed partial class ProcessStartInfo
{
    [SupportedOSPlatform("windows")]
    public uint CreationFlags { get; set; }
}
  • I don't think there needs to be any validation of the flags beyond what CreateProcessW might do; the assumption would be that anyone using this property knows what they're doing.
  • I'm agnostic on whether the property should throw PlatformNotSupportedException on non-Windows platforms vs just being ignored.
  • CreationFlags could be strongly typed as an enum, but I'm not sure the BCL should be in the business of exposing such an enum and keeping it up to date with the Win32 API.

API Usage

var info = new ProcessStartInfo(fileName);

foreach (var arg in args)
    info.ArgumentList.Add(arg);

info.CreationFlags = 0x4 /* CREATE_SUSPENDED */;

using var proc = new Process
{
    StartInfo = info,
};

proc.Start();

// Do some Win32-specific stuff that ultimately causes the process to be woken up.

proc.WaitForExit();

Alternative Designs

None.

Risks

None.

Author: alexrp
Assignees: -
Labels:

api-suggestion, area-System.Diagnostics.Process

Milestone: -

@jkotas
Copy link
Member

jkotas commented Jul 1, 2022

There are number of other Windows-specific parameters that ProcessStartInfo does not expose. For example, all parameters from https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute

If one needs a full control over the Windows-specific parameters, it is better to PInvoke CreateProcess Windows API directly. For example, using https://github.com/dotnet/pinvoke/ .

@alexrp
Copy link
Contributor Author

alexrp commented Jul 1, 2022

There are number of other Windows-specific parameters that ProcessStartInfo does not expose. For example, all parameters from https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-updateprocthreadattribute

Yes, but these aren't simple flags. The complexity of exposing this functionality would be considerably higher, and I'm also not sure to what extent anyone actually needs/wants them.

By contrast, I have 3 separate projects where I have the need to add extra process creation flags.

If one needs a full control over the Windows-specific parameters, it is better to PInvoke CreateProcess Windows API directly. For example, using https://github.com/dotnet/pinvoke/ .

This ignores what I wrote in the motivation section:

You have to reimplement environment block construction, string[] -> string command line pasting, and standard I/O piping.

I think it's unreasonable to ask people to reimplement all of that when exposing a CreationFlags property is so trivial.

I'll also add that there's precedent in other languages:

@NN---
Copy link
Contributor

NN--- commented Jul 1, 2022

#67642

@NN---
Copy link
Contributor

NN--- commented Jul 1, 2022

Perhaps there should be a constructor from SafeProcessHandle similar to Socket constructor from SafeSockegHandle:
https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.-ctor?view=net-6.0#system-net-sockets-socket-ctor(system-net-sockets-safesockethandle)

@alexrp
Copy link
Contributor Author

alexrp commented Jul 1, 2022

Perhaps there should be a constructor from SafeProcessHandle similar to Socket constructor from SafeSockegHandle

That would not actually address any of the issues outlined in the motivation section above.

@DaZombieKiller
Copy link
Contributor

DaZombieKiller commented Jul 1, 2022

Another problem that arises is that there is no way to go from a process handle to a Process object, which locks you out of any APIs requiring the latter once you dive down to that level. You can attempt to work around this by using the process ID* instead (which you can retrieve a Process for), but then you're still unable to take advantage of features like stream redirection.

* Notably, there is no API to retrieve the process ID from a pseudo-handle allocated by Process.Handle on non-Windows.

@fbrosseau
Copy link

Constructing a Process from a pre-existing handle would be very useful thing on its own, and it would probably be a lot less controversial since there are precedents. I think this would be a good API addition alone. However:

but then you're still unable to take advantage of features like stream redirection.

I do not see how you could possibly achieve that with in any way with an existing handle? If you have a handle, it is too late.

But I definitely see the value in the idea of constructing Process objects from handles - there are cases where GetProcessById simply is impossible - such as when the process in question is higher-privileged. In those cases, then you need to PInvoke absolutely everything, even things that are nicely available in the framework.

--

For the actual proposal in this issue, as others said I think there are extremely few cases where all you would want are the flags, but not the other options, the primary one being tightly controlling handle inheritance. Another one that's almost always required is STARTUPINFO::dwFlags. Creating processes just has too many knobs - and I imagine there are as many various knobs in Linux, in Mac...

@alexrp
Copy link
Contributor Author

alexrp commented Jul 4, 2022

For the actual proposal in this issue, as others said I think there are extremely few cases where all you would want are the flags, but not the other options

I have a bunch of use cases right off the bat:

  • CREATE_NEW_PROCESS_GROUP can be used to achieve more POSIX-like semantics for synthesized console break signals. It's the only way to correctly obtain the process group ID for a process.
  • Needing to suspend a process with CREATE_SUSPENDED as it's created to tightly control its initial execution, allowing binary patching of some fundamental facilities of the process (e.g. malloc/free).
  • DEBUG_ONLY_THIS_PROCESS/DEBUG_PROCESS are immensely useful when doing any kind of diagnostic or binary analysis work.
  • I've had some multiprocessing situations where CREATE_NEW_CONSOLE would have been convenient.

Another one that's almost always required is STARTUPINFO::dwFlags. Creating processes just has too many knobs - and I imagine there are as many various knobs in Linux, in Mac...

I think we can address those knobs as use cases come up. It doesn't seem to me that they would conflict with this proposal.

@DaZombieKiller
Copy link
Contributor

@fbrosseau

I do not see how you could possibly achieve that with in any way with an existing handle? If you have a handle, it is too late.

Yeah. I didn't word it as clearly as I'd hoped, but I was intending to say that being able to go from handle -> Process would be a nice addition, though it wouldn't solve the root issue mentioned by the OP: being able to provide creation flags and still take advantage of the ProcessStartInfo functionality.

@alexrp

Needing to suspend a process with CREATE_SUSPENDED as it's created to tightly control its initial execution, allowing binary patching of some fundamental facilities of the process (e.g. malloc/free).

It should be noted that CREATE_SUSPENDED requires a bit more than just passing the flag, since you need a handle to the main thread too, so you can resume it. This is currently thrown away on process creation.

@alexrp
Copy link
Contributor Author

alexrp commented Jul 4, 2022

It should be noted that CREATE_SUSPENDED requires a bit more than just passing the flag, since you need a handle to the main thread too, so you can resume it. This is currently thrown away on process creation.

There are lots of ways to get at the main thread or simply resume it while ignoring the returned handle from CreateProcessW: tool help APIs, NtQuerySystemInformation (and by extension the Process.Threads property), NtResumeProcess, etc. I'm not too concerned about this aspect.

@fbrosseau
Copy link

fbrosseau commented Jul 4, 2022

[alexrp] I have a bunch of use cases right off the bat:

But that is not my point - and from what I understand, not the point of other posters in this thread.
The point is this: Yes, there are many important use cases for the creation flags - they play a key role. However, it is very rarely enough to customize just that. This API suggestion is not enough, and doing enough would be too much :)

Besides, every listed use case idea in this list would require at least some amount of extra PInvoke after creation.

@alexrp
Copy link
Contributor Author

alexrp commented Jul 4, 2022

The point is this: Yes, there are many important use cases for the creation flags - they play a key role. However, it is very rarely enough to customize just that.

But how do you actually figure that? I've put forth several real use cases where the only thing needed from the System.Diagnostics.Process API is the ability to actually set the flags. Everything else in my use cases can be achieved without any further support from System.Diagnostics.Process.

This API suggestion is not enough, and doing enough would be too much :)

But it is enough for those cases I listed. I am not trying to solve every process creation problem under the sun, just a fairly reasonable subset for Windows. And, judging by the precedent in other languages that I mentioned above, I'm not the only person who thinks it's a reasonable subset.

Besides, every listed use case idea in this list would require at least some amount of extra PInvoke after creation.

All of which are quite trivial, but that's besides the point.

The principal motivation for this API proposal is the maintenance nightmare that would result in the alternative solution: I would be copying almost all of the System.Diagnostics.Process code to every project where I need to set process creation flags, otherwise I'm simply not able to do stream redirection, for example. At least for argument pasting and environment block creation, I could copy only a subset of the code, but for stream redirection, the design of the API is such that I would be copying basically everything. And even if I did that, all of that code would now no longer integrate with the actual System.Diagnostics.Process class. I would have effectively created a fork of the System.Diagnostics.Process.dll assembly at that point.

It's plainly untenable.

@jkotas
Copy link
Member

jkotas commented Jul 4, 2022

judging by the precedent in other languages that I mentioned above, I'm not the only person who thinks it's a reasonable subset.

Python exposes STARTUPINFOEX.lpAttributeList as startupinfo too, only supports one specific attribute currently.

@alexrp
Copy link
Contributor Author

alexrp commented Jul 4, 2022

Python exposes STARTUPINFOEX.lpAttributeList as startupinfo too, only supports one specific attribute currently.

Sure. Just to be clear, I'm not at all against exposing more process creation options. But I see e.g. lpAttributeList as completely independent of what's being proposed here. It can be added in the future in a separate proposal if needed, and it will not conflict with this proposal either. The Process code can just do info.CreationFlags | EXTENDED_STARTUPINFO_PRESENT as needed if/when that day comes. We could even go one step further and throw NotSupportedException today if the user tries to set EXTENDED_STARTUPINFO_PRESENT on CreationFlags.

Since what's being asked for here is very simple and straightforward to implement, and won't conflict with future efforts to add more similar functionality, I'd just like to avoid scope creep so that this has a chance of actually happening.

@jkotas
Copy link
Member

jkotas commented Jul 4, 2022

Since what's being asked for here is very simple and straightforward to implement, and won't conflict with future efforts to add more similar functionality

It is not obvious that it won't conflict. We prefer to review API proposals with full solution to the problem, so that we can see what the full API shape may look like eventually.

We have number of examples where we failed to follow this principle, added an API in one version of the runtime, and added a new API in next version that is duplicate or inconsistent with the one added earlier.

@alexrp
Copy link
Contributor Author

alexrp commented Jul 4, 2022

It is not obvious that it won't conflict.

I'm just not seeing it, I suppose. EXTENDED_STARTUPINFO_PRESENT is the only one that stands out to me as potentially problematic, and as I said, I'm fine with actively blocking that one until we figure out a proper design for it (if/when there's demand).

I also said in the issue description that I'm okay with individual properties for each flag. The 5 that I mentioned here are the ones I would personally really like to see (I could also see arguments for CREATE_BREAKAWAY_FROM_JOB and DETACHED_PROCESS, but I have no use for those myself). Would you be more comfortable with that approach?

We prefer to review API proposals with full solution to the problem, so that we can see what the full API shape may look like eventually.

That is understandable, but at the same time, "full solution to the problem" seems kind of nebulous here. It doesn't seem like anyone actually has an idea what the proposal should cover in full, so even if I were willing to spec out a full API, I wouldn't know what to do.

It also seems to conflict with the notion that API proposals should be driven by real world needs. I've seen many cases where an API proposal gets shot down (and rightfully so) because it's either a solution looking for a problem, or there isn't a compelling enough use case.

@jkotas
Copy link
Member

jkotas commented Jul 4, 2022

"full solution to the problem" seems kind of nebulous here.

This proposal is based on the premise that there are platform-specific details that one needs to pass into the OS API that backs Process.Start, but that it does not make sense to expose these details as proper managed API. These details include dwCreationFlags (Windows), STARTUPINFOEX.lpAttributeList (Windows) and posix_spawnattr set of API on non-Windows. It would be useful see a pattern for how we may deal with all of these.

It also seems to conflict with the notion that API proposals should be driven by real world needs.

It is a long tail, but I believe that you will be able to find some real-world need for any of these platform-specific details. For example, github search shows number of instance of InitializeProcThreadAttributeList used in C#: https://github.com/search?l=C%23&q=InitializeProcThreadAttributeList&type=Code

@alexrp
Copy link
Contributor Author

alexrp commented Jul 5, 2022

This proposal is based on the premise that there are platform-specific details that one needs to pass into the OS API that backs Process.Start, but that it does not make sense to expose these details as proper managed API.

Well, not exactly. Like I said, I'm open to bool properties for the flags that make sense. I just didn't imagine the team would be interested in going that far for relatively niche process options like these.

Due to the nature of the UpdateProcThreadAttribute API, I think it would be a very unwieldy and excessively 'un-.NET' API if not designed as a proper managed API. If there is appetite for specifying a proper managed API for all the extended process creation options, I can do it. However, I would probably only personally commit to implementing the subset of options that I'm interested in. Would this all be acceptable?

These details include dwCreationFlags (Windows), STARTUPINFOEX.lpAttributeList (Windows)

There's also a whole bunch of stuff on STARTUPINFO, e.g. dwFlags and all the fields it controls.

and posix_spawnattr set of API on non-Windows

I think we use fork/exec on Unix platforms, so this wouldn't apply per se. That said, we could probably just emulate what posix_spawnattr provides.

@danielkornev
Copy link

btw does this all mean that launching console apps using Process.Start on Windows silently is no longer an option (for NET6.0 and beyond)?

@alexrp
Copy link
Contributor Author

alexrp commented Jul 17, 2022

btw does this all mean that launching console apps using Process.Start on Windows silently is no longer an option (for NET6.0 and beyond)?

Can you elaborate? It's not obvious to me how that's connected to the proposal here; no functionality would be taken away.

@danielkornev
Copy link

Sure. This might be unrelated as I don't have an understanding of how the Process.Start works internally.

Here's a case: I have a NET6.0 WPF app that needs to launch a console app and keep it hidden. The WPF app runs on Windows and so it should be able to control this behavior in a way like below:
https://stackoverflow.com/questions/836427/how-to-run-a-c-sharp-console-application-with-the-console-hidden

In NETFX this works but I don't see this working in NET6.0. So given that Process.Start in NET6.0 is cross-platform (and given some other conversations like PowerShell/PowerShell#3028) I thought that maybe the ability to run console apps hidden in Process.Start was a Windows-only feature. If that's the case then I suppose one cannot achieve silent/hidden launch of console apps in NET6.0, right?

If these assumptions are wrong then I apologize for the intrusion!

@alexrp
Copy link
Contributor Author

alexrp commented Jul 18, 2022

@danielkornev I have to admit I'm a bit confused at what the concern is still, but, if ProcessStartInfo.WindowStyle works on .NET Framework and doesn't on .NET 6, that sounds like a bug that should be filed separately.

I thought that maybe the ability to run console apps hidden in Process.Start was a Windows-only feature.

It is. WindowStyle is ignored on non-Windows platforms for a variety of reasons - e.g. on Linux, the concept of a "window" is not as well-defined as on Windows and there's no guarantee that libraries related to window manipulation are even present, so it is not obvious what WindowStyle ought to do there.

If that's the case then I suppose one cannot achieve silent/hidden launch of console apps in NET6.0, right?

You should be able to achieve that on Windows just like with .NET Framework.

@alexrp
Copy link
Contributor Author

alexrp commented Jul 18, 2022

By the way, @jkotas ping re: my questions in #71515 (comment).

@jkotas
Copy link
Member

jkotas commented Jul 18, 2022

Due to the nature of the UpdateProcThreadAttribute API, I think it would be a very unwieldy and excessively 'un-.NET' API if not designed as a proper managed API

It argues that the dwCreationFlags should be a proper managed API too (bool properties).

I would probably only personally commit to implementing the subset of options that I'm interested in. Would this all be acceptable?

I think so.

@Jozkee
Copy link
Member

Jozkee commented Jul 19, 2022

The approach of creating a Process object form an existing process handle sounds more powerful to me, allows for more interoperability and avoids OS-exclusive APIs.

@Jozkee Jozkee added this to the Future milestone Jul 19, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jul 19, 2022
@alexrp
Copy link
Contributor Author

alexrp commented Jul 19, 2022

The approach of creating a Process object form an existing process handle sounds more powerful to me, allows for more interoperability and avoids OS-exclusive APIs.

As I mentioned earlier, it does not actually solve the problem that this proposal is trying to solve.

@jkotas
Copy link
Member

jkotas commented Jul 19, 2022

The approach of creating a Process object form an existing process handle sounds more powerful to me, allows for more interoperability and avoids OS-exclusive APIs.

As I mentioned earlier, it does not actually solve the problem that this proposal is trying to solve.

I think it can be an option if it is accompanied by several helper APIs like an API to create the input/output redirection streams to address your concern. Yes, such APIs would be OS-exclusive, but it is likely going to be much smaller public surface than all possible APIs to customize the process creation.

@alexrp
Copy link
Contributor Author

alexrp commented Jul 19, 2022

I think it can be an option if it is accompanied by several helper APIs like an API to create the input/output redirection streams to address your concern.

That's a good point. I can include that as a possible alternative in the proposal.

It would need to also cover argument pasting and environment block creation, and the hypothetical Process.Open(SafeProcessHandle) API would also need overloads that accept redirection streams that were created prior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Diagnostics.Process
Projects
None yet
Development

No branches or pull requests

7 participants