Ignore ERROR_PIPE_NOT_CONNECTED in WindowsConsoleStream.Write #75734
Conversation
I don't know under what circumstances `ERROR_PIPE_NOT_CONNECTED` is produced instead of `ERROR_BROKEN_PIPE`, but some of our users were experiencing exceptions due to this error code. `ERROR_BROKEN_PIPE` and `ERROR_PIPE_NOT_CONNECTED` are treated the same in many other locations in the dotnet src.
ghost
added
the
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-console Issue DetailsI don't know under what circumstances
|
|
This seems reasonable. Is there a reason we wouldn't also do so earlier in the same file here? What about in these FileStream/RandomAccess cases? It'd be good to understand in what situations we'd actually get ERROR_PIPE_NOT_CONNECTED vs ERROR_BROKEN_PIPE and make sure we're consistently handling each everywhere we should be. cc: @adamsitnik |
|
Unfortunately I couldn't figure out the difference from google searches. |
There was a problem hiding this comment.
Based on reading various websites and our own comments my understanding is following:
ERROR_BROKEN_PIPEshould be reported when client or server closes the handle (easy to reproduce)ERROR_PIPE_NOT_CONNECTEDshould be reported after server disconnects (not closes).
In my opinion it's perfectly fine to "ignore" ERROR_PIPE_NOT_CONNECTED for Console, as we don't have the control over named pipe server.
I am not sure about other places pointed by @stephentoub like FileStream or RandomAccess, where this error might mean that somebody has disconnected the server on purpose. I would expect that we throw in such scenarios.
Speaking of which, I've prepared following repro app to demonstrate the diffrences in behaviours:
using System.IO.Pipes;
using System.Text;
namespace NamedPipes
{
internal class Program
{
// usage: dotnet run -c Release server $pipeName
// dotnet run -c Release clientPipe $pipeName
// dotnet run -c Release clientFileStream $pipeName
static async Task Main(string[] args)
{
string pipeName = args[1];
CancellationTokenSource timeout = new(TimeSpan.FromSeconds(10));
if (args[0] == "server")
{
using NamedPipeServerStream server = new(pipeName, PipeDirection.In, maxNumberOfServerInstances: 1, PipeTransmissionMode.Byte, PipeOptions.Asynchronous);
Console.WriteLine("Server waiting for client to connect");
await server.WaitForConnectionAsync(timeout.Token);
Console.WriteLine("Server established a connection!");
using StreamReader streamReader = new StreamReader(server, Encoding.UTF8);
Console.WriteLine(await streamReader.ReadLineAsync());
Console.WriteLine("Will disconnect now!");
await server.DisposeAsync();
// don't quit immediately (handle is closed and client gets a different error)
await Task.Delay(TimeSpan.FromSeconds(10));
}
else if (args[0] == "clientPipe")
{
using NamedPipeClientStream client = new(".", pipeName, PipeDirection.Out, PipeOptions.Asynchronous);
Console.WriteLine("Client waiting to connect to a server");
await client.ConnectAsync(timeout.Token);
Console.WriteLine("Client connected!");
await client.WriteAsync(Encoding.UTF8.GetBytes($"Line 1 {Environment.NewLine}"), timeout.Token);
await client.WriteAsync(Encoding.UTF8.GetBytes($"Line 2 {Environment.NewLine}"), timeout.Token);
}
else if (args[0] == "clientFileStream")
{
using FileStream fileStream = new($@"\\.\pipe\{pipeName}", FileMode.Open, FileAccess.Write, FileShare.None, 0, FileOptions.Asynchronous);
Console.WriteLine("FileStream connected!");
await fileStream.WriteAsync(Encoding.UTF8.GetBytes($"Line 1 {Environment.NewLine}"), timeout.Token);
await fileStream.WriteAsync(Encoding.UTF8.GetBytes($"Line 2 {Environment.NewLine}"), timeout.Token);
}
}
}
}In one terminal, I run:
dotnet run -c Release server uniqueThen in other:
dotnet run -c Release clientFileStream uniqueThe error is ignored (no exception being thrown):
FileStream connected!
And again, I start the server in first tab, and then pipe client in other:
dotnet run -c Release clientPipe uniquePS C:\Users\adsitnik\source\repos\NamedPipes_Disconnect> dotnet run -c Release clientPipe unique
Client waiting to connect to a server
Client connected!
Unhandled exception. System.IO.IOException: Pipe is broken.
at System.IO.Pipes.PipeStream.WriteAsyncCore(ReadOnlyMemory`1 buffer, CancellationToken cancellationToken)
at System.IO.Pipes.PipeStream.WriteAsync(ReadOnlyMemory`1 buffer, CancellationToken cancellationToken)
at NamedPipes.Program.Main(String[] args) in C:\Users\adsitnik\source\repos\NamedPipes_Disconnect\Program.cs:line 42
at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecutionContextCallback(Object s)
at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.MoveNext(Thread threadPoolThread)
at System.Runtime.CompilerServices.AsyncTaskMethodBuilder`1.AsyncStateMachineBox`1.ExecuteFromThreadPool(Thread threadPoolThread)
at System.Threading.ThreadPoolWorkQueue.Dispatch()
at System.Threading.PortableThreadPool.WorkerThread.WorkerThreadStart()
--- End of stack trace from previous location ---
at NamedPipes.Program.Main(String[] args) in C:\Users\adsitnik\source\repos\NamedPipes_Disconnect\Program.cs:line 42
at NamedPipes.Program.<Main>(String[] args)
@stephentoub I think that we should extend this PR to ignore the error for reading from console as you have suggested and then send another one which should unify the edge case scenarios handling for FileStream/RandomAccess and pipe clients and servers. What do you think?
There was a problem hiding this comment.
The proposed change LGTM, I am going to send a separate PR with error handling unification.
Thank you for your contribution @Chicken-Bones !
Was going to say "sounds good", but as this is now merged, a separate PR is fine :) |
|
Follow up PR: #77543 |
ghost
locked as resolved and limited conversation to collaborators
I don't know under what circumstances
ERROR_PIPE_NOT_CONNECTEDis produced instead ofERROR_BROKEN_PIPE, but some of our users were experiencing exceptions due to this error code.ERROR_BROKEN_PIPEandERROR_PIPE_NOT_CONNECTEDare treated the same in many other locations in the dotnet src.