Avoid encoding unchanged process environments on Unix

src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs

@@ -13,7 +13,7 @@ internal static partial class Interop
     internal static partial class Sys
     {
         internal static unsafe int ForkAndExecProcess(
-            string filename, string[] argv, IDictionary<string, string?> env, string? cwd,
+            string filename, string[] argv, IDictionary<string, string?>? env, string? cwd,
             bool setUser, uint userId, uint groupId, uint[]? groups,
             out int lpChildPid, SafeFileHandle? stdinFd, SafeFileHandle? stdoutFd, SafeFileHandle? stderrFd,
             ProcessStartInfo startInfo, SafeHandle[]? inheritedHandles = null)
@@ -67,7 +67,10 @@ internal static unsafe int ForkAndExecProcess(
                 }
 
                 AllocArgvArray(argv, ref argvPtr);
-                AllocEnvpArray(env, ref envpPtr);
+                if (env is not null)
+                {
+                    AllocEnvpArray(env, ref envpPtr);
+                }
                 fixed (uint* pGroups = groups)
                 fixed (int* pInheritedFds = inheritedFds)
                 {

src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs

@@ -7,6 +7,7 @@
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
+using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Runtime.InteropServices.Marshalling;
 using System.Runtime.Versioning;
@@ -218,7 +219,7 @@ internal static SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFile
             string filename;
             string[] argv;
 
-            IDictionary<string, string?> env = startInfo.Environment;
+            IDictionary<string, string?>? env = GetEnvironmentVariables(startInfo);
             string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null;
 
             bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName);
@@ -356,7 +357,7 @@ internal static unsafe SafeProcessHandle StartWithCallback(ProcessStartInfo star
         private static SafeProcessHandle StartWithShellExecute(ProcessStartInfo startInfo, SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle,
             SafeFileHandle? stderrHandle, out ProcessWaitState.Holder? waitStateHolder)
         {
-            IDictionary<string, string?> env = startInfo.Environment;
+            IDictionary<string, string?>? env = GetEnvironmentVariables(startInfo);
             string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null;
 
             bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName);
@@ -425,9 +426,15 @@ private static SafeProcessHandle StartWithShellExecute(ProcessStartInfo startInf
         private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle)
             => ProcessUtils.IsTerminal(stdinHandle) || ProcessUtils.IsTerminal(stdoutHandle) || ProcessUtils.IsTerminal(stderrHandle);
 
+        private static IDictionary<string, string?>? GetEnvironmentVariables(ProcessStartInfo startInfo)
+            => GetHasEnvironmentVariablesBeenModified(null) ? startInfo.Environment : startInfo._environmentVariables;
+
+        [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "get_HasEnvironmentVariablesBeenModified")]
+        private static extern bool GetHasEnvironmentVariablesBeenModified([UnsafeAccessorType("System.Environment, System.Private.CoreLib")] object? _);
+
         private static SafeProcessHandle ForkAndExecProcess(
             ProcessStartInfo startInfo, string? resolvedFilename, string[] argv,
-            IDictionary<string, string?> env, string? cwd, bool setCredentials, uint userId,
+            IDictionary<string, string?>? env, string? cwd, bool setCredentials, uint userId,
             uint groupId, uint[]? groups,
             SafeFileHandle? stdinHandle, SafeFileHandle? stdoutHandle, SafeFileHandle? stderrHandle,
             bool usesTerminal, SafeHandle[]? inheritedHandles, out ProcessWaitState.Holder? waitStateHolder, bool throwOnNoExec = true)

src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs

@@ -280,6 +280,46 @@ public void TestEnvironmentOfChildProcess()
             }
         }
 
+        [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
+        [InlineData("not_null", true)]
+        [InlineData("not_null", false)]
+        [InlineData("null", false)]
+        [InlineData("null", true)]
+        public void SeesEnvironmentVariableSetBeforeStart(string value, bool modifyParent)
+        {
+            const string Name = "TestEnvironmentOfChildProcess_ParentSetBeforeStart";
+
+            // We run a dedicated process to ensure that each test has its own copy of the static shared state.
+            using Process isolation = CreateProcess((expectedValue, modifyParentStr) =>
+            {
+                using Process process = CreateProcess(static (value) =>
+                {
+                    Assert.Equal(value == "null" ? null : value, Environment.GetEnvironmentVariable(Name));
+
+                    return RemoteExecutor.SuccessExitCode;
+                }, expectedValue);
+
+                if (bool.Parse(modifyParentStr))
+                {
+                    Environment.SetEnvironmentVariable(Name, expectedValue == "null" ? null : expectedValue);
+                }
+                else
+                {
+                    process.StartInfo.Environment[Name] = expectedValue == "null" ? null : expectedValue;
+                }
+
+                process.Start();
+                Assert.True(process.WaitForExit(WaitInMS));
+                Assert.Equal(RemoteExecutor.SuccessExitCode, process.ExitCode);
+
+                return RemoteExecutor.SuccessExitCode;
+            }, value, modifyParent.ToString());
+
+            isolation.Start();
+            Assert.True(isolation.WaitForExit(WaitInMS));
+            Assert.Equal(RemoteExecutor.SuccessExitCode, isolation.ExitCode);
+        }
+
         [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
         public void EnvironmentNullValue()
         {

src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml

@@ -8,6 +8,10 @@
       <!-- Used by VS4Mac via reflection to symbolize stack traces -->
       <method name="GetMethodFromNativeIP" />
     </type>
+    <type fullname="System.Environment">
+      <!-- Used by System.Diagnostics.Process via UnsafeAccessor -->
+      <method name="get_HasEnvironmentVariablesBeenModified" />
+    </type>
     <type fullname="System.Reflection.Emit.AssemblyBuilder">
       <!-- Used by System.Linq.Expressions via reflection -->
       <method name="ForceAllowDynamicCode" />

src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs

@@ -12,6 +12,12 @@ namespace System
 {
     public static partial class Environment
     {
+        private static volatile bool s_hasEnvironmentVariablesBeenModified;
+
+        internal static bool HasEnvironmentVariablesBeenModified => s_hasEnvironmentVariablesBeenModified;
+
+        static partial void SetHasEnvironmentVariablesBeenModified() => s_hasEnvironmentVariablesBeenModified = true;
+
         public static string[] GetLogicalDrives() => Interop.Sys.GetAllMountPoints();
 
         public static string MachineName

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

@@ -89,9 +89,12 @@ public static IDictionary GetEnvironmentVariables(EnvironmentVariableTarget targ
             return GetEnvironmentVariablesFromRegistry(fromMachine);
         }
 
+        static partial void SetHasEnvironmentVariablesBeenModified();
+
         public static void SetEnvironmentVariable(string variable, string? value)
         {
             ValidateVariable(variable);
+            SetHasEnvironmentVariablesBeenModified();
             SetEnvironmentVariableCore(variable, value);
         }
 

src/native/libs/System.Native/pal_process.c

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 #include "pal_config.h"
+#include "pal_environment.h"
 #include "pal_process.h"
 #include "pal_signal.h"
 #include "pal_io.h"
@@ -565,7 +566,7 @@ static int32_t ForkAndExecProcessInternal(
     int32_t* inheritedFds, int32_t inheritedFdCount, int32_t startDetached, int32_t applyPDeathSig, int32_t startSuspended)
 {
 #if HAVE_FORK || defined(TARGET_OSX) || defined(TARGET_MACCATALYST)
-    assert(NULL != filename && NULL != argv && NULL != envp && NULL != childPid &&
+    assert(NULL != filename && NULL != argv && NULL != childPid &&
             (groupsLength == 0 || groups != NULL) && "null argument.");
 
 #if !HAVE_PR_SET_PDEATHSIG
@@ -712,7 +713,7 @@ static int32_t ForkAndExecProcessInternal(
         }
 
         // Spawn the process
-        result = posix_spawn(&spawnedPid, filename, &file_actions, &attr, argv, envp);
+        result = posix_spawn(&spawnedPid, filename, &file_actions, &attr, argv, envp != NULL ? envp : SystemNative_GetEnviron());
 
         posix_spawn_file_actions_destroy(&file_actions);
         posix_spawnattr_destroy(&attr);
@@ -934,7 +935,7 @@ static int32_t ForkAndExecProcessInternal(
 #endif
 
         // Finally, execute the new process.  execve will not return if it's successful.
-        execve(filename, argv, envp);
+        execve(filename, argv, envp != NULL ? envp : SystemNative_GetEnviron());
         ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); // execve failed
     }
 

src/native/libs/System.Native/pal_process.h

@@ -21,7 +21,7 @@
 PALEXPORT int32_t SystemNative_ForkAndExecProcess(
                    const char* filename,   // filename argument to execve
                    char* const argv[],     // argv argument to execve
-                   char* const envp[],     // envp argument to execve
+                   char* const envp[],     // envp argument to execve; NULL to inherit the current environment
                    const char* cwd,        // path passed to chdir in child process
                    int32_t setCredentials, // whether to set the userId and groupId for the child process
                    uint32_t userId,        // the user id under which the child process should run