Threading and Sockets

Question

I have the following:

   ThreadStart startThread =
     delegate
     {
             mySocket.StartListen();
     };

mySocket is now looping on a Listen() when I:

new Thread(startThread).Start();

Here is StartListen:

public void StartListen()
{
 Object locker = new Object();

 // Lock resources
 lock (locker)
 {
  S = new System.Net.Sockets.Socket(System.Net.Sockets.AddressFamily.InterNetwork,
       System.Net.Sockets.SocketType.Stream,
       System.Net.Sockets.ProtocolType.Tcp);
  S.Blocking = false;

  try
  {
   S.Bind(serverEP);
   S.Listen(1000);
   isListening = true;

   /*
    * While statement not required here because AcceptConnection()
    * method instructs the socket to BeginAccept() again...
    */
   //while (true)
   //{
   connectDone.Reset();
   S.BeginAccept(new AsyncCallback(AcceptConnection), Channel);
   connectDone.WaitOne();
   //}
  }
  catch (System.Net.Sockets.SocketException SockEx)
  {
   Console.WriteLine("*****[" + name + "] " + SockEx.SocketErrorCode + ":");
   Console.WriteLine("*****[" + name + "] " + SockEx.Message);
  }
 }
}

Because of the asynchronous methods things don't really 'finish' and return a signal for anything else to continue. Any commands I implement after the Thread.Start() above are not working properly. For instance, in StartListen, note that I have an isListening = true. After I start the thread, I want to use the property IsListening. It is always returned as false.

How should I be starting the Thread. Would an asynchronous method (i.e. ThreadStart.BeginInvoke()) be preferrable? Isn't that similar to using a ManualResetEvent?

Answer 1

Mark isListening as volatile. As I understand it, you're referencing this flag in two different threads. Normally, data shared between threads must be synchronized, but it doesn't appear you're doing that. However, given that the flag is a bool type, you don't technically have to synchronize access to it because the .NET framework guarantees that reads and writes from bool types are atomic (I think I'm saying that correctly...someone please correct me if that it not technically correct). But, in order to make sure that reads from that type actually get the most recent value, you need to mark it as volatile.

Also, you'd be better off using the TcpListener class instead of trying to do this with the Socket class. What you're doing is not wrong, but the TcpListener class would make your code simpler to read and maintain, IMO.

Finally, the locker object you're using doesn't really do anything since it is local to the StartListen() method. In order for synchronization objects to have any effect, they need to be accessible to the threads that need serialized access to shared data. The way you're doing it here, other threads cannot access the locker variable, thus rendering it useless.