4

I have written following Tcp Server applictaion. The problem is it is not executing individual clients in parallel. That is if one client is connected, the server doesn't accept connections to other clients. Please help me fix the code:

void Run()
{
    tcpListener.Start();           

    while (true)
    {
        Console.WriteLine("Waiting for a connection...");

        try
        { 
            var client = await tcpListener.AcceptTcpClientAsync();
            await Accept(client);
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }
}

private async Task Accept(TcpClient client)
{
    //get client information 
    string clientEndPoint = GetClientIPAddress(client);            
    Console.WriteLine("Client connected at " + clientEndPoint);
    log.Info("Client connected at " + clientEndPoint);

    await Task.Yield (); 

    try 
    {              
        using (client) 
            using (NetworkStream stream = client.GetStream ()) 
            { 
                byte[] dataReceived = new byte [50];                  
                while (await stream.ReadAsync(dataReceived, 0, dataReceived.Length) != 0) //read input stream                
                {                    
                    //process data here                        
                    await ProcessData(dataReceived);                      
                }                   
            }
    } //end try
    catch (Exception ex) 
    {
        Console.WriteLine(ex.Message);                
        if (client.Connected)
            client.Close();
    }
} //end Accept(TcpClient client)
helb
  • 7,609
  • 8
  • 36
  • 58
AM0
  • 165
  • 2
  • 4
  • 20

1 Answers1

4

The problem is this:

await Accept(client);

You're awaiting the result of Accept, so you're unable to accept new connections (as you're not executing AcceptTcpClientAsync while Accept is "in-flight").

Here is an example of how it can be done properly: https://stackoverflow.com/a/21018042/1768303.

Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
  • You should change it to `async void Accept(client) {...}` and just call it normally without the `await` keyword in the Run method. – Laith Mar 06 '14 at 00:37
  • @Laith, I'm not a big fan of `async void` fire-and-forget approach. I prefer to keep track of the pending tasks, as done in the linked [answer](http://stackoverflow.com/a/21018042/1768303). – noseratio Mar 06 '14 at 00:38
  • you're right. If you want to keep track of all the running tasks, you'd want to keep a reference to them somehow. You're approach is better :) – Laith Mar 06 '14 at 00:42
  • Thank you so much! Yes, taking the await away from Accept call and changing Accept return from Task to void did solve the problem. – AM0 Mar 06 '14 at 00:53
  • 1
    actually based on your comments i added task back to Accept and calling it in this way: var task = Accept(client) – AM0 Mar 06 '14 at 01:01
  • @AM0, the idea of using a `Task` without `await` rather than `async void` there is to facilitate the error handling and keep track of the pending connections (in `List`). – noseratio Mar 06 '14 at 01:04