Revert "Use socketpair to implement Process.Start redirection" (#47461) #47644
Conversation
…t#47461) * Revert dotnet#34861 * reinstate removed test * Update src/libraries/System.Diagnostics.Process/tests/ProcessStreamReadTests.cs Co-authored-by: Stephen Toub <stoub@microsoft.com> Co-authored-by: Stephen Toub <stoub@microsoft.com>
eiriktsarpalis
added
Servicing-consider
|
Tagging subscribers to this area: Issue DetailsBackport of #47461 to release/5.0, fixing #46469. Customer ImpactMultiple customers reporting a Linux regression introduced in .NET 5: if a .NET process spawns a child process with redirected IO, the child process will fail with TestingAdded a unit test that validates the regressed behaviour. RiskLow. This reverts to the IO redirection implementation used in previous .NET Core versions.
|
* Add test validating against regression in dotnet#46469 * fix test bug
eiriktsarpalis
force-pushed
the
revert-processsocketpair-5.0
branch
from
a256ec6
to
31a10e2
Compare
|
@eiriktsarpalis could you please note in the template what is lost by returning to the old behavior -- presumably there was a reason for the original change -- will we break anyone that now depends on it. |
Prior to that original change, "async" operations performed on a Process' stdin/stdout/stderr stream would a) be async-over-sync (meaning they'd actually be done with sync operations that block thread pool threads) and b) wouldn't be cancelable. The change made it so that async operations on such streams were truly async and were cancelable. |
OK so with this revert, nothing will break? they will simply not cancel successfully when requested to. |
Essentially. Cancellation may not take effect, and thread pool threads may be blocked. The former impacts responsiveness, and the latter impacts scalability. |
|
Another thing you might mention is whether this was a clean revert or needed some hand merging. The latter adds risk. |
leecow
added
Servicing-approved
ghost
locked as resolved and limited conversation to collaborators
Backport of #47461 and #47643 to release/5.0, fixing #46469.
Customer Impact
Multiple customers reporting a Linux regression introduced in .NET 5: if a .NET process spawns a child process with redirected IO, the child process will fail with
ENXIOif it attempts toopenthe/dev/std*file paths. There are no known workarounds other than changing the code of the child process.Testing
Added a unit test that validates the regressed behaviour.
Risk
Low. This reverts to the IO redirection implementation used in previous .NET Core versions. The revert does not change functional behaviour but could have impact on scalability and responsiveness, per the comments in #47644 (comment). The change is a clean revert, incorporating a test that verifies the regression.