Environment.ProcessPath shouldn't be marked as nullable

[Symbai]

My application is explicit marked as windows only by using -windows. The docs say Returns null when the path is not available. but in which case can this be true? You cannot move/delete/rename the executable when its running (at least under Windows). Also the implementation doesn't return null anywhere:

runtime/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs

Lines 95 to 110 in 0022738

	private static string? GetProcessPath()
	{
	var builder = new ValueStringBuilder(stackalloc char[Interop.Kernel32.MAX_PATH]);
	uint length;
	while ((length = Interop.Kernel32.GetModuleFileName(IntPtr.Zero, ref builder.GetPinnableReference(), (uint)builder.Capacity)) >= builder.Capacity)
	{
	builder.EnsureCapacity((int)length);
	}
	if (length == 0)
	throw Win32Marshal.GetExceptionForLastWin32Error();
	builder.Length = (int)length;
	return builder.ToString();
	}

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[alexrp]

• runtime/src/libraries/System.Private.CoreLib/src/System/Environment.Browser.cs

  Line 34 in 0022738

  private static string? GetProcessPath() => null;

• runtime/src/native/minipal/getexepath.h

  Line 28 in 7414af2

  static inline char* minipal_getexepath(void)

[Symbai]

First it's a windows application. Second if these are the only cases when its null, it should throw a not supported exception instead. Because its obviously not supported (immediately return null).

[alexrp]

First it's a windows application.

NRT annotations are not platform-specific.

Second if these are the only cases when its null, it should throw a not supported exception instead. Because its obviously not supported (immediately return null).

That would be a breaking change.

[Symbai]

That would be a breaking change.

Exactly, was done for ProcessModule.FileName as well. If the only situation when it can be null, is when it's not supported by the platform. Then it should throw instead, that's why we have this exception for.

[alexrp]

Exactly, was done for ProcessModule.FileName as well. If the only situation when it can be null, is when it's not supported by the platform. Then it should throw instead, that's why we have this exception for.

These situations aren't the same. ProcessModule.FileName cannot be null on any platform, so marking it as such is appropriate. Environment.ProcessPath has already shipped with the behavior of returning null on some platforms in certain scenarios. Changing it to throw PlatformNotSupportedException in those cases would be breaking. It's a bit unfortunate, but the ship has sailed on this.

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

Issue Details

---

My application is explicit marked as windows only by using -windows. The docs say Returns null when the path is not available. but in which case can this be true? You cannot move/delete/rename the executable when its running (at least under Windows). Also the implementation doesn't return null anywhere:

runtime/src/libraries/System.Private.CoreLib/src/System/Environment.Windows.cs

Lines 95 to 110 in 0022738

	private static string? GetProcessPath()
	{
	var builder = new ValueStringBuilder(stackalloc char[Interop.Kernel32.MAX_PATH]);
	uint length;
	while ((length = Interop.Kernel32.GetModuleFileName(IntPtr.Zero, ref builder.GetPinnableReference(), (uint)builder.Capacity)) >= builder.Capacity)
	{
	builder.EnsureCapacity((int)length);
	}
	if (length == 0)
	throw Win32Marshal.GetExceptionForLastWin32Error();
	builder.Length = (int)length;
	return builder.ToString();
	}

Author:	Symbai
Assignees:	-
Labels:	area-System.Runtime, untriaged
Milestone:	-

[jkotas]

Changing it to throw PlatformNotSupportedException in those cases would be breaking.

+1

Also, the advantage of returning null instead of throwing PlatformNotSupportedException is that it allows cheap runtime check and fallback for the unsupported case without hardcoding the list of unsupported platforms.

[Symbai]

The disadvantage is that it creates unnecessary warnings for all developers which doesn't write a browser application, because it CANNOT be null for them (which also the docs doesn't say). Some people might even add an unnecessary null check.