Reusing thread and UdpClient for sending and receiving on same port

Question

The working and functional code below is a simplification of a real test application I've written. It acts as a client which asynchronously sends UDP data from a local endpoint and listens for an echo'ed response from a server implemented elsewhere.

The requirements are:

1. Send and listen on the same port number;
2. The port number must be able to change (simulates data coming in from various sources)

The program does what I want, but I have doubts about its correctness. Specifically regarding the class members of listenThread and udpClient which are reused when the port number changes. Are they implemented, closed and cleaned-up correctly.

using System;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace cs_console
{
    class Tester
    {
        public void Send(string message, int localPort)
        {
            PrepareClient(localPort);
            SendBytes(System.Text.Encoding.UTF8.GetBytes(message));
        }

        private void PrepareClient(int localPort)
        {
            if (localEP == null || localEP.Port != localPort)
            {
                localEP = new IPEndPoint(IPAddress.Parse(localHost), localPort);
                listenThread = new Thread(StartListening);
                listenThread.Start();
            }
        }

        private void StartListening()
        {
            udpClient = new UdpClient();
            udpClient.ExclusiveAddressUse = false;
            udpClient.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
            udpClient.Client.Bind(localEP);

            var s = new UdpState(localEP, udpClient);
            currentAsyncResult = udpClient.BeginReceive(new AsyncCallback(ReceiveCallback), s);
        }

        private void ReceiveCallback(IAsyncResult ar)
        {
            if (ar == currentAsyncResult)
            {
                UdpClient c = (UdpClient)((UdpState)(ar.AsyncState)).c;
                IPEndPoint e = (IPEndPoint)((UdpState)(ar.AsyncState)).e;

                Byte[] buffer = c.EndReceive(ar, ref e);
                if (buffer.Length > 0)
                    System.Console.WriteLine(System.Text.Encoding.UTF8.GetString(buffer));

                UdpState s = new UdpState(e, c);
                currentAsyncResult = udpClient.BeginReceive(new AsyncCallback(ReceiveCallback), s);
            }
        }

        private void SendBytes(byte[] messageData)
        {
            var sender = new UdpClient();
            sender.ExclusiveAddressUse = false;
            sender.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
            sender.Client.Bind(localEP);
            sender.Send(messageData, messageData.Length, remoteEP);
            sender.Close();
        }

        private static string localHost = "10.27.21.109";
        private IPEndPoint localEP = null;
        private IPEndPoint remoteEP = new IPEndPoint(IPAddress.Parse(localHost), 11000);
        private Thread listenThread = null;
        private UdpClient udpClient = null;
        private IAsyncResult currentAsyncResult = null;

        private class UdpState : Object
        {
            public UdpState(IPEndPoint e, UdpClient c) { this.e = e; this.c = c; }
            public IPEndPoint e;
            public UdpClient c;
        }
    }

    class Program
    {
        static void Main(string[] args)
        {
            Tester tester = new Tester();
            int localPort = 2532;
            while (true)
            {
                ++localPort;
                var message = Console.ReadLine();
                tester.Send(message, localPort);
            }
        }
    }
}

This is as compact a working example as I could make.

Answer 1

1. I find a better naming convention for private fields on a class is _PascalCase or _camelCase. This makes it easy to see whether you are using a field or a local variable.

2. It is completely unnecessary to create a new Thread to call StartListening. The thread will terminate once udpClient.BeginReceive() has been called as this is async. You might as well call StartListening() directly in PrepareClient().

3. UdpClient is IDisposable and should therefor be disposed when not needed anymore. You do not clean up the existing UdpClient at all.

   The answer to this question indicates that when you call Dispose or Close on a socket then the callback is invoked but will throw an ObjectDisposedException when calling EndReceive . Because you are storing the currentAsyncResult you can probably get around this by changing StartListening() to this:

   private readonly object _clientLock = new object();
   
   private void StartListening()
   {
       lock (_clientLock)
       {
           if (udpClient != null)
           {
               currentAsyncResult = null;
               updClient.Dispose();
           }
           udpClient = new UdpClient();
           udpClient.ExclusiveAddressUse = false;
           udpClient.Client.SetSocketOption(SocketOptionLevel.Socket, SocketOptionName.ReuseAddress, true);
           udpClient.Client.Bind(localEP);
   
           var s = new UdpState(localEP, udpClient);
           currentAsyncResult = udpClient.BeginReceive(new AsyncCallback(ReceiveCallback), s);
       }
   }
   

   Plus you should lock the inner block of your receive callback. This should ensure that you don't kill the client in the middle of receiving.