Search code examples
c#sockets

BeginAccept not calling my function that calls EndAccept


I'm trying to get a console app running, where the server and client can message each other.

I had it working before, but it was with blocking or synchronous calls, like Send() or Receive(). I want to switch over to non-blocking calls because either the client or server has to wait to receive a message to send one and it isn't practical.

So, I started with BeginAccept() to get a feel of how they work, but my BeginAccept() isn't calling back to my AcceptCallback() that has the EndAccept() for a proper connection.

using System;
using System.Net;
using System.Net.Sockets;
using System.Text;

public class Server
{
    public static void Main()
    {
        //Set ipv4 address and socket to variables
        IPAddress ServerAddress = IPAddress.Parse("127.0.0.1");
        int ServerPort = 1234;
        bool connected = false;

        //Setup the new socket with the parameters of addressFamily.interNetwork (tells it to use a ipv4 address)
        //socketType.stream and protocolType.tcp (tells it to have a connection tye of tcp)
        Socket ServerSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
        ServerSocket.Bind(new IPEndPoint(ServerAddress, ServerPort));

        //telling the socket to start listening and to only let 5 connections at one time
        ServerSocket.Listen(5);
        Console.WriteLine("Server is listening for connections");

        while (!connected)
        {
            ServerSocket.BeginAccept(new AsyncCallback(AcceptCallback), ServerSocket);
            
            if (ServerSocket.Connected == true)
            {
                connected = true;
                Console.WriteLine("Client connected!");
            }
        }
        
        while (connected == true)
        {
            if (ServerSocket.Connected == true)
            {
                connected = true;
            }
            else
            {
                connected = false;
            }

            //setup a array called temp to store the clients data, set what you recieve from clientSocket to be put into an int called ClientBytes
            //convert the bytes from clientbytes to string to read it 
            byte[] temp = new byte[1024];
            int ClientBytes = ServerSocket.Receive(temp);

            if (ClientBytes <= 0 || temp == null)
            {
                return;
            }
            else
            {
                string ClientMessage = Encoding.ASCII.GetString(temp, 0, ClientBytes);
                Console.WriteLine("Recieved data from client: " + ClientMessage);
            }

            //Convert the message to bytes, store it in a array then send it down the client socket
            string ServerMessage = Console.ReadLine();
            if (ServerMessage == null || ServerMessage.Length == 0)
            {
                return;
            }
            else
            {
                byte[] temp2 = Encoding.ASCII.GetBytes(ServerMessage);
                ServerSocket.Send(temp2);
            }

            /*ClientSocket.Shutdown(SocketShutdown.Both);
            ClientSocket.Close();*/
        }

        if (Console.KeyAvailable && Console.ReadKey(true).Key == ConsoleKey.F10)
        {
            ServerSocket.Close();
        }
    }

    private static void AcceptCallback(IAsyncResult ar)
    {
        Socket ServerSocket = (Socket)ar.AsyncState;
        Socket Handler = ServerSocket.EndAccept(ar);
    }
}

I expected it to just connect, but it doesn't. I haven't tried much, to be honest, because I thought I had a good understanding of it, but I guess not.


Solution

  • There is a lot wrong with your code:

    • BeginAccept etc is old-hat. Use the new Async functions with await.
    • Use TcpListener and TcpClient rather than raw sockets.
    • Don't bind to 127.0.0.1 unless you only want to listen to localhost requests (and then just use IPAddress.Loopback) otherwise just use IPAddress.Any.
    public static async Task Main()
    {
        int ServerPort = 1234;
        var listener = new TcpListener(IPAddress.Any, ServerPort);
        listener.Start(5);
        Console.WriteLine("Server is listening for connections");
    
        while (true)
        {
            var client = await listener.AcceptTcpClientAsync();
            Console.WriteLine("Client connected!");
            // hand off client so we can carry on listening
            Task.Run(() => HandleClient(client));
        }
    }    
    
    public static async Task HandleClient(TcpClient client)
    {
        using var _ = client;
        using var stream = client.GetStream();
        //setup a array called temp to store the clients data, set what you recieve from clientSocket to be put into an int called ClientBytes
        //convert the bytes from clientbytes to string to read it 
        byte[] temp = new byte[1024];
        do
        {
            int bytesRead = await stream.ReadAsync(temp);
            if (bytesRead == 0)
                return;
    
            string ClientMessage = Encoding.ASCII.GetString(temp, 0, ClientBytes);
            Console.WriteLine("Recieved data from client: " + ClientMessage);
    
            //Convert the message to bytes, store it in a array then send it down the client socket
            string ServerMessage = Console.ReadLine();
            if (ServerMessage is not { Length : > 0 })
                return;
        
            byte[] temp2 = Encoding.ASCII.GetBytes(ServerMessage);
            await stream.WriteAsync(temp2);
        } while (Console.ReadKey(true).Key != ConsoleKey.F10);  // how to end this loop??
    }
    

    Further notes:

    • You need some sort of message framing otherwise you have no guarantee as to how much data comes through on each read. TCP is a continuous stream, and any read/write may be broken up or joined together.
      So you would read until you reach a message endpoint, parse what you have then loop back. Alternatively, each message can be prefixed with a length.
    • Encoding.ASCII is rather limiting.
    • Consider using some kind of CancellationTokenSource, perhaps combined with Console.CancelKeyPress, to shutdown the listener and sockets.