CODEWITHSUNDEEP
c#socketsnetwork-programmingasynchronoustcp
Assassinbeast
Assassinbeast

Reputation: 1277

Using async in async function?

I'm making a simple server that listens for clients which is going to read the clients requests, do some calculations, send a response back to the client and the close again ASAP (somewhat similar to HTTP).

There might be many connections every seconds, so I want it to make it as fast and efficient as possible.

So far, the best way I can think of doing this, is shown as an example below:

private static ManualResetEvent gate = new ManualResetEvent(false);

static async void ListenToClient(TcpListener listener)
{
    Console.WriteLine("Waiting for connection");
    TcpClient client = await listener.AcceptTcpClientAsync();
    Console.WriteLine("Connection accepted & establised");
    gate.Set(); //Unblocks the mainthread

    Stream stream = client.GetStream();
    byte[] requestBuffer = new byte[1024];
    int size = await stream.ReadAsync(requestBuffer, 0, requestBuffer.Length);

    //PSEUDO CODE: Do some calculations

    byte[] responseBuffer = Encoding.ASCII.GetBytes("Ok");
    await stream.WriteAsync(responseBuffer, 0, responseBuffer.Length);

    stream.Close();
    client.Close();
}
static void Main(string[] args)
{
    TcpListener listener = new TcpListener(IPAddress.Any, 8888);
    listener.Start();
    while (true)
    {
        gate.Reset(); 
        ListenToClient(listener);
        gate.WaitOne(); //Blocks the main thread and waits until the gate.Set() is called
    }
}

Note: for this example and simplicity, I haven't made any error handling like try-catch and I know the response here will always be "Ok"

The code here is simply waiting for a connection, when it arrives to await listener.AcceptTcpClientAsync(), then it jumps back to the whileloop and waits until a connection is made and gate.Set() is called so it can listen for new connections again. So this will allow multiple clients at the same time (Especially if the calculations can take long time)

But should I use stream.ReadAsync() or stream.Read() instead? Im curious if it even matters because I'm already in an asynchronous function which will not block the main thread.

So my final questions are:

  1. Is this the best/right way to accomplish this task (also by using ManualResetEvent class)?
  2. Would there be any difference in this scenario to use async or non async operations when reading and writing to the stream? (because I'm not blocking the mainthread)
  3. If it lags, and takes 1-2 seconds to send/receive the data, would it still matter to choose between async and nonasync operations?

UPDATE FOR NEW IMPROVEMENTS

Due to an answer, I have updated my code to this:

private static ManualResetEvent gate = new ManualResetEvent(false);

static async Task ListenToClient(TcpListener listener)
{
    //Same Code
}
static void Main(string[] args)
{
    TcpListener listener = new TcpListener(IPAddress.Any, 8888);
    listener.Start();
    while (true)
    {
        gate.Reset();
        Task task = ListenToClient(listener);
        task.ContinueWith((Task paramTask) =>
            {
                //Inspect the paramTask
            });
        gate.WaitOne(); //Blocks the main thread and waits until the gate.Set() is called
    }
}

Upvotes: 3

Views: 266

Answers (2)

usr
usr

Reputation: 171178

Is this the best/right way to accomplish this task (also by using ManualResetEvent class)?

No. You start an async operation, then immediately wait for it. For some reason I often see this crazy dance. Just make it synchronous:

while (true) {
 var clientSocket = Accept();
 ProcessClientAsync(clientSocket);
}

So simple.

Would there be any difference in this scenario to use async or non async operations when reading and writing to the stream?

Using async IO with sockets is good if you have very many clients. For a few dozens at a time you can just use sync IO with threads. Async IO is about not blocking threads (which each use 1MB of stack space).

If you decide to use async IO, ProcessClientAsync should be an async function like you have it now.

If you decide for sync IO, start ProcessClientAsync on a new thread to be able to process multiple clients simultaneously.

If it lags, and takes 1-2 seconds to send/receive the data, would it still matter to choose between async and nonasync operations?

As long as you process individual clients independently, you are fine. The choice between sync and async only comes into play at high scale (more than dozens of connections open at the same time).

It is a common mistake to overcomplicate things by going async without the need for it. Basically all tutorials make this mistake.

Upvotes: 2

David
David

Reputation: 218827

Right off the bat I see two common async mistakes:

async void

Don't do this. The only reason the compiler even supports async void is for handling existing event-driven interfaces. This isn't one of those, so here it's an anti-pattern. async void effectively results in losing any way of ever responding to that task or doing anything with it, such as handling an error.

Speaking of responding to tasks...

ListenToClient(listener);

You're spawning a task, but never examining its state. What will you do if there's an exception within that task? It's not caught anyway, it'll just be silently ignored. At the very least you should provide a top-level callback for the task once it's completed. Even something as simple as this:

ListenToClient(listener).ContinueWith(t =>
{
    // t is the task.  Examine it for errors, cancelations, etc.
    // Respond to error conditions here.
});

Upvotes: 3

Related Questions