Asynchronous Sockets

I need to implement a TCP client application. The client and the server send messages to each other. I want to make this program scalable enough to handle connections to multiple servers at the same time. It seems like asynchronous sockets is the way to go for this. I'm new to C# so I'm pretty sure I don't know what I'm doing here. I wrote some classes and a simple console program to get started with. Eventually, I want to create a Windows Forms application but I want to start small and simple first. The Client class runs in its own thread. Is this all thread-safe and correctly done? It's a lot of code and I tried to cut out some fat.

Program.cs

using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Text;
    using System.Threading;

    namespace FastEyeClient
    {
        class Program
        {
            static void Main(string[] args)
            {
                Client client = new Client();
                client.ConnectEvent += new ConnectEventHandler(OnConnect);
                client.SetLiveStatusEvent += new SetLiveStatusEventHandler(OnSetLiveStatus);

                client.Connect("hostname", 1987);

                Thread.Sleep(1000);

                client.SetLiveStatus("hostname", true);
            }

            private static void OnConnect(object sender, ConnectEventArgs e)
            {
                Console.WriteLine(e.Message);
            }

            private static void OnSetLiveStatus(object sender, SetLiveStatusEventArgs e)
            {
                Console.WriteLine(e.Message);
            }
        }
    }

Client.cs

```
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Net;
using System.Net.Sockets;
using System.Threading;

namespace FastEyeClient
{
public delegate void ConnectEventHandler(object sender, ConnectEventArgs e);
public delegate void SetLiveStatusEventHandler(object se

Thread safety stuff.

Raising events

This is not thread safe:

if (SetLiveStatusEvent != null)
{
    SetLiveStatusEvent(this, new SetLiveStatusEventArgs(hostname, message));
}

The value of SetLiveStatusEvent could change after the check for null, but before you invoke it. This not an issue in the example code you posted, but would be in a more complex system. Try something like this instead:

var handler = SetLiveStatusEvent;
if (handler != null)
{
    handler(this, new SetLiveStatusEventArgs(hostname, message));
}

When you are writing your event listeners, keep in mind that there is it possibility that they will be invoked after they have been unsubscribed from the event.

Use of Queue class with manual synchronization
I don't see any problems with how you interact with the Queue, but in .Net 4.0 there is the System.Collection.Concurrent.BlockingCollection class that will take care of all of the synchronization logic for you.

Other stuff

Explicit delegate creation

You don't need to create a new instance of a delegate.

socket.BeginConnect(IPs, port, new AsyncCallback(ConnectCallback), socket);
client.ConnectEvent += new ConnectEventHandler(OnConnect);

You can just pass the name of the method.

socket.BeginConnect(IPs, port, ConnectCallback, socket);
client.ConnectEvent += OnConnect;

Empty catch clause

In the ReceiveCallback method and some other places, there are empty catch (Exception) blocks, in production code I would want to at least see the exception logged.

Also, when possible avoid catching Exception in favor of more specific exceptions.

Auto Properties

Assuming you are using C# 4.0, you could use:

public string Hostname
{
   get;
   set;
}

Instead of:

public string Hostname
{
   get
   {
       return m_Hostname;
   }
   set
   {
       m_Hostname = value;
   }
}

StackExchange Code Review Q#2444, answer score: 5