Implementation questions #31
Description
Hi, thanks for the great job. I have noticed several problems
- After approximately 7,000 clients contacted, 4GB of memory disappears in the system.
My solution:
_server.ClientDisconnected += async (o, args) =>
{
await args.Connection.DisposeAsync();
Debug.WriteLine($"Client {args.Connection.PipeName} disconnected");
};
After that the memory ceased to disappear. This helped free up memory.
- After some time, the server stops responding. However, it does not log an error anywhere. To reproduce this quickly, you need to terminate the client in the middle of the connection. Thus the exception is returned from the handshakeWrapper
Then the break stops the while cycle. New clients can no longer connect to the server.
My solution:
replace break; to throw;
Question: How reasonable was the break in this place? Is my fix correct or should this cycle be created per client?
- I looked at this commit 0db0952 here I have doubts about the correct usage System.Memory. I'd rather not fill head with newfangled classes and conditions, instead add a warning to Suppress CA1835 message. And then use a single codebase, like this
{
var buffer = new byte[length];
var read = await BaseStream.ReadAsync(buffer, 0, buffer.Length, cancellationToken).ConfigureAwait(false);
if (read != length)
{
return throwIfReadLessThanLength
? throw new IOException($"Expected {length} bytes but read {read}")
: Array.Empty<byte>();
}
return buffer;
}
this code looks cleaner and easier to maintain. Same with WriteAsync.