1

I'm creating an asynchronous server that can have multiple clients. Similar to a chat client/server architecture, all clients are updated on each server state change based on any client's request. I've found a lot of examples to follow and wrote a simple application for testing. I've just written the processing of client requests for now but have come across a situation that I normally don't encounter. Here's the sample server I wrote:

class Server
{
    int _port;
    TcpListener _listener;
    IList<TcpClient> _clients = new List<TcpClient>();

    public Server(int port)
    {
        _port = port;
        _listener = new TcpListener(IPAddress.Any, _port);
    }

    public async Task StartListening()
    {
        _listener.Start();
        Console.WriteLine("The server is listening on port {0}...", _port);

        while (true)
        {
            try
            {
                var client = await _listener.AcceptTcpClientAsync();
                Console.WriteLine("We have a client!");
                _clients.Add(client);
                Process(client);

            }
            catch (Exception e)
            {
                Console.WriteLine(e.Message);
            }
        }
    }

    private async Task Process(TcpClient client)
    {
        try
        {
            var stream = client.GetStream();
            var reader = new StreamReader(stream);
            var writer = new StreamWriter(stream) { AutoFlush = true };
            char[] buffer = new char[1024];
            while (true)
            {
                var request = await reader.ReadLineAsync();
                if (request != null)
                {
                    Console.WriteLine(request);
                }
            }
        }
        catch (Exception e)
        {
            Console.WriteLine(e.Message);
            client.Close();
        }
    }

}

Here's Program.cs:

class Program
{
    static void Main(string[] args)
    {
        var server = new Server(6029);
        server.StartListening().Wait();
    }
}

I get a warning on the Process call since the Task isn't awaited. I understand the behavior of the code without the await call but I'm wondering if I should be coding this differently (ThreadPool, etc...) even though this gives me the behavior that I want. Should Tasks always be awaited?

Seapoe
  • 459
  • 4
  • 15
  • 1
    `Should Tasks always be awaited?` No... Fire&Forget tasks also have some usages.. – L.B Oct 03 '16 at 21:28

1 Answers1

0

It's hard to know what you're really asking here. You have asked both the specific question implied by "I'm wondering if I should be coding this differently", as well as the broad, primarily opinion-based question "Should Tasks always be awaited?"

On the latter, the only answer that is conceivably correct is "no." There's practically nothing in programming that always has to be done.

That said, the code you posted certainly seems flawed. For one, you have an async Task method that cannot ever possibly complete. What's the point of that? You might as well declare it async void. The program can just wait indefinitely via some other mechanism, like sleeping for an infinitely long time or blocking on a Console.ReadLine() method or something.

Even better, give the program a way to shut itself down gracefully. Close the listening socket when you want the server to stop listening. Store all the Task objects returned by Process(), and wait on them before allowing the process to complete, to ensure that your server gracefully closes the connections instead of forcefully resetting them.

The code you posted isn't really itself specific enough to provide anything specific in the way of advice. It looks like starter code, used mainly to demonstrate some basic concepts rather than to do anything real. As such, it's necessarily going to be subject to different rules than code that's supposed to work correctly in all situations.


It seems to me that given your code example, I would await at some point the tasks you create. You don't necessarily need to use await Process(...) (indeed, you probably don't want to, since that would prevent you from handling more than one client at a time), but you should keep the reference and wait eventually.

But does that mean a Task must always be awaited? No. It just means in your example, you haven't shown a compelling reason not to. In most cases, you should. If nothing else, it gives you the opportunity to observe any exception that might occur (speaking of which, you should not be catching Exception…catch only those exceptions which you expect and for which you know for sure how to handle). But in rare cases, it can make sense to pay no attention to a task you've started, once it's started, if for no other reason than simply sheer practicality (or rather, the impracticality of trying to observe the task).


Additional reading:
How to safely call an async method in C# without await
warning this call is not awaited, execution of the current method continues
Where to stop using async /await keywords?

Community
  • 1
  • 1
Peter Duniho
  • 68,759
  • 7
  • 102
  • 136