C# Async Sockets - Code Analysis

Question

My client part is closing after I do a request to server a second time, but without errors, it just goes away:

class Client
{
    static void Main(string[] args)
    {
        try
        {
            Console.Title = "Client";
            AsyncClient client = new AsyncClient(60101);
            client.Connect();
            Console.Read();
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
            Console.Read();
        }
    }
}

public class AsyncClient
{
    private IPAddress ipAddress;
    private int port;

    /// <summary>
    /// Connects to the local IPAddress.
    /// </summary>
    /// <param name="port"></param>
    public AsyncClient(int port)
    {
        this.port = port;
        IPHostEntry ipHostInfo = Dns.GetHostEntry(Dns.GetHostName());
        this.ipAddress = null;
        for (int i = 0; i < ipHostInfo.AddressList.Length; i++)
        {
            if (ipHostInfo.AddressList[i].AddressFamily == AddressFamily.InterNetwork)
            {
                this.ipAddress = ipHostInfo.AddressList[i];
                break;
            }
        }
        if (this.ipAddress == null)
            throw new Exception("No IPv4 address has been found");
    }

    public AsyncClient(string ip, int port)
    {
        this.port = port;
        IPAddress.TryParse(ip, out ipAddress);
    }

    public async void Connect()
    {
        int attempts = 0;
        TcpClient client = new TcpClient();
        while (!client.Connected)
        {
            try
            {
                attempts++;
                client.Connect(this.ipAddress, this.port);
                Console.Clear();
                Console.WriteLine("Connected");
                await Process(client);
            }
            catch (SocketException)
            {
                Console.Clear();
                Console.WriteLine("Connection Attempts: {0}", attempts);
            }
        }
    }

    public async Task Process(TcpClient tcpClient)
    {
        try
        {
            NetworkStream stream = tcpClient.GetStream();
            StreamWriter writer = new StreamWriter(stream);
            StreamReader reader = new StreamReader(stream);
            writer.AutoFlush = true;
            while (true)
            {
                Console.WriteLine("Enter a Request: ");
                await writer.WriteLineAsync(Console.ReadLine());
                string response = await reader.ReadLineAsync();
                if (response != null)
                    Console.WriteLine(response);
                else
                    break;
            }
        }
        catch (Exception)
        {
            //
        }
        finally
        {
            if (!tcpClient.Connected)
            {
                for (int i = 5; i >= 1; i--)
                {
                    Console.WriteLine($"Connection lost, trying to reconnect in {i}");
                    Thread.Sleep(1000);
                }
                Connect();
            }
        }
    }
}

This under is the server side code, it's just for study purpose. I am trying to learn how to work with sockets and after trying with many different ways like "begin" methods, etc, I feel like I've finally found the right way to do it, since with the others I had problems like concurrent access, closing connection, etc, but this time I believe I got it right. Am I wrong or this time it's really all good with my code?

class Server
{
    static void Main(string[] args)
    {
        try
        {
            Console.Title = "Server";
            AsyncServer server = new AsyncServer(60101);
            server.Run();
            Console.Read();
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
            Console.Read();
        }
    }
}

public class AsyncServer
{
    private IPAddress ipAddress;
    private int port;

    public AsyncServer(int port)
    {
        this.port = port;
        IPHostEntry ipHostInfo = Dns.GetHostEntry(Dns.GetHostName());
        this.ipAddress = null;
        for (int i = 0; i < ipHostInfo.AddressList.Length; i++)
        {
            if (ipHostInfo.AddressList[i].AddressFamily == AddressFamily.InterNetwork)
            {
                this.ipAddress = ipHostInfo.AddressList[i];
                break;
            }
        }
        if (this.ipAddress == null)
            throw new Exception("No IPv4 address for server");
    }

    public async void Run()
    {
        TcpListener listener = new TcpListener(this.ipAddress, this.port);
        listener.Start();
        Console.WriteLine($"Server is now online on Port: {this.port}");
        Console.WriteLine("Hit <Enter> to stop the service");
        while (true)
        {
            try
            {
                TcpClient tcpClient = await listener.AcceptTcpClientAsync();
                Process(tcpClient);
            }
            catch (Exception ex)
            {
                Console.WriteLine(ex.Message);
            }
        }
    }

    private async void Process(TcpClient tcpClient)
    {
        string clientEndPoint = tcpClient.Client.RemoteEndPoint.ToString();
        Console.WriteLine($"Received connection request from {clientEndPoint}");
        try
        {
            NetworkStream networkStream = tcpClient.GetStream();
            StreamReader reader = new StreamReader(networkStream);
            StreamWriter writer = new StreamWriter(networkStream);
            writer.AutoFlush = true;
            while (true)
            {
                string request = await reader.ReadLineAsync();
                if (request != null)
                    Handle(request, writer);
                else
                    break;
            }
        }
        catch (Exception)
        {
            //
        }
        finally
        {
            if (tcpClient.Connected)
                tcpClient.Close();
            Console.WriteLine($"{clientEndPoint} has closed the connection, aborting operation");
        }
    }

    private string Response(string request)
    {
        Thread.Sleep(10000); 
        if (request.ToLower() == "get time")
            return DateTime.Now.ToLongTimeString();
        else
            return $"\"{request}\" is a invalid request";
    }

    private async void Handle(string request, StreamWriter writer)
    {
        try
        {
            Console.WriteLine($"Received request: {request}");
            string response = Response(request);
            Console.WriteLine($"Computed response is: {response}");
            await writer.WriteLineAsync(response);
        }
        catch (Exception)
        {
            //
        }
    }
}

Plus, I would like to know, if I want to make it work on my external IP, so ppl from different IPs can use it, what should I change?

Answer 1

My client part is closing after I do a request to server a second time, but without errors, it just goes away:

The reason for this is that your client calls async method client.Connect(), but doesn't (a)wait this method, so execution on the main thread continues to the next line, Console.Read(), which blocks only until you press [ENTER] for the second time (first [ENTER] is consumed by Console.ReadLine() in a Process() method). Then there is nothing for main thread to do and main thread (as well as whole client application) exits.

As a side note, it is good practise to name all async methods such that it's name ends with 'Async', so that caller of such a method is aware of it's async behaviour and doesn't forget to (a)wait the method. So you should rename Connect to ConnectAsync and Process to ProcessAsync.

Solution is to change return type of Connect method to Task, making method awaitable (it is strongly discouraged for async method to return void anyway):

public async Task ConnectAsync()

and add .Wait() in the Main method, which blocks main thread, until ConnectAsync() exits.

client.ConnectAsync().Wait();

In C# 7.1 you could also use async Main instead:

static async Task Main(string[] args)
{
    ...
    await client.ConnectAsync();
    ...
}

Plus, I would like to know, if I want to make it work on my external IP, so ppl from different IPs can use it, what should I change?

Just make sure that if server has more than one IP address, TcpListener listens on the correct one, and enable port or application in the firewall.