From 3cf4baa1a45d9aeb9fed716cc7f7fd6dfc6ce6e2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:36:35 +0000 Subject: [PATCH 01/17] Avoid encoding untouched Unix process environments Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Interop.ForkAndExecProcess.cs | 7 +++++-- .../SafeHandles/SafeProcessHandle.Unix.cs | 6 +++--- .../tests/ProcessTests.Unix.cs | 21 +++++++++++++++++++ src/native/libs/System.Native/pal_process.c | 16 +++++++++++--- src/native/libs/System.Native/pal_process.h | 2 +- 5 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index f7e1a68da1631b..ddc70b6284c4b4 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/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 env, string? cwd, + string filename, string[] argv, IDictionary? 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) { diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index e191c6c067a443..fce68cb7691d2a 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -218,7 +218,7 @@ internal static SafeProcessHandle StartCore(ProcessStartInfo startInfo, SafeFile string filename; string[] argv; - IDictionary env = startInfo.Environment; + IDictionary? env = startInfo._environmentVariables; string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName); @@ -356,7 +356,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 env = startInfo.Environment; + IDictionary? env = startInfo._environmentVariables; string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName); @@ -427,7 +427,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, - IDictionary env, string? cwd, bool setCredentials, uint userId, + IDictionary? 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) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index cf485f5dc8e887..e684d1cc4f7b58 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -36,6 +36,27 @@ private void TestWindowApisUnix() } } + [Fact] + public void Start_EnvironmentNotAccessed_InheritsEnvironmentWithoutInitializingDictionary() + { + string? path = Environment.GetEnvironmentVariable("PATH"); + Assert.NotNull(path); + + ProcessStartInfo startInfo = new("/bin/sh") + { + ArgumentList = { "-c", "printf %s \"$PATH\"" }, + RedirectStandardOutput = true, + }; + + using Process process = Process.Start(startInfo); + Assert.Equal(path, process.StandardOutput.ReadToEnd()); + Assert.True(process.WaitForExit(WaitInMS)); + Assert.Equal(0, process.ExitCode); + Assert.Null(typeof(ProcessStartInfo) + .GetField("_environmentVariables", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(startInfo)); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void MainWindowHandle_GetUnix_ReturnsDefaultValue() { diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index c5af90e397287e..27c27af47021a2 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -336,6 +336,16 @@ static int32_t ForkAndExecProcessInternal( int32_t* childPid, int32_t stdinFd, int32_t stdoutFd, int32_t stderrFd, int32_t* inheritedFds, int32_t inheritedFdCount, int32_t startDetached, int32_t applyPDeathSig, int32_t startSuspended); +static char** GetEnviron(void) +{ +#if HAVE_NSGETENVIRON + return *(_NSGetEnviron()); +#else + extern char** environ; + return environ; +#endif +} + #if HAVE_PR_SET_PDEATHSIG // Dedicated thread infrastructure for PR_SET_PDEATHSIG. // @@ -565,7 +575,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 +722,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 : GetEnviron()); posix_spawn_file_actions_destroy(&file_actions); posix_spawnattr_destroy(&attr); @@ -934,7 +944,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 : GetEnviron()); ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); // execve failed } diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 515cfa5ea8e090..61ef54c65001ca 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/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 From 3a19eca850adce70f5eff54265ae756a9e59aa97 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Jul 2026 14:55:42 +0000 Subject: [PATCH 02/17] Preserve managed environment changes for process start Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 19 +++++++++-- .../tests/ProcessTests.Unix.cs | 33 ++++++++++--------- .../src/System/Environment.cs | 6 ++++ 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index fce68cb7691d2a..7b6db0adb44712 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/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? env = startInfo._environmentVariables; + IDictionary? 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? env = startInfo._environmentVariables; + IDictionary? env = GetEnvironmentVariables(startInfo); string? cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName); @@ -425,6 +426,20 @@ 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? GetEnvironmentVariables(ProcessStartInfo startInfo) + { + if (startInfo._environmentVariables is null && Volatile.Read(ref GetEnvironmentVariablesChanged(null))) + { + return startInfo.Environment; + } + + return startInfo._environmentVariables; + } + + [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name = "HasChanged")] + private static extern ref bool GetEnvironmentVariablesChanged( + [UnsafeAccessorType("System.Environment+ProcessEnvironmentState, System.Private.CoreLib")] object? _); + private static SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, IDictionary? env, string? cwd, bool setCredentials, uint userId, diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index e684d1cc4f7b58..6db407026e3fe1 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -36,25 +36,28 @@ private void TestWindowApisUnix() } } - [Fact] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void Start_EnvironmentNotAccessed_InheritsEnvironmentWithoutInitializingDictionary() { - string? path = Environment.GetEnvironmentVariable("PATH"); - Assert.NotNull(path); - - ProcessStartInfo startInfo = new("/bin/sh") + RemoteExecutor.Invoke(() => { - ArgumentList = { "-c", "printf %s \"$PATH\"" }, - RedirectStandardOutput = true, - }; + string? path = Environment.GetEnvironmentVariable("PATH"); + Assert.NotNull(path); - using Process process = Process.Start(startInfo); - Assert.Equal(path, process.StandardOutput.ReadToEnd()); - Assert.True(process.WaitForExit(WaitInMS)); - Assert.Equal(0, process.ExitCode); - Assert.Null(typeof(ProcessStartInfo) - .GetField("_environmentVariables", BindingFlags.NonPublic | BindingFlags.Instance)! - .GetValue(startInfo)); + ProcessStartInfo startInfo = new("/bin/sh") + { + ArgumentList = { "-c", "printf %s \"$PATH\"" }, + RedirectStandardOutput = true, + }; + + using Process process = Process.Start(startInfo); + Assert.Equal(path, process.StandardOutput.ReadToEnd()); + Assert.True(process.WaitForExit(WaitInMS)); + Assert.Equal(0, process.ExitCode); + Assert.Null(typeof(ProcessStartInfo) + .GetField("_environmentVariables", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(startInfo)); + }).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index dabc44cab4c4bf..a93927d6a3bcb4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.cs @@ -93,6 +93,7 @@ public static void SetEnvironmentVariable(string variable, string? value) { ValidateVariable(variable); SetEnvironmentVariableCore(variable, value); + Volatile.Write(ref ProcessEnvironmentState.HasChanged, true); } public static void SetEnvironmentVariable(string variable, string? value, EnvironmentVariableTarget target) @@ -109,6 +110,11 @@ public static void SetEnvironmentVariable(string variable, string? value, Enviro SetEnvironmentVariableFromRegistry(variable, value, fromMachine: fromMachine); } + private sealed class ProcessEnvironmentState + { + internal static bool HasChanged; + } + #if !MONO internal static string[]? s_commandLineArgs; From fa1d43bb8dee673cb3720db035551b444e984afd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:07:33 +0000 Subject: [PATCH 03/17] Address Unix process environment feedback Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- src/coreclr/pal/src/misc/environ.cpp | 5 +++ .../Unix/System.Native/Interop.SetEnv.cs | 17 ++++++++++ .../SafeHandles/SafeProcessHandle.Unix.cs | 7 +--- .../tests/ProcessTests.Unix.cs | 33 +++++++++---------- .../System.Private.CoreLib.Shared.projitems | 3 ++ .../src/System/Environment.Variables.Unix.cs | 2 ++ .../src/System/Environment.cs | 6 ---- src/native/libs/System.Native/entrypoints.c | 2 ++ .../libs/System.Native/pal_environment.c | 10 ++++++ .../libs/System.Native/pal_environment.h | 4 +++ .../libs/System.Native/pal_environment.m | 10 ++++++ src/native/libs/System.Native/pal_process.c | 2 +- 12 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs diff --git a/src/coreclr/pal/src/misc/environ.cpp b/src/coreclr/pal/src/misc/environ.cpp index 4a9eb640e2e64a..4f91af9c00c9cf 100644 --- a/src/coreclr/pal/src/misc/environ.cpp +++ b/src/coreclr/pal/src/misc/environ.cpp @@ -552,6 +552,7 @@ SetEnvironmentVariableA( } EnvironUnsetenv(lpName); + (void)unsetenv(lpName); } else { @@ -568,6 +569,10 @@ SetEnvironmentVariableA( sprintf_s(string, iLen, "%s=%s", lpName, lpValue); nResult = EnvironPutenv(string, FALSE) ? 0 : -1; + if (nResult == 0) + { + (void)setenv(lpName, lpValue, 1); + } free(string); string = nullptr; diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs new file mode 100644 index 00000000000000..4f016ac9ad831b --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Runtime.InteropServices; + +internal static partial class Interop +{ + internal static partial class Sys + { + [LibraryImport(Interop.Libraries.SystemNative, StringMarshalling = StringMarshalling.Utf8, EntryPoint = "SystemNative_SetEnv")] + internal static partial int SetEnv(string name, string value); + + [LibraryImport(Interop.Libraries.SystemNative, StringMarshalling = StringMarshalling.Utf8, EntryPoint = "SystemNative_UnsetEnv")] + internal static partial int UnsetEnv(string name); + } +} diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 7b6db0adb44712..119f44d0a9d057 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -7,7 +7,6 @@ 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; @@ -428,7 +427,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) { - if (startInfo._environmentVariables is null && Volatile.Read(ref GetEnvironmentVariablesChanged(null))) + if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst()) { return startInfo.Environment; } @@ -436,10 +435,6 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st return startInfo._environmentVariables; } - [UnsafeAccessor(UnsafeAccessorKind.StaticField, Name = "HasChanged")] - private static extern ref bool GetEnvironmentVariablesChanged( - [UnsafeAccessorType("System.Environment+ProcessEnvironmentState, System.Private.CoreLib")] object? _); - private static SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, IDictionary? env, string? cwd, bool setCredentials, uint userId, diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index 6db407026e3fe1..e684d1cc4f7b58 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -36,28 +36,25 @@ private void TestWindowApisUnix() } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [Fact] public void Start_EnvironmentNotAccessed_InheritsEnvironmentWithoutInitializingDictionary() { - RemoteExecutor.Invoke(() => + string? path = Environment.GetEnvironmentVariable("PATH"); + Assert.NotNull(path); + + ProcessStartInfo startInfo = new("/bin/sh") { - string? path = Environment.GetEnvironmentVariable("PATH"); - Assert.NotNull(path); + ArgumentList = { "-c", "printf %s \"$PATH\"" }, + RedirectStandardOutput = true, + }; - ProcessStartInfo startInfo = new("/bin/sh") - { - ArgumentList = { "-c", "printf %s \"$PATH\"" }, - RedirectStandardOutput = true, - }; - - using Process process = Process.Start(startInfo); - Assert.Equal(path, process.StandardOutput.ReadToEnd()); - Assert.True(process.WaitForExit(WaitInMS)); - Assert.Equal(0, process.ExitCode); - Assert.Null(typeof(ProcessStartInfo) - .GetField("_environmentVariables", BindingFlags.NonPublic | BindingFlags.Instance)! - .GetValue(startInfo)); - }).Dispose(); + using Process process = Process.Start(startInfo); + Assert.Equal(path, process.StandardOutput.ReadToEnd()); + Assert.True(process.WaitForExit(WaitInMS)); + Assert.Equal(0, process.ExitCode); + Assert.Null(typeof(ProcessStartInfo) + .GetField("_environmentVariables", BindingFlags.NonPublic | BindingFlags.Instance)! + .GetValue(startInfo)); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 9670007e075ff4..2ccd12961ab297 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2500,6 +2500,9 @@ Common\Interop\Unix\System.Native\Interop.GetEnviron.cs + + Common\Interop\Unix\System.Native\Interop.SetEnv.cs + Common\Interop\Unix\System.Native\Interop.GetHostName.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs index 1f9f9b0cdd2878..3d1fffde27dc2d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs @@ -49,10 +49,12 @@ private static unsafe void SetEnvironmentVariableCore(string variable, string? v { if (value == null) { + Interop.Sys.UnsetEnv(variable); s_environment.Remove(variable); } else { + Interop.Sys.SetEnv(variable, value); s_environment[variable] = value; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index a93927d6a3bcb4..dabc44cab4c4bf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.cs @@ -93,7 +93,6 @@ public static void SetEnvironmentVariable(string variable, string? value) { ValidateVariable(variable); SetEnvironmentVariableCore(variable, value); - Volatile.Write(ref ProcessEnvironmentState.HasChanged, true); } public static void SetEnvironmentVariable(string variable, string? value, EnvironmentVariableTarget target) @@ -110,11 +109,6 @@ public static void SetEnvironmentVariable(string variable, string? value, Enviro SetEnvironmentVariableFromRegistry(variable, value, fromMachine: fromMachine); } - private sealed class ProcessEnvironmentState - { - internal static bool HasChanged; - } - #if !MONO internal static string[]? s_commandLineArgs; diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index 78e08565015511..dfa932405c8bc9 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -297,6 +297,8 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetPlatformSIGSTOP) DllImportEntry(SystemNative_GetGroups) DllImportEntry(SystemNative_GetEnv) + DllImportEntry(SystemNative_SetEnv) + DllImportEntry(SystemNative_UnsetEnv) DllImportEntry(SystemNative_GetEnviron) DllImportEntry(SystemNative_FreeEnviron) DllImportEntry(SystemNative_GetGroupName) diff --git a/src/native/libs/System.Native/pal_environment.c b/src/native/libs/System.Native/pal_environment.c index b61c8fab6c8751..980dd51e2266e5 100644 --- a/src/native/libs/System.Native/pal_environment.c +++ b/src/native/libs/System.Native/pal_environment.c @@ -15,6 +15,16 @@ char* SystemNative_GetEnv(const char* variable) return getenv(variable); } +int32_t SystemNative_SetEnv(const char* variable, const char* value) +{ + return setenv(variable, value, 1); +} + +int32_t SystemNative_UnsetEnv(const char* variable) +{ + return unsetenv(variable); +} + char** SystemNative_GetEnviron(void) { #if HAVE_NSGETENVIRON diff --git a/src/native/libs/System.Native/pal_environment.h b/src/native/libs/System.Native/pal_environment.h index dee7a10f3aecdc..c9cb1754bbfe79 100644 --- a/src/native/libs/System.Native/pal_environment.h +++ b/src/native/libs/System.Native/pal_environment.h @@ -8,6 +8,10 @@ PALEXPORT char* SystemNative_GetEnv(const char* variable); +PALEXPORT int32_t SystemNative_SetEnv(const char* variable, const char* value); + +PALEXPORT int32_t SystemNative_UnsetEnv(const char* variable); + PALEXPORT char** SystemNative_GetEnviron(void); PALEXPORT void SystemNative_FreeEnviron(char** environ); diff --git a/src/native/libs/System.Native/pal_environment.m b/src/native/libs/System.Native/pal_environment.m index 7705a63da129ac..e938efbec10698 100644 --- a/src/native/libs/System.Native/pal_environment.m +++ b/src/native/libs/System.Native/pal_environment.m @@ -18,6 +18,16 @@ return getenv(variable); } +int32_t SystemNative_SetEnv(const char* variable, const char* value) +{ + return setenv(variable, value, 1); +} + +int32_t SystemNative_UnsetEnv(const char* variable) +{ + return unsetenv(variable); +} + static char *empty_key_value_pair = "="; static void get_environ_helper(const void *key, const void *value, void *context) diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 27c27af47021a2..ef7c858d74498c 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -722,7 +722,7 @@ static int32_t ForkAndExecProcessInternal( } // Spawn the process - result = posix_spawn(&spawnedPid, filename, &file_actions, &attr, argv, envp != NULL ? envp : GetEnviron()); + result = posix_spawn(&spawnedPid, filename, &file_actions, &attr, argv, envp); posix_spawn_file_actions_destroy(&file_actions); posix_spawnattr_destroy(&attr); From 73ed42b76219d60dc9871d994ca8851d8de55833 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Jul 2026 16:55:19 +0000 Subject: [PATCH 04/17] Remove implementation-detail process environment test Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../tests/ProcessTests.Unix.cs | 21 ------------------- 1 file changed, 21 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index e684d1cc4f7b58..cf485f5dc8e887 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -36,27 +36,6 @@ private void TestWindowApisUnix() } } - [Fact] - public void Start_EnvironmentNotAccessed_InheritsEnvironmentWithoutInitializingDictionary() - { - string? path = Environment.GetEnvironmentVariable("PATH"); - Assert.NotNull(path); - - ProcessStartInfo startInfo = new("/bin/sh") - { - ArgumentList = { "-c", "printf %s \"$PATH\"" }, - RedirectStandardOutput = true, - }; - - using Process process = Process.Start(startInfo); - Assert.Equal(path, process.StandardOutput.ReadToEnd()); - Assert.True(process.WaitForExit(WaitInMS)); - Assert.Equal(0, process.ExitCode); - Assert.Null(typeof(ProcessStartInfo) - .GetField("_environmentVariables", BindingFlags.NonPublic | BindingFlags.Instance)! - .GetValue(startInfo)); - } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void MainWindowHandle_GetUnix_ReturnsDefaultValue() { From 9527830538ab9d28d92b918b696d2fe9f3aadf68 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 13:05:26 +0000 Subject: [PATCH 05/17] Apply remaining changes Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- src/coreclr/pal/src/misc/environ.cpp | 5 ----- .../Unix/System.Native/Interop.SetEnv.cs | 17 ----------------- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 9 ++++++++- .../ref/System.Private.CoreLib.ExtraApis.cs | 7 +++++++ .../ref/System.Private.CoreLib.ExtraApis.txt | 1 + .../src/System.Private.CoreLib.Shared.projitems | 3 --- .../src/System/Environment.Variables.Unix.cs | 2 -- .../src/System/Environment.cs | 8 ++++++++ .../src/CompatibilitySuppressions.xml | 4 ++++ src/native/libs/System.Native/entrypoints.c | 2 -- src/native/libs/System.Native/pal_environment.c | 10 ---------- src/native/libs/System.Native/pal_environment.h | 4 ---- src/native/libs/System.Native/pal_environment.m | 10 ---------- 13 files changed, 28 insertions(+), 54 deletions(-) delete mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs diff --git a/src/coreclr/pal/src/misc/environ.cpp b/src/coreclr/pal/src/misc/environ.cpp index 4f91af9c00c9cf..4a9eb640e2e64a 100644 --- a/src/coreclr/pal/src/misc/environ.cpp +++ b/src/coreclr/pal/src/misc/environ.cpp @@ -552,7 +552,6 @@ SetEnvironmentVariableA( } EnvironUnsetenv(lpName); - (void)unsetenv(lpName); } else { @@ -569,10 +568,6 @@ SetEnvironmentVariableA( sprintf_s(string, iLen, "%s=%s", lpName, lpValue); nResult = EnvironPutenv(string, FALSE) ? 0 : -1; - if (nResult == 0) - { - (void)setenv(lpName, lpValue, 1); - } free(string); string = nullptr; diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs deleted file mode 100644 index 4f016ac9ad831b..00000000000000 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.SetEnv.cs +++ /dev/null @@ -1,17 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Runtime.InteropServices; - -internal static partial class Interop -{ - internal static partial class Sys - { - [LibraryImport(Interop.Libraries.SystemNative, StringMarshalling = StringMarshalling.Utf8, EntryPoint = "SystemNative_SetEnv")] - internal static partial int SetEnv(string name, string value); - - [LibraryImport(Interop.Libraries.SystemNative, StringMarshalling = StringMarshalling.Utf8, EntryPoint = "SystemNative_UnsetEnv")] - internal static partial int UnsetEnv(string name); - } -} diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 119f44d0a9d057..0fb49f888ac9cd 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/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; @@ -427,7 +428,9 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) { - if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst()) + if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || + OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || + GetEnvironmentHasBeenModified(null)) { return startInfo.Environment; } @@ -435,6 +438,10 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st return startInfo._environmentVariables; } + [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "get_HasBeenModified")] + private static extern bool GetEnvironmentHasBeenModified( + [UnsafeAccessorType("System.Environment, System.Private.CoreLib")] object? _); + private static SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, IDictionary? env, string? cwd, bool setCredentials, uint userId, diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs index 7fbc3da42edee1..a862feea219360 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs @@ -4,6 +4,13 @@ // NOTE: Types/members which are not publicly exposed in System.Runtime.dll but still used internally by libraries. // Manually maintained, keep in sync with System.Private.CoreLib.ExtraApis.txt +namespace System +{ + public static partial class Environment + { + public static bool HasBeenModified { get { throw null; } } + } +} namespace System.Runtime.Serialization { public readonly partial struct DeserializationToken : System.IDisposable diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt index 477f41ba86acf7..b90150e85d2dc4 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt @@ -1,6 +1,7 @@ # NOTE: Types/members which are not publicly exposed in System.Runtime.dll but still used internally by libraries. # Manually maintained, keep in sync with System.Private.CoreLib.cs +P:System.Environment.HasBeenModified T:System.Runtime.Serialization.DeserializationToken M:System.Runtime.Serialization.SerializationInfo.StartDeserialization T:System.Diagnostics.DebugProvider diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 2ccd12961ab297..9670007e075ff4 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -2500,9 +2500,6 @@ Common\Interop\Unix\System.Native\Interop.GetEnviron.cs - - Common\Interop\Unix\System.Native\Interop.SetEnv.cs - Common\Interop\Unix\System.Native\Interop.GetHostName.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs index 3d1fffde27dc2d..1f9f9b0cdd2878 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs @@ -49,12 +49,10 @@ private static unsafe void SetEnvironmentVariableCore(string variable, string? v { if (value == null) { - Interop.Sys.UnsetEnv(variable); s_environment.Remove(variable); } else { - Interop.Sys.SetEnv(variable, value); s_environment[variable] = value; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index dabc44cab4c4bf..aa2ec8588c46e6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.cs @@ -42,6 +42,7 @@ public readonly struct ProcessCpuUsage public static int ProcessorCount { get; } = RuntimeFeature.IsMultithreadingSupported ? GetProcessorCount() : 1; private static NullableBool s_privilegedProcess; + private static bool s_hasBeenModified; /// /// Gets whether the current process is authorized to perform security-relevant functions. @@ -62,6 +63,12 @@ public static bool IsPrivilegedProcess // Unconditionally return false since .NET Core does not support object finalization during shutdown. public static bool HasShutdownStarted => false; + public static bool HasBeenModified + { + get => Volatile.Read(ref s_hasBeenModified); + private set => Volatile.Write(ref s_hasBeenModified, value); + } + public static string? GetEnvironmentVariable(string variable) { ArgumentNullException.ThrowIfNull(variable); @@ -92,6 +99,7 @@ public static IDictionary GetEnvironmentVariables(EnvironmentVariableTarget targ public static void SetEnvironmentVariable(string variable, string? value) { ValidateVariable(variable); + HasBeenModified = true; SetEnvironmentVariableCore(variable, value); } diff --git a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml index 8d6cd745858ae4..62ae059b30475f 100644 --- a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml +++ b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml @@ -13,4 +13,8 @@ CP0002 M:System.Runtime.Serialization.SerializationInfo.StartDeserialization + + CP0002 + M:System.Environment.get_HasBeenModified + diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index dfa932405c8bc9..78e08565015511 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -297,8 +297,6 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetPlatformSIGSTOP) DllImportEntry(SystemNative_GetGroups) DllImportEntry(SystemNative_GetEnv) - DllImportEntry(SystemNative_SetEnv) - DllImportEntry(SystemNative_UnsetEnv) DllImportEntry(SystemNative_GetEnviron) DllImportEntry(SystemNative_FreeEnviron) DllImportEntry(SystemNative_GetGroupName) diff --git a/src/native/libs/System.Native/pal_environment.c b/src/native/libs/System.Native/pal_environment.c index 980dd51e2266e5..b61c8fab6c8751 100644 --- a/src/native/libs/System.Native/pal_environment.c +++ b/src/native/libs/System.Native/pal_environment.c @@ -15,16 +15,6 @@ char* SystemNative_GetEnv(const char* variable) return getenv(variable); } -int32_t SystemNative_SetEnv(const char* variable, const char* value) -{ - return setenv(variable, value, 1); -} - -int32_t SystemNative_UnsetEnv(const char* variable) -{ - return unsetenv(variable); -} - char** SystemNative_GetEnviron(void) { #if HAVE_NSGETENVIRON diff --git a/src/native/libs/System.Native/pal_environment.h b/src/native/libs/System.Native/pal_environment.h index c9cb1754bbfe79..dee7a10f3aecdc 100644 --- a/src/native/libs/System.Native/pal_environment.h +++ b/src/native/libs/System.Native/pal_environment.h @@ -8,10 +8,6 @@ PALEXPORT char* SystemNative_GetEnv(const char* variable); -PALEXPORT int32_t SystemNative_SetEnv(const char* variable, const char* value); - -PALEXPORT int32_t SystemNative_UnsetEnv(const char* variable); - PALEXPORT char** SystemNative_GetEnviron(void); PALEXPORT void SystemNative_FreeEnviron(char** environ); diff --git a/src/native/libs/System.Native/pal_environment.m b/src/native/libs/System.Native/pal_environment.m index e938efbec10698..7705a63da129ac 100644 --- a/src/native/libs/System.Native/pal_environment.m +++ b/src/native/libs/System.Native/pal_environment.m @@ -18,16 +18,6 @@ return getenv(variable); } -int32_t SystemNative_SetEnv(const char* variable, const char* value) -{ - return setenv(variable, value, 1); -} - -int32_t SystemNative_UnsetEnv(const char* variable) -{ - return unsetenv(variable); -} - static char *empty_key_value_pair = "="; static void get_environ_helper(const void *key, const void *value, void *context) From 4fd7339f5dc520fadc29504a2f8a772d8a0b5b0f Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 16 Jul 2026 15:18:34 +0200 Subject: [PATCH 06/17] address my own feedback --- .../Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs | 8 +------- .../src/System.Diagnostics.Process.csproj | 1 + .../System.Private.CoreLib/src/System/Environment.cs | 7 +------ 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 0fb49f888ac9cd..85752abaf94d51 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -428,9 +428,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) { - if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || - OperatingSystem.IsIOS() || OperatingSystem.IsTvOS() || - GetEnvironmentHasBeenModified(null)) + if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || Environment.HasBeenModified) { return startInfo.Environment; } @@ -438,10 +436,6 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st return startInfo._environmentVariables; } - [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "get_HasBeenModified")] - private static extern bool GetEnvironmentHasBeenModified( - [UnsafeAccessorType("System.Environment, System.Private.CoreLib")] object? _); - private static SafeProcessHandle ForkAndExecProcess( ProcessStartInfo startInfo, string? resolvedFilename, string[] argv, IDictionary? env, string? cwd, bool setCredentials, uint userId, diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 87deed9c06d8cb..951cc4ae7de411 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -425,6 +425,7 @@ + diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index aa2ec8588c46e6..b5a20a7f16a2ba 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.cs @@ -42,7 +42,6 @@ public readonly struct ProcessCpuUsage public static int ProcessorCount { get; } = RuntimeFeature.IsMultithreadingSupported ? GetProcessorCount() : 1; private static NullableBool s_privilegedProcess; - private static bool s_hasBeenModified; /// /// Gets whether the current process is authorized to perform security-relevant functions. @@ -63,11 +62,7 @@ public static bool IsPrivilegedProcess // Unconditionally return false since .NET Core does not support object finalization during shutdown. public static bool HasShutdownStarted => false; - public static bool HasBeenModified - { - get => Volatile.Read(ref s_hasBeenModified); - private set => Volatile.Write(ref s_hasBeenModified, value); - } + public static bool HasBeenModified { get; private set; } public static string? GetEnvironmentVariable(string variable) { From a9cc034aaeda74140b3f4de044c487fed5de0a04 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 14:57:25 +0000 Subject: [PATCH 07/17] Address remaining process environment review feedback Co-authored-by: adamsitnik <6668460+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 2 +- .../src/System.Diagnostics.Process.csproj | 1 + .../System/Diagnostics/ProcessUtils.Unix.cs | 2 +- .../tests/ProcessStartInfoTests.cs | 58 +++++++++++++++++++ .../ref/System.Private.CoreLib.ExtraApis.cs | 3 +- .../ref/System.Private.CoreLib.ExtraApis.txt | 2 +- .../src/System/Environment.cs | 22 ++++++- .../src/CompatibilitySuppressions.xml | 2 +- 8 files changed, 85 insertions(+), 7 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 85752abaf94d51..ea0394682695e9 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -428,7 +428,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) { - if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || Environment.HasBeenModified) + if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || Environment.HasEnvironmentVariablesBeenModified) { return startInfo.Environment; } diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 951cc4ae7de411..4e4b7ca6bc0b99 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -438,6 +438,7 @@ + diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs index 75283b4280e20e..7f0d66e8d69b66 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs @@ -158,7 +158,7 @@ private static unsafe bool TryGetPasswd(string name, byte* buf, int bufLen, out } } // filename is a uri - else if (Uri.TryCreate(filename, UriKind.Absolute, out Uri? uri)) + else if (System.Uri.TryCreate(filename, System.UriKind.Absolute, out System.Uri? uri)) { if (uri.IsFile && uri.Host == "" && File.Exists(uri.LocalPath)) { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index ba1eb8445aa33c..4bf4e61ac08f6c 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -280,6 +280,64 @@ public void TestEnvironmentOfChildProcess() } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void TestEnvironmentOfChildProcess_SeesParentEnvironmentVariableSetBeforeStart() + { + const string Name = "TestEnvironmentOfChildProcess_ParentSetBeforeStart"; + string value = "\x1234" + Environment.NewLine + "\x5678"; + string? previousValue = Environment.GetEnvironmentVariable(Name); + + try + { + Process p = CreateProcess(static (variableName, expectedValue) => + { + return Environment.GetEnvironmentVariable(variableName) == expectedValue ? + RemoteExecutor.SuccessExitCode : + 1; + }, Name, value); + + Environment.SetEnvironmentVariable(Name, value); + + p.Start(); + Assert.True(p.WaitForExit(WaitInMS)); + Assert.Equal(RemoteExecutor.SuccessExitCode, p.ExitCode); + } + finally + { + Environment.SetEnvironmentVariable(Name, previousValue); + } + } + + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void TestEnvironmentOfChildProcess_SeesParentEnvironmentVariableRemovalBeforeStart() + { + const string Name = "TestEnvironmentOfChildProcess_ParentRemoveBeforeStart"; + string? previousValue = Environment.GetEnvironmentVariable(Name); + + try + { + Process p = CreateProcess(static variableName => + { + return Environment.GetEnvironmentVariable(variableName) is null ? + RemoteExecutor.SuccessExitCode : + 1; + }, Name); + + Environment.SetEnvironmentVariable(Name, "temporary-value"); + Environment.SetEnvironmentVariable(Name, null); + + p.Start(); + Assert.True(p.WaitForExit(WaitInMS)); + Assert.Equal(RemoteExecutor.SuccessExitCode, p.ExitCode); + } + finally + { + Environment.SetEnvironmentVariable(Name, previousValue); + } + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void EnvironmentNullValue() { diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs index a862feea219360..ee27dcaec2d318 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs @@ -8,7 +8,8 @@ namespace System { public static partial class Environment { - public static bool HasBeenModified { get { throw null; } } + [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")] + public static bool HasEnvironmentVariablesBeenModified { get { throw null; } } } } namespace System.Runtime.Serialization diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt index b90150e85d2dc4..f00d2da163b81c 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt @@ -1,7 +1,7 @@ # NOTE: Types/members which are not publicly exposed in System.Runtime.dll but still used internally by libraries. # Manually maintained, keep in sync with System.Private.CoreLib.cs -P:System.Environment.HasBeenModified +P:System.Environment.HasEnvironmentVariablesBeenModified T:System.Runtime.Serialization.DeserializationToken M:System.Runtime.Serialization.SerializationInfo.StartDeserialization T:System.Diagnostics.DebugProvider diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index b5a20a7f16a2ba..cb22f4b0b4356c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.cs @@ -4,6 +4,7 @@ using System.Collections; using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Runtime.Versioning; using System.Threading; namespace System @@ -62,7 +63,21 @@ public static bool IsPrivilegedProcess // Unconditionally return false since .NET Core does not support object finalization during shutdown. public static bool HasShutdownStarted => false; - public static bool HasBeenModified { get; private set; } + private static volatile bool s_hasEnvironmentVariablesBeenModified; + + [UnsupportedOSPlatform("windows")] + public static bool HasEnvironmentVariablesBeenModified + { + get + { + if (OperatingSystem.IsWindows()) + { + throw new PlatformNotSupportedException(); + } + + return s_hasEnvironmentVariablesBeenModified; + } + } public static string? GetEnvironmentVariable(string variable) { @@ -94,8 +109,11 @@ public static IDictionary GetEnvironmentVariables(EnvironmentVariableTarget targ public static void SetEnvironmentVariable(string variable, string? value) { ValidateVariable(variable); - HasBeenModified = true; SetEnvironmentVariableCore(variable, value); + if (!OperatingSystem.IsWindows()) + { + s_hasEnvironmentVariablesBeenModified = true; + } } public static void SetEnvironmentVariable(string variable, string? value, EnvironmentVariableTarget target) diff --git a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml index 62ae059b30475f..c1ccca44f024d6 100644 --- a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml +++ b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml @@ -15,6 +15,6 @@ CP0002 - M:System.Environment.get_HasBeenModified + M:System.Environment.get_HasEnvironmentVariablesBeenModified From e7d1075fa7c44b1640df87e1e011934023856bd1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:34:15 +0000 Subject: [PATCH 08/17] Address remaining review feedback: remove unused using, add lazy-path test Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../SafeHandles/SafeProcessHandle.Unix.cs | 1 - .../tests/ProcessStartInfoTests.cs | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index ea0394682695e9..bb55ae26adc648 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -7,7 +7,6 @@ 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; diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 4bf4e61ac08f6c..d9045e64604d11 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -338,6 +338,45 @@ public void TestEnvironmentOfChildProcess_SeesParentEnvironmentVariableRemovalBe } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void TestEnvironmentOfChildProcess_PristineStartInfoInheritsParentEnvironment() + { + // Verify the lazy-inheritance path: when ProcessStartInfo.Environment is never + // accessed and Environment.HasEnvironmentVariablesBeenModified is false, the child + // process should inherit the parent's full environment via a null envp on non-Apple + // Unix. We run this inside a fresh subprocess so HasEnvironmentVariablesBeenModified + // starts as false and the parent environment can be controlled precisely. + const string varName = "TestLazyEnvInheritance_Key"; + const string varValue = "TestLazyEnvInheritance_Value"; + + // Inject the variable into the outer fresh process via StartInfo.Environment + // (this does NOT call Environment.SetEnvironmentVariable, so HasEnvironmentVariablesBeenModified + // stays false inside that process). + var outerOptions = new RemoteInvokeOptions { StartInfo = new ProcessStartInfo() }; + outerOptions.StartInfo.Environment[varName] = varValue; + + RemoteExecutor.Invoke(static (name, value) => + { + // Start a child using a pristine ProcessStartInfo: do NOT access .Environment. + // Accessing .Environment would initialize _environmentVariables and bypass the + // lazy-inheritance optimization on non-Apple platforms. + var psi = new ProcessStartInfo("printenv", name) + { + RedirectStandardOutput = true, + UseShellExecute = false + }; + + using Process child = Process.Start(psi)!; + string output = child.StandardOutput.ReadToEnd().Trim(); + Assert.True(child.WaitForExit(30_000)); + Assert.Equal(0, child.ExitCode); + Assert.Equal(value, output); + + return RemoteExecutor.SuccessExitCode; + }, varName, varValue, outerOptions); + } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] public void EnvironmentNullValue() { From fc01ed6d3c6077f8b0254d93412cf4724cc07dce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 15:34:56 +0000 Subject: [PATCH 09/17] Use WaitInMS instead of hardcoded timeout in new test Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/ProcessStartInfoTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index d9045e64604d11..1e58981fd7905a 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -369,7 +369,7 @@ public void TestEnvironmentOfChildProcess_PristineStartInfoInheritsParentEnviron using Process child = Process.Start(psi)!; string output = child.StandardOutput.ReadToEnd().Trim(); - Assert.True(child.WaitForExit(30_000)); + Assert.True(child.WaitForExit(WaitInMS)); Assert.Equal(0, child.ExitCode); Assert.Equal(value, output); From 61316ccbb12cc888abb220ba2724a3af45617768 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Thu, 16 Jul 2026 17:51:29 +0200 Subject: [PATCH 10/17] address more feedback --- .../tests/ProcessStartInfoTests.cs | 106 +++++------------- .../ref/System.Private.CoreLib.ExtraApis.cs | 1 - .../src/System/Environment.Variables.Unix.cs | 8 ++ .../System/Environment.Variables.Windows.cs | 2 + .../src/System/Environment.cs | 21 ---- src/native/libs/System.Native/pal_process.c | 15 +-- 6 files changed, 39 insertions(+), 114 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 1e58981fd7905a..10e6b6c38e6253 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -280,101 +280,45 @@ public void TestEnvironmentOfChildProcess() } } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.AnyUnix)] - public void TestEnvironmentOfChildProcess_SeesParentEnvironmentVariableSetBeforeStart() + [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"; - string value = "\x1234" + Environment.NewLine + "\x5678"; string? previousValue = Environment.GetEnvironmentVariable(Name); try { - Process p = CreateProcess(static (variableName, expectedValue) => + using Process process = CreateProcess(static (expectedValue) => { - return Environment.GetEnvironmentVariable(variableName) == expectedValue ? - RemoteExecutor.SuccessExitCode : - 1; - }, Name, value); - - Environment.SetEnvironmentVariable(Name, value); - - p.Start(); - Assert.True(p.WaitForExit(WaitInMS)); - Assert.Equal(RemoteExecutor.SuccessExitCode, p.ExitCode); - } - finally - { - Environment.SetEnvironmentVariable(Name, previousValue); - } - } + Assert.Equal(expectedValue, Environment.GetEnvironmentVariable(Name)); - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.AnyUnix)] - public void TestEnvironmentOfChildProcess_SeesParentEnvironmentVariableRemovalBeforeStart() - { - const string Name = "TestEnvironmentOfChildProcess_ParentRemoveBeforeStart"; - string? previousValue = Environment.GetEnvironmentVariable(Name); + return RemoteExecutor.SuccessExitCode; + }, value); - try - { - Process p = CreateProcess(static variableName => + if (modifyParent) { - return Environment.GetEnvironmentVariable(variableName) is null ? - RemoteExecutor.SuccessExitCode : - 1; - }, Name); - - Environment.SetEnvironmentVariable(Name, "temporary-value"); - Environment.SetEnvironmentVariable(Name, null); + Environment.SetEnvironmentVariable(Name, value); + } + else + { + process.StartInfo.Environment[Name] = value; + } - p.Start(); - Assert.True(p.WaitForExit(WaitInMS)); - Assert.Equal(RemoteExecutor.SuccessExitCode, p.ExitCode); + process.Start(); + Assert.True(process.WaitForExit(WaitInMS)); + Assert.Equal(RemoteExecutor.SuccessExitCode, process.ExitCode); } finally { - Environment.SetEnvironmentVariable(Name, previousValue); - } - } - - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [PlatformSpecific(TestPlatforms.AnyUnix)] - public void TestEnvironmentOfChildProcess_PristineStartInfoInheritsParentEnvironment() - { - // Verify the lazy-inheritance path: when ProcessStartInfo.Environment is never - // accessed and Environment.HasEnvironmentVariablesBeenModified is false, the child - // process should inherit the parent's full environment via a null envp on non-Apple - // Unix. We run this inside a fresh subprocess so HasEnvironmentVariablesBeenModified - // starts as false and the parent environment can be controlled precisely. - const string varName = "TestLazyEnvInheritance_Key"; - const string varValue = "TestLazyEnvInheritance_Value"; - - // Inject the variable into the outer fresh process via StartInfo.Environment - // (this does NOT call Environment.SetEnvironmentVariable, so HasEnvironmentVariablesBeenModified - // stays false inside that process). - var outerOptions = new RemoteInvokeOptions { StartInfo = new ProcessStartInfo() }; - outerOptions.StartInfo.Environment[varName] = varValue; - - RemoteExecutor.Invoke(static (name, value) => - { - // Start a child using a pristine ProcessStartInfo: do NOT access .Environment. - // Accessing .Environment would initialize _environmentVariables and bypass the - // lazy-inheritance optimization on non-Apple platforms. - var psi = new ProcessStartInfo("printenv", name) + if (modifyParent) { - RedirectStandardOutput = true, - UseShellExecute = false - }; - - using Process child = Process.Start(psi)!; - string output = child.StandardOutput.ReadToEnd().Trim(); - Assert.True(child.WaitForExit(WaitInMS)); - Assert.Equal(0, child.ExitCode); - Assert.Equal(value, output); - - return RemoteExecutor.SuccessExitCode; - }, varName, varValue, outerOptions); + Environment.SetEnvironmentVariable(Name, previousValue); + } + } } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -486,7 +430,7 @@ private string[] ExecuteProcessAndReturnParsedOutput(Dictionary // Environment Variables are case-insensitive on Windows. // But it's possible to start a process with duplicate case-sensitive env vars using CreateProcess API (see #42029) // To mimic this behaviour, we can't use Environment.SetEnvironmentVariable here as it's case-insensitive on Windows. - // We also can't use p.StartInfo.Environment as it's comparer is set to OrdinalIgnoreCAse. + // We also can't use process.StartInfo.Environment as it's comparer is set to OrdinalIgnoreCAse. // But we can overwrite it using reflection to mimic the CreateProcess behaviour and avoid having this test call CreateProcess directly. Type.GetType("System.Collections.Specialized.DictionaryWrapper, System.Diagnostics.Process")! .GetField("_contents", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance) diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs index ee27dcaec2d318..f72a14088efcab 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs @@ -8,7 +8,6 @@ namespace System { public static partial class Environment { - [System.Runtime.Versioning.UnsupportedOSPlatformAttribute("windows")] public static bool HasEnvironmentVariablesBeenModified { get { throw null; } } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs index 1f9f9b0cdd2878..f1454e8c4f32ad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs @@ -15,6 +15,13 @@ namespace System public partial class Environment { private static Dictionary? s_environment; + private static volatile bool s_hasEnvironmentVariablesBeenModified; + + public static bool HasEnvironmentVariablesBeenModified + { + get => s_hasEnvironmentVariablesBeenModified; + private set => s_hasEnvironmentVariablesBeenModified = value; + } private static unsafe string? GetEnvironmentVariableCore(string variable) { @@ -41,6 +48,7 @@ public partial class Environment private static unsafe void SetEnvironmentVariableCore(string variable, string? value) { Debug.Assert(variable != null); + HasEnvironmentVariablesBeenModified = true; EnsureEnvironmentCached(); variable = TrimStringOnFirstZero(variable); diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs index 8a24094c505d15..6ccd6d17cc3a09 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs @@ -10,6 +10,8 @@ namespace System { public static partial class Environment { + public static bool HasEnvironmentVariablesBeenModified => throw new PlatformNotSupportedException(); + private static unsafe string? GetEnvironmentVariableCore(string variable) { var builder = new ValueStringBuilder(stackalloc char[128]); diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index cb22f4b0b4356c..dabc44cab4c4bf 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.cs @@ -4,7 +4,6 @@ using System.Collections; using System.Diagnostics; using System.Runtime.CompilerServices; -using System.Runtime.Versioning; using System.Threading; namespace System @@ -63,22 +62,6 @@ public static bool IsPrivilegedProcess // Unconditionally return false since .NET Core does not support object finalization during shutdown. public static bool HasShutdownStarted => false; - private static volatile bool s_hasEnvironmentVariablesBeenModified; - - [UnsupportedOSPlatform("windows")] - public static bool HasEnvironmentVariablesBeenModified - { - get - { - if (OperatingSystem.IsWindows()) - { - throw new PlatformNotSupportedException(); - } - - return s_hasEnvironmentVariablesBeenModified; - } - } - public static string? GetEnvironmentVariable(string variable) { ArgumentNullException.ThrowIfNull(variable); @@ -110,10 +93,6 @@ public static void SetEnvironmentVariable(string variable, string? value) { ValidateVariable(variable); SetEnvironmentVariableCore(variable, value); - if (!OperatingSystem.IsWindows()) - { - s_hasEnvironmentVariablesBeenModified = true; - } } public static void SetEnvironmentVariable(string variable, string? value, EnvironmentVariableTarget target) diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index ef7c858d74498c..53fdf8d8c1e1be 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/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" @@ -336,16 +337,6 @@ static int32_t ForkAndExecProcessInternal( int32_t* childPid, int32_t stdinFd, int32_t stdoutFd, int32_t stderrFd, int32_t* inheritedFds, int32_t inheritedFdCount, int32_t startDetached, int32_t applyPDeathSig, int32_t startSuspended); -static char** GetEnviron(void) -{ -#if HAVE_NSGETENVIRON - return *(_NSGetEnviron()); -#else - extern char** environ; - return environ; -#endif -} - #if HAVE_PR_SET_PDEATHSIG // Dedicated thread infrastructure for PR_SET_PDEATHSIG. // @@ -586,6 +577,8 @@ static int32_t ForkAndExecProcessInternal( #endif #if defined(TARGET_OSX) || defined(TARGET_MACCATALYST) + assert(NULL != envp); + #if !HAVE_FORK // On MacCatalyst, fork(2) exists in the SDK but is blocked by the kernel at runtime (EPERM). // setuid/setgid-based credential changes require fork. @@ -944,7 +937,7 @@ static int32_t ForkAndExecProcessInternal( #endif // Finally, execute the new process. execve will not return if it's successful. - execve(filename, argv, envp != NULL ? envp : GetEnviron()); + execve(filename, argv, envp != NULL ? envp : SystemNative_GetEnviron()); ExitChild(waitForChildToExecPipe[WRITE_END_OF_PIPE], errno); // execve failed } From f5cc4605f09401d5f3cef74d43c2c6099b4aba2a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 16 Jul 2026 17:19:58 +0000 Subject: [PATCH 11/17] Address remaining review feedback on docs/suppressions Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/ProcessStartInfoTests.cs | 2 +- .../System.Runtime/src/CompatibilitySuppressions.xml | 4 ++-- src/native/libs/System.Native/pal_process.h | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 10e6b6c38e6253..3f71d40c436f30 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -430,7 +430,7 @@ private string[] ExecuteProcessAndReturnParsedOutput(Dictionary // Environment Variables are case-insensitive on Windows. // But it's possible to start a process with duplicate case-sensitive env vars using CreateProcess API (see #42029) // To mimic this behaviour, we can't use Environment.SetEnvironmentVariable here as it's case-insensitive on Windows. - // We also can't use process.StartInfo.Environment as it's comparer is set to OrdinalIgnoreCAse. + // We also can't use p.StartInfo.Environment as it's comparer is set to OrdinalIgnoreCAse. // But we can overwrite it using reflection to mimic the CreateProcess behaviour and avoid having this test call CreateProcess directly. Type.GetType("System.Collections.Specialized.DictionaryWrapper, System.Diagnostics.Process")! .GetField("_contents", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance) diff --git a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml index c1ccca44f024d6..00f9b01d029647 100644 --- a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml +++ b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml @@ -11,10 +11,10 @@ CP0002 - M:System.Runtime.Serialization.SerializationInfo.StartDeserialization + M:System.Environment.get_HasEnvironmentVariablesBeenModified CP0002 - M:System.Environment.get_HasEnvironmentVariablesBeenModified + M:System.Runtime.Serialization.SerializationInfo.StartDeserialization diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index 61ef54c65001ca..abde3d3a11b320 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/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; NULL to inherit the current environment + char* const envp[], // envp argument to execve; NULL to inherit the current environment (except on Apple platforms, where envp must be non-NULL) 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 From ac585916c98aed44ead2a07c88bcb2d6696cce62 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 17 Jul 2026 10:54:56 +0000 Subject: [PATCH 12/17] Address review feedback: reverts, UnsafeAccessor usage, and test updates Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com> --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 13 ++++++++++++- .../src/System.Diagnostics.Process.csproj | 2 -- .../src/System/Diagnostics/ProcessUtils.Unix.cs | 2 +- .../tests/ProcessStartInfoTests.cs | 12 ++++++------ .../ref/System.Private.CoreLib.ExtraApis.cs | 7 ------- .../ref/System.Private.CoreLib.ExtraApis.txt | 1 - .../src/System/Environment.Variables.Unix.cs | 8 ++------ .../src/System/Environment.Variables.Windows.cs | 2 +- .../src/CompatibilitySuppressions.xml | 4 ---- src/native/libs/System.Native/pal_process.c | 3 +++ 10 files changed, 25 insertions(+), 29 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index bb55ae26adc648..df44d3017b1219 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -7,6 +7,8 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; +using System.Reflection; +using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; using System.Runtime.Versioning; @@ -427,7 +429,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) { - if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || Environment.HasEnvironmentVariablesBeenModified) + if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || HasEnvironmentVariablesBeenModified()) { return startInfo.Environment; } @@ -435,6 +437,15 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st return startInfo._environmentVariables; } + private static readonly bool s_hasEnvironmentVariablesBeenModifiedAccessor = + typeof(Environment).GetMethod("get_HasEnvironmentVariablesBeenModified", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic) is not null; + + private static bool HasEnvironmentVariablesBeenModified() => + s_hasEnvironmentVariablesBeenModifiedAccessor && GetHasEnvironmentVariablesBeenModified(null); + + [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? env, string? cwd, bool setCredentials, uint userId, diff --git a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj index 4e4b7ca6bc0b99..87deed9c06d8cb 100644 --- a/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj +++ b/src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj @@ -425,7 +425,6 @@ - @@ -438,7 +437,6 @@ - diff --git a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs index 7f0d66e8d69b66..75283b4280e20e 100644 --- a/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/System/Diagnostics/ProcessUtils.Unix.cs @@ -158,7 +158,7 @@ private static unsafe bool TryGetPasswd(string name, byte* buf, int bufLen, out } } // filename is a uri - else if (System.Uri.TryCreate(filename, System.UriKind.Absolute, out System.Uri? uri)) + else if (Uri.TryCreate(filename, UriKind.Absolute, out Uri? uri)) { if (uri.IsFile && uri.Host == "" && File.Exists(uri.LocalPath)) { diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 3f71d40c436f30..309e3d478ebda8 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -283,9 +283,9 @@ 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) + [InlineData("null", false)] + [InlineData("null", true)] + public void SeesEnvironmentVariableSetBeforeStart(string value, bool modifyParent) { const string Name = "TestEnvironmentOfChildProcess_ParentSetBeforeStart"; string? previousValue = Environment.GetEnvironmentVariable(Name); @@ -294,18 +294,18 @@ public void SeesEnvironmentVariableSetBeforeStart(string? value, bool modifyPare { using Process process = CreateProcess(static (expectedValue) => { - Assert.Equal(expectedValue, Environment.GetEnvironmentVariable(Name)); + Assert.Equal(expectedValue == "null" ? null : expectedValue, Environment.GetEnvironmentVariable(Name)); return RemoteExecutor.SuccessExitCode; }, value); if (modifyParent) { - Environment.SetEnvironmentVariable(Name, value); + Environment.SetEnvironmentVariable(Name, value == "null" ? null : value); } else { - process.StartInfo.Environment[Name] = value; + process.StartInfo.Environment[Name] = value == "null" ? null : value; } process.Start(); diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs index f72a14088efcab..7fbc3da42edee1 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.cs @@ -4,13 +4,6 @@ // NOTE: Types/members which are not publicly exposed in System.Runtime.dll but still used internally by libraries. // Manually maintained, keep in sync with System.Private.CoreLib.ExtraApis.txt -namespace System -{ - public static partial class Environment - { - public static bool HasEnvironmentVariablesBeenModified { get { throw null; } } - } -} namespace System.Runtime.Serialization { public readonly partial struct DeserializationToken : System.IDisposable diff --git a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt index f00d2da163b81c..477f41ba86acf7 100644 --- a/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt +++ b/src/libraries/System.Private.CoreLib/ref/System.Private.CoreLib.ExtraApis.txt @@ -1,7 +1,6 @@ # NOTE: Types/members which are not publicly exposed in System.Runtime.dll but still used internally by libraries. # Manually maintained, keep in sync with System.Private.CoreLib.cs -P:System.Environment.HasEnvironmentVariablesBeenModified T:System.Runtime.Serialization.DeserializationToken M:System.Runtime.Serialization.SerializationInfo.StartDeserialization T:System.Diagnostics.DebugProvider diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs index f1454e8c4f32ad..a01060c13e1596 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs @@ -17,11 +17,7 @@ public partial class Environment private static Dictionary? s_environment; private static volatile bool s_hasEnvironmentVariablesBeenModified; - public static bool HasEnvironmentVariablesBeenModified - { - get => s_hasEnvironmentVariablesBeenModified; - private set => s_hasEnvironmentVariablesBeenModified = value; - } + internal static bool HasEnvironmentVariablesBeenModified => s_hasEnvironmentVariablesBeenModified; private static unsafe string? GetEnvironmentVariableCore(string variable) { @@ -48,7 +44,7 @@ public static bool HasEnvironmentVariablesBeenModified private static unsafe void SetEnvironmentVariableCore(string variable, string? value) { Debug.Assert(variable != null); - HasEnvironmentVariablesBeenModified = true; + s_hasEnvironmentVariablesBeenModified = true; EnsureEnvironmentCached(); variable = TrimStringOnFirstZero(variable); diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs index 6ccd6d17cc3a09..2f32fcc8eed530 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs @@ -10,7 +10,7 @@ namespace System { public static partial class Environment { - public static bool HasEnvironmentVariablesBeenModified => throw new PlatformNotSupportedException(); + internal static bool HasEnvironmentVariablesBeenModified => throw new PlatformNotSupportedException(); private static unsafe string? GetEnvironmentVariableCore(string variable) { diff --git a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml index 00f9b01d029647..8d6cd745858ae4 100644 --- a/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml +++ b/src/libraries/System.Runtime/src/CompatibilitySuppressions.xml @@ -9,10 +9,6 @@ CP0002 M:System.Diagnostics.Debug.SetProvider(System.Diagnostics.DebugProvider) - - CP0002 - M:System.Environment.get_HasEnvironmentVariablesBeenModified - CP0002 M:System.Runtime.Serialization.SerializationInfo.StartDeserialization diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 53fdf8d8c1e1be..84a2a2134aaac1 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -577,6 +577,9 @@ static int32_t ForkAndExecProcessInternal( #endif #if defined(TARGET_OSX) || defined(TARGET_MACCATALYST) + // We cannot substitute SystemNative_GetEnviron() here: this path uses posix_spawn + // without fork, so concurrent setenv/putenv/unsetenv/clearenv in other threads can + // mutate the shared process environment while posix_spawn is consuming envp. assert(NULL != envp); #if !HAVE_FORK From b5045c14fa40badd8ebeccaed3bcef15736cbc5b Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Jul 2026 13:53:23 +0200 Subject: [PATCH 13/17] address my own feedback --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 9 +-------- .../src/ILLink/ILLink.Descriptors.LibraryBuild.xml | 4 ++++ 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index df44d3017b1219..19209634bcf5d4 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -7,7 +7,6 @@ using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.IO; -using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; using System.Runtime.InteropServices.Marshalling; @@ -429,7 +428,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) { - if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || HasEnvironmentVariablesBeenModified()) + if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || GetHasEnvironmentVariablesBeenModified(null)) { return startInfo.Environment; } @@ -437,12 +436,6 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st return startInfo._environmentVariables; } - private static readonly bool s_hasEnvironmentVariablesBeenModifiedAccessor = - typeof(Environment).GetMethod("get_HasEnvironmentVariablesBeenModified", BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic) is not null; - - private static bool HasEnvironmentVariablesBeenModified() => - s_hasEnvironmentVariablesBeenModifiedAccessor && GetHasEnvironmentVariablesBeenModified(null); - [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "get_HasEnvironmentVariablesBeenModified")] private static extern bool GetHasEnvironmentVariablesBeenModified([UnsafeAccessorType("System.Environment, System.Private.CoreLib")] object? _); diff --git a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml index 9ea6ada68e8cf6..4203cb285a09de 100644 --- a/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml +++ b/src/libraries/System.Private.CoreLib/src/ILLink/ILLink.Descriptors.LibraryBuild.xml @@ -8,6 +8,10 @@ + + + + From 08895f741230056447faa66250a99afcb1aabc7c Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Jul 2026 16:12:20 +0200 Subject: [PATCH 14/17] fix the build: Environment.Variables.Windows.cs is included on Unix for CLR, it has caused a lot of confusion... --- .../System.Private.CoreLib/src/System/Environment.Unix.cs | 6 ++++++ .../src/System/Environment.Variables.Unix.cs | 4 ---- .../src/System/Environment.Variables.Windows.cs | 2 -- .../System.Private.CoreLib/src/System/Environment.cs | 3 +++ 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs index 7dd6f903cf9dfa..4a5129806823dc 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Unix.cs +++ b/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 diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs index a01060c13e1596..1f9f9b0cdd2878 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Unix.cs @@ -15,9 +15,6 @@ namespace System public partial class Environment { private static Dictionary? s_environment; - private static volatile bool s_hasEnvironmentVariablesBeenModified; - - internal static bool HasEnvironmentVariablesBeenModified => s_hasEnvironmentVariablesBeenModified; private static unsafe string? GetEnvironmentVariableCore(string variable) { @@ -44,7 +41,6 @@ public partial class Environment private static unsafe void SetEnvironmentVariableCore(string variable, string? value) { Debug.Assert(variable != null); - s_hasEnvironmentVariablesBeenModified = true; EnsureEnvironmentCached(); variable = TrimStringOnFirstZero(variable); diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs index 2f32fcc8eed530..8a24094c505d15 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Environment.Variables.Windows.cs @@ -10,8 +10,6 @@ namespace System { public static partial class Environment { - internal static bool HasEnvironmentVariablesBeenModified => throw new PlatformNotSupportedException(); - private static unsafe string? GetEnvironmentVariableCore(string variable) { var builder = new ValueStringBuilder(stackalloc char[128]); diff --git a/src/libraries/System.Private.CoreLib/src/System/Environment.cs b/src/libraries/System.Private.CoreLib/src/System/Environment.cs index dabc44cab4c4bf..b8f676f0fcea12 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Environment.cs +++ b/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); } From 5ecbfa64f73baf36b72bdef91967e66ceac5acf3 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Jul 2026 16:19:03 +0200 Subject: [PATCH 15/17] address code review feedback: apply the optimization on apple platforms as well --- .../Win32/SafeHandles/SafeProcessHandle.Unix.cs | 9 +-------- src/native/libs/System.Native/pal_process.c | 7 +------ src/native/libs/System.Native/pal_process.h | 2 +- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs index 19209634bcf5d4..03da601f06a1d5 100644 --- a/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs +++ b/src/libraries/System.Diagnostics.Process/src/Microsoft/Win32/SafeHandles/SafeProcessHandle.Unix.cs @@ -427,14 +427,7 @@ private static bool UsesTerminal(SafeFileHandle? stdinHandle, SafeFileHandle? st => ProcessUtils.IsTerminal(stdinHandle) || ProcessUtils.IsTerminal(stdoutHandle) || ProcessUtils.IsTerminal(stderrHandle); private static IDictionary? GetEnvironmentVariables(ProcessStartInfo startInfo) - { - if (OperatingSystem.IsMacOS() || OperatingSystem.IsMacCatalyst() || GetHasEnvironmentVariablesBeenModified(null)) - { - return startInfo.Environment; - } - - return startInfo._environmentVariables; - } + => GetHasEnvironmentVariablesBeenModified(null) ? startInfo.Environment : startInfo._environmentVariables; [UnsafeAccessor(UnsafeAccessorKind.StaticMethod, Name = "get_HasEnvironmentVariablesBeenModified")] private static extern bool GetHasEnvironmentVariablesBeenModified([UnsafeAccessorType("System.Environment, System.Private.CoreLib")] object? _); diff --git a/src/native/libs/System.Native/pal_process.c b/src/native/libs/System.Native/pal_process.c index 84a2a2134aaac1..d6d16aaf83ba79 100644 --- a/src/native/libs/System.Native/pal_process.c +++ b/src/native/libs/System.Native/pal_process.c @@ -577,11 +577,6 @@ static int32_t ForkAndExecProcessInternal( #endif #if defined(TARGET_OSX) || defined(TARGET_MACCATALYST) - // We cannot substitute SystemNative_GetEnviron() here: this path uses posix_spawn - // without fork, so concurrent setenv/putenv/unsetenv/clearenv in other threads can - // mutate the shared process environment while posix_spawn is consuming envp. - assert(NULL != envp); - #if !HAVE_FORK // On MacCatalyst, fork(2) exists in the SDK but is blocked by the kernel at runtime (EPERM). // setuid/setgid-based credential changes require fork. @@ -718,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); diff --git a/src/native/libs/System.Native/pal_process.h b/src/native/libs/System.Native/pal_process.h index abde3d3a11b320..61ef54c65001ca 100644 --- a/src/native/libs/System.Native/pal_process.h +++ b/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; NULL to inherit the current environment (except on Apple platforms, where envp must be non-NULL) + 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 From cb1e290dc206f1b6831467b3a1b8e46addfc39fb Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Jul 2026 16:25:55 +0200 Subject: [PATCH 16/17] add isolation layer to the test, so each test has it's own copy of the static shared state --- .../tests/ProcessStartInfoTests.cs | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 309e3d478ebda8..3f38800476d8b4 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -290,35 +290,35 @@ public void SeesEnvironmentVariableSetBeforeStart(string value, bool modifyParen const string Name = "TestEnvironmentOfChildProcess_ParentSetBeforeStart"; string? previousValue = Environment.GetEnvironmentVariable(Name); - try + // We run a dedicated process to ensure that each test has it's own copy of the static shared state. + using Process isolation = CreateProcess((expectedValue, modifyParentStr) => { - using Process process = CreateProcess(static (expectedValue) => + using Process process = CreateProcess(static (value) => { - Assert.Equal(expectedValue == "null" ? null : expectedValue, Environment.GetEnvironmentVariable(Name)); + Assert.Equal(value == "null" ? null : value, Environment.GetEnvironmentVariable(Name)); return RemoteExecutor.SuccessExitCode; - }, value); + }, expectedValue); - if (modifyParent) + if (bool.Parse(modifyParentStr)) { - Environment.SetEnvironmentVariable(Name, value == "null" ? null : value); + Environment.SetEnvironmentVariable(Name, expectedValue == "null" ? null : expectedValue); } else { - process.StartInfo.Environment[Name] = value == "null" ? null : value; + process.StartInfo.Environment[Name] = expectedValue == "null" ? null : expectedValue; } process.Start(); Assert.True(process.WaitForExit(WaitInMS)); Assert.Equal(RemoteExecutor.SuccessExitCode, process.ExitCode); - } - finally - { - if (modifyParent) - { - Environment.SetEnvironmentVariable(Name, previousValue); - } - } + + 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))] From c0f3fd41b949d098b3707019c262d58ebfb170f6 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Fri, 17 Jul 2026 16:58:14 +0200 Subject: [PATCH 17/17] Apply suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- .../System.Diagnostics.Process/tests/ProcessStartInfoTests.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 3f38800476d8b4..bd32ad6fc92878 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -288,9 +288,8 @@ public void TestEnvironmentOfChildProcess() public void SeesEnvironmentVariableSetBeforeStart(string value, bool modifyParent) { const string Name = "TestEnvironmentOfChildProcess_ParentSetBeforeStart"; - string? previousValue = Environment.GetEnvironmentVariable(Name); - // We run a dedicated process to ensure that each test has it's own copy of the static shared state. + // 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) =>