3
\$\begingroup\$

I'm mainly wondering about whether or not I have used the (TAP) async/await pattern correctly. I get a bunch of these warnings, "Because this call is not awaited, execution of the current method continues before the call is completed.", however that sounds like exactly what I want. Which is for the server to be able to handle clients independently and asynchronously.

As a side note, I know it's bad for performance to re-initialize the streams in for example HandleConnection, I just kept it that way for brevity's sake.

Client.cs:

using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Threading.Tasks;

class Client {
    TcpClient tcpClient;
    public void Connect() {
        ConnectAsync();
    }

    public async void Ping() {
        await SendMessage("Ping!");
    }

    private async Task ConnectAsync() {
        tcpClient = new TcpClient();
        await tcpClient.ConnectAsync(IPAddress.Parse("127.0.0.1"), 3344);
        MainWindow.ClientLog("Connected to server!");

        HandleConnection(tcpClient);
    }

    private async Task HandleConnection(TcpClient tcpClient) {
        NetworkStream ns = tcpClient.GetStream();
        StreamReader sr = new StreamReader(ns);
        string message = await sr.ReadLineAsync();
        MainWindow.ClientLog("Received message from server: " + message);

        HandleConnection(tcpClient);
    }

    private async Task SendMessage(string message) {
        NetworkStream ns = tcpClient.GetStream();
        StreamWriter sw = new StreamWriter(ns);
        await sw.WriteLineAsync(message);
        await sw.FlushAsync();
        MainWindow.ClientLog("Sent Ping! to server.");
    }
}

Server.cs:

using System.Collections.Generic;
using System.IO;
using System.Net;
using System.Net.Sockets;
using System.Threading.Tasks;

class Server {
    List<TcpClient> connectedClients;
    public void Host() {
        connectedClients = new List<TcpClient>();
        HostAsync();
    }

    private async Task HostAsync() {
        IPEndPoint local = new IPEndPoint(IPAddress.Parse("127.0.0.1"), 3344);
        TcpListener mainListener = new TcpListener(local);

        mainListener.Start();
        MainWindow.ServerLog("Server started.");
        while(true) {
            TcpClient client = await mainListener.AcceptTcpClientAsync();
            MainWindow.ServerLog("Client connected!");

            connectedClients.Add(client);
            HandleClient(client);
        }
    }

    private async Task HandleClient(TcpClient client) {
        NetworkStream ns = client.GetStream();
        StreamReader sr = new StreamReader(ns);
        string message = await sr.ReadLineAsync();
        MainWindow.ServerLog("Message from client nr. " + connectedClients.IndexOf(client) + ": " + message);

        if (message == "Ping!") {
            await SendMessage(client, "Pong!");
        }

        HandleClient(client);
    }

    private async Task SendMessage(TcpClient client, string message) {
        NetworkStream ns = client.GetStream();
        StreamWriter sw = new StreamWriter(ns);
        await sw.WriteLineAsync(message);
        await sw.FlushAsync();
        MainWindow.ServerLog("Sent message to client: " + message);
    }

    private async Task SendMessages(IEnumerable<TcpClient> clients, string message) {
        foreach (TcpClient client in clients) {
            SendMessage(client, message);
        }
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ Does it work as intended or not? Your question seems ambiguous about this. \$\endgroup\$
    – Mast
    Aug 26 '16 at 14:53
  • \$\begingroup\$ As far as I'm able to tell/test, it works as intended. However I suspect this isn't the right way of going about things, and that there are gotchas I have missed. From other posts I've looked at there seems to at least be issues regarding unhandled exceptions with this approach. \$\endgroup\$
    – Dan
    Aug 26 '16 at 14:58
1
\$\begingroup\$

As far as I know, it seems to be only a "warning". Async/Await are supposed to be used as a pair in most cases. Let's have a simple example based on your code :

 HostAsync();

Here you have written a basic method call, and the method is :

private async Task HostAsync()

So the warning appears because compiler is expecting something like :

await ...

inside the HostAsync method.

By not putting the await keyword inside this async method, we get basically a synchronous call, a standard method. Not a big issue in most cases but have a look at :

private async Task SendMessages(IEnumerable<TcpClient> clients, string message)

This one has no await too.

Hope this helps !

\$\endgroup\$
3
  • 1
    \$\begingroup\$ It doesn't seem to turn into a synchronous call at all. For example, if I convert HostAsync to a regular non-async method, the UI thread would freeze while it waits for connections. \$\endgroup\$
    – Dan
    Aug 26 '16 at 20:44
  • 1
    \$\begingroup\$ The intention is to run HostAsync in a different "thread" (task), so that the rest of my program can keep running separately from it. Similarly I want each client to be handled by separate tasks (HandleClient-s). SendMessages doesn't have an await, because I want it to just fire off messages and forget about them, I can't "await" any meaningful result because it could send a message that the client doesn't have to respond to. \$\endgroup\$
    – Dan
    Aug 26 '16 at 20:52
  • \$\begingroup\$ Sorry, I've missed something : I think a better explanation was a call Inside a new task. Now the real difference is linked to how exception are handled with or without the await keyword. \$\endgroup\$
    – thesyndarn
    Aug 26 '16 at 22:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.