Reusing thread and UdpClient for sending and receiving on same port

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:

• Send and listen on the same port number;

• 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 = (IPEnd

-
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.

-
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().

-
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.

StackExchange Code Review Q#27145, answer score: 5