HavenDV / H.Pipes

A simple, easy to use, strongly-typed, async wrapper around .NET named pipes.
MIT License
242 stars 26 forks source link

PipeServer crashing with many connections, IOException: 'Pipe is broken' #22

Open erezwanderman opened 2 years ago

erezwanderman commented 2 years ago

Describe the bug

I have a program with a client that sends a message and waits for a response, and a server that waits for clients, and when one sends a message, the server sends a response.

Sometimes the server crashes when it tries to send a response with args.Connection.WriteAsync image

   at System.IO.Pipes.PipeStream.CheckWriteOperations()
   at System.IO.Pipes.PipeStream.Flush()
   at System.IO.Pipes.PipeStream.FlushAsync(CancellationToken cancellationToken)
--- End of stack trace from previous location ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at H.Pipes.IO.PipeStreamWriter.<WriteAsync>d__8.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at H.Pipes.IO.PipeStreamWrapper.<WriteAsync>d__17.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at H.Pipes.PipeConnection`1.<WriteAsync>d__38.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at Program.<>c.<<<Main>$>b__0_2>d.MoveNext() in C:\Users\Erez\source\repos\HPipeMultiClient\Server\Server.cs:line 21

Happens when running the server in debug from Visual Studio. Doesn't happen when running the server from console.

Steps to reproduce the bug

  1. In Visual Studio 2022, Open this minimal solution that reproduces the problem: https://github.com/erezwanderman/HPipeMultiClient
  2. Build the solution and start debugging (the Server project)
  3. Start Client.exe outside of Visual Studio
  4. Watch the server crash on line 21

server.MessageReceived += async (sender, args) => { Console.WriteLine($"Client {args.Connection.PipeName} says: {args.Message}"); var response = new Message { Response = args.Message!.Request + args.Message!.Request }; Console.WriteLine($"Sending to {args.Connection.PipeName}: {response}"); await args.Connection.WriteAsync(response); };

Expected behavior

No crash, send message successfully

Screenshots

image

NuGet package version

2.0.37

Platform

Console

IDE

Visual Studio 2022

Additional context

No response

HavenDV commented 2 years ago

I remember having some problems with Debug when I didn't use H.Pipes.AccessControl for the server. Are you using it? In particular server.AllowUsersReadWrite();

erezwanderman commented 2 years ago

Yes, I used H.Formatters.MessagePack & H.Pipes.AccessControl in the original program but not in this reproduction example.

Now added them in a new branch: https://github.com/erezwanderman/HPipeMultiClient/tree/MessagePack%26AccessControl. It does not change behaviour. still crashing.

erezwanderman commented 2 years ago

After analyzing it furthuer, I think that in IO/PipeStreamWriter.cs, PipeStreamWriter.WriteAsync, the call BaseStream.WriteAsync sends the response, and sometimes the client is fast enough to quickly disconnect before the server finishes flushing and draining, and then the server throws exception on flushing or draining.

It can be avoided by catching and ignoring in PipeStreamWriter.cs call "await BaseStream.FlushAsync(cancellationToken).ConfigureAwait(false);" PipeStreamWrapper.cs call "Writer.WaitForPipeDrain()"

I think this Stack Overflow question and answer addresses this exact problem.

Idea for solution: have a closing-handshake. When calling DisconnectAsync or DisposeAsync on PipeClient, do not disconnect but rather send a message to the server to disconnect. And then the server will disconnect.

HavenDV commented 2 years ago

I have released a new version with a suggested fix, please check it out. The ideal situation would be if you could write a test that fails in the version of the library before this fix, so that the situation does not happen again and I do not miss anything.

erezwanderman commented 2 years ago

Thanks! This fixes the issue and now the program works without exceptions.

pontusbredin commented 2 years ago

Hi, I'm evaluating some c# Named Pipes nugets and while testing this one I encountered this "Pipe is broken" problem, perhaps in a little bit different way but none the less.

I started the ConsoleApp sample and used a SysInternals tool called accesschk.exe to check what it reports. Upon doing so I can't catch any exceptions, but the pipe server stops working after a single calling like this

accesschk.exe \pipe\named_pipe_test_server

The tool prints a list of permissions set on the pipe and then very quickly disconnects. Perhaps this tool can be used to find out what happens in H.Pipe, and perhaps prevent the pipeserver from stop serving clients. I find it very useful to check that running pipeservers do have the right permissions. Unfortunately meny named pipe implementations fail to handle this tools rather crude behaviour.

This nuget seams to be a very nice package, thank you!

Yakonrus commented 2 years ago

The problem is computer related. I have a client server application. On the server - 4 PipeServers. I'm running 17 client applications on same laptop, each with 4 PipeClient. On i7 10870H it works more or less. On i5 4210U - permanent IOException: 'Pipe is broken' CPU usage is very high.

HavenDV commented 2 years ago

The problem is computer related. I have a client server application. On the server - 4 PipeServers. I'm running 17 client applications on same laptop, each with 4 PipeClient. On i7 10870H it works more or less. On i5 4210U - permanent IOException: 'Pipe is broken' CPU usage is very high.

Just to be sure - are your clients and servers console applications?

Yakonrus commented 2 years ago

No, .NET 6, WPF And this is a highly loaded application. Therefore, perhaps on a weak computer it becomes noticeable.

HavenDV commented 2 years ago

I will write tests for high-load situations when I have free time. You can speed up this process if you wish. Submitting a PR with a test that will work on both of your computers will greatly speed up the fix for this issue. You can take these tests as a basis, run several copies in parallel, generating names for the pipes: https://github.com/HavenDV/H.Pipes/blob/master/src/tests/H.Pipes.Tests/DataTests.cs

Yakonrus commented 2 years ago

The problem is that I have to isolate the Canon EDSDK dll, the only option was to run a separate client for each camera. I am using modified: https://github.com/acdvorak/named-pipe-wrapper There are problems here too. 0x800705AA insufficient system resource I decided to find another solution. I tried your library.

I'll check later.

Yakonrus commented 2 years ago

Wanted so to follow a lot of connections. But for some reason even one client does not connect. What am I doing wrong?

    [TestMethod]
    public async Task TypeTest()
    {
        using var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromMinutes(1));
        var completionSource = new TaskCompletionSource<bool>(false);
        using var registration = cancellationTokenSource.Token.Register(() => completionSource.TrySetCanceled());

        var formatter = new BinaryFormatter();

        const string pipeName = "data_test_pipe";

        await using var server = new PipeServer<byte[]>(pipeName, formatter);
        server.ExceptionOccurred += (_, args) => Assert.Fail(args.Exception.ToString());
        server.MessageReceived += (_, args) =>
        {
            Console.WriteLine($"MessageReceived: {args.Message[0]}");
            completionSource.TrySetResult(true);
        };

        await server.StartAsync(cancellationTokenSource.Token);

        Task[] tasks = new Task[1];
        for (int j = 0; j < tasks.Length; j++)
        {
            int i = j;
            tasks[i] = Multi(i, pipeName);
        }
        Task.WaitAll(tasks);

        Assert.IsTrue(await completionSource.Task);
    }

    private async Task Multi(int i, string pipeName)
    {
        using var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromMinutes(1));

        var formatter = new BinaryFormatter();

        await using var client = new PipeClient<byte[]>(pipeName, formatter: formatter);

        client.ExceptionOccurred += (_, args) => Assert.Fail(args.Exception.ToString());

        await client.ConnectAsync(cancellationTokenSource.Token);

        byte[] bytes = new byte[1024 * 1024];
        bytes[0] = (byte)(i+1);

        await client.WriteAsync(bytes, cancellationTokenSource.Token);
    }
HavenDV commented 2 years ago

Replace Task.WaitAll(tasks); to await Task.WhenAll(tasks);.