• Add android, ios, tvos, maccatalyst to TargetFrameworks in System.Diagnostics.Process.Tests.csproj

@copilot For now, please stick only with android

• Add android, ios, tvos, maccatalyst to TargetFrameworks in System.Diagnostics.Process.Tests.csproj

@copilot For now, please stick only with android

Done in 403f131. Removed ios, tvos, and maccatalyst from TargetFrameworks, and renamed the test file to ProcessTests.Android.cs with its compile condition scoped to android only.

@copilot address my feedback

Addressed in ba2dc9c — removed the GetAndroidProgramPath helper, replaced all var with explicit types, and dropped the now-unused System.IO import.

/azp list

/azp run runtime-android runtime-androidemulator

/azp run runtime-android

/azp run runtime-androidemulator

@copilot the tests fail due to recently introduced ProcessHandlesTests tests. Go to [ProcessHandlesTests.cs](https://github.com/dotnet/runtime/blob/7602c30b43e12e3ced08087cc1503f017ac4206e/src/libra...

Done in a2efbde. Added [SkipOnPlatform(TestPlatforms.Android, "sh is not available on Android")] to the ProcessHandlesTests class.

/azp run runtime-android

/azp run runtime-androidemulator

🤖 Copilot Code Review — PR #126099

Holistic Assessment

Motivation: This PR adds StandardInputHandle, StandardOutputHandle, and StandardErrorHandle properties to ProcessStartInfo, enabling users to pass pre-created SafeFileHandle objects directly to child processes. This addresses a real need (process piping, redirecting to files/null without going through StreamReader/StreamWriter). The refactoring to centralize pipe creation in Process.cs and unify the process-start lock across platforms is well-motivated.

Approach: The approach is sound — moving pipe creation from platform-specific StartCore methods into the shared Start() method reduces duplication and makes handle management consistent. The native ForkAndExecProcess API change (accepting pre-created fds instead of creating pipes internally) is clean. The DuplicateAsInheritableIfNeeded pattern on Windows avoids unnecessary handle duplication.

Summary: ⚠️ Needs Human Review. The code is well-structured and the handle ownership model is correct. However, I identified two concerns that warrant human reviewer attention: (1) API approval verification for issue #125848 (I cannot verify the api-approved label), and (2) a minor handle leak risk in the Windows duplication path. The behavioral change to usesTerminal detection on Unix is actually more correct but should be consciously acknowledged.

Detailed Findings

⚠️ API Approval — Cannot verify api-approved label on #125848

This PR adds new public API surface in ref/System.Diagnostics.Process.cs:

public SafeFileHandle? StandardErrorHandle { get; set; }
public SafeFileHandle? StandardInputHandle { get; set; }
public SafeFileHandle? StandardOutputHandle { get; set; }

The commit message references issue #125848. Per dotnet/runtime policy, new public APIs require an api-approved label on the linked issue. I cannot access the GitHub issue to verify this label. A human reviewer should confirm that #125848 has api-approved and that the implementation matches the approved shape exactly (property names, types, nullability).

⚠️ Windows Handle Leak on Partial Duplication Failure

File: Process.Windows.cs lines 458-460

DuplicateAsInheritableIfNeeded(stdinHandle, ref inheritableStdinHandle);
DuplicateAsInheritableIfNeeded(stdoutHandle, ref inheritableStdoutHandle);   // If this throws...
DuplicateAsInheritableIfNeeded(stderrHandle, ref inheritableStderrHandle);   // ...never reached

If DuplicateHandle succeeds for stdin but fails for stdout, the exception propagates to the catch block at line 589 which disposes procSH, then the finally block at line 594 disposes inheritableStdinHandle (correct). However, the catch block does not explicitly clean up duplicated handles — it relies on the finally block. This actually works correctly because finally always runs and disposes all three nullable handles. (Flagged as advisory — verified it's handled, but the code flow is non-obvious.)

⚠️ usesTerminal Detection Behavioral Change (Unix)

File: Process.Unix.cs lines 389-392

Old behavior:

bool usesTerminal = !(startInfo.RedirectStandardInput &&
                      startInfo.RedirectStandardOutput &&
                      startInfo.RedirectStandardError);

New behavior:

bool usesTerminal = (stdinHandle is not null && Interop.Sys.IsATty(stdinHandle))
    || (stdoutHandle is not null && Interop.Sys.IsATty(stdoutHandle))
    || (stderrHandle is not null && Interop.Sys.IsATty(stderrHandle));

The new code is more correct — it checks whether the actual handles point to a TTY rather than relying on the heuristic "any non-redirected stream implies terminal use." This means a process whose parent has stdout piped to a file will no longer trigger unnecessary terminal configuration. However, this is a behavioral change that should be consciously acknowledged. It could affect edge cases where the old heuristic triggered terminal configuration that happened to be needed.

✅ Handle Ownership Model — Correct

The ownership model in Process.cs Start() is well-designed:

• User-provided handles (StandardInputHandle/etc.) are NOT disposed by the framework (lines 1410-1421 check is null to skip disposal).
• Pipe handles created by CreateAnonymousPipe are properly disposed in finally blocks.
• Console handles from Console.OpenStandard*Handle() are safe to dispose (they don't own the underlying resource).
• Parent pipe handles are disposed in the catch block on failure, and wrapped in streams on success.

No double-close or use-after-dispose risks were found.

✅ Android Support — Correct

On Android, Console.OpenStandard*Handle() throws PlatformNotSupportedException. The code correctly guards with !OperatingSystem.IsAndroid() checks (lines 1349, 1362, 1375). When handles are null, the native ForkAndExecProcess receives fd=-1, which correctly causes the child to inherit the parent's handles (verified in pal_process.c line 362: stdinFd != -1 && ...).

✅ Locking Strategy — Sound

• s_processStartLock correctly moved to ProcessUtils.cs for cross-platform sharing.
• Windows always acquires a write lock for CreateProcess (same serialization as old lock(s_createProcessLock) — no regression).
• Unix uses read locks for ForkAndExecProcess (allows concurrent starts) and write locks for terminal configuration and SIGCHLD handling.
• Conditional pipe-creation lock in Start() only activates on Unix without pipe2 support — correctly avoids unnecessary locking when atomic non-inheritable pipe creation is available.
• No deadlock risks identified (lock upgrade in Unix finally block properly exits read lock before entering write lock).

✅ Native Interop Changes — Correct

Interop.ForkAndExecProcess.cs correctly uses DangerousAddRef/DangerousRelease to pin handles during the native call (lines 30-67). The SafeFileHandle? → int conversion handles null correctly (defaults to -1). The native C code in pal_process.c skips dup2 for fd=-1, correctly inheriting parent handles.

✅ Test Coverage — Good

ProcessHandlesTests.cs covers key scenarios: pipe redirection (sync/async), separate vs. combined stdout/stderr, inherited handles, process piping, and mutual exclusion with RedirectStandard* properties. The test is correctly skipped on Android (no sh). ProcessTests.Android.cs provides basic Android coverage.

💡 Error Message Precision

File: Process.cs lines 1291-1301

When StandardInputHandle conflicts with RedirectStandardInput, the error message SR.CantSetHandleAndRedirect references all six properties:

"The StandardInputHandle, StandardOutputHandle, and StandardErrorHandle properties cannot be used together with the corresponding RedirectStandardInput, RedirectStandardOutput, and RedirectStandardError properties."

This is thrown even when only one pair conflicts. A more precise message identifying the specific conflicting pair would be clearer, though the current message is acceptable since it explains the general constraint.

💡 Unix OpenStream Ownership Transfer Pattern

File: Process.Unix.cs lines 718-720

SafePipeHandle safePipeHandle = new(handle.DangerousGetHandle(), ownsHandle: true);
handle.SetHandleAsInvalid();

This ownership transfer via DangerousGetHandle() + SetHandleAsInvalid() is correct but non-obvious. If the SafePipeHandle constructor threw between these two lines, the handle would leak. In practice, the constructor can't throw for a valid IntPtr, so this is safe. The existing comment explains the intent well.

Generated by Code Review for issue #126099 · ◷

Motivation: This PR adds StandardInputHandle, StandardOutputHandle, and StandardErrorHandle properties to ProcessStartInfo, enabling users to pass pre-created SafeFileHandle objects directly to child processes. This addresses a

/ba-g failures are unrelated, the Android tests for Process are passing now

Tagging subscribers to 'arch-android': @vitek-karas, @simonrozsival, @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.