.Net TCP Server

I saw this question "Is this implementation of an Asynchronous TCP/UDP Server correct" and it is very similar to what I want to do but it goes about it in a different way so I am wondering if anyone would mind doing some constructive critism on my method please.

I'm looking for any feedback along the lines of "when x happens your server will fall over because of y" or something like that.

My intention is that the server should log out errors that happen and raise an event when it has data. I want it to be really resilient and just deal with errors where it can and carry on waiting for clients but there is a point where I think that is not possible which is why I have the FatalError event.

So, my server class will have this interface:

public interface IMySocketServer
{
    // Start the server listening and return immediately 
    void Open();

    // Stop the server
    void Close();

    // Raised when data is receieved
    event EventHandler DataReceived;

    // Raised when a fatal error has happened such that the
    // server cannot recover itself
    event EventHandler FatalError;
}

and for completeness here is the event arguments class:

public class SocketDataEventArgs : EventArgs
{
    private readonly byte[] _data;

    public SocketDataEventArgs(byte[] data)
    {
        _data = data;
    }

    public byte[] GetData()
    {
        return _data;
    }
}

My implementation looks like this:

```
public class MySocketServer : IMySocketServer
{

private readonly TcpListener _listener;
private ILogger _logger = NullLogger.Instance;

public event EventHandler DataReceived;

public event EventHandler FatalError;

public MySocketServer(int port) : this(port, 2048)
{
}

public MySocketServer(int port, int inputBufferSize)
{
InputBufferSize = inputBufferSize;
_listener = new TcpListener(IPAddress.Any, port);
}

public ILogger Logger
{
get { return _logger; }

I'd raise the events DataReceived and FatalError on a different thread, to avoid blocking the TCP work. To elaborate this point, I suggest you take a look at Kayak's implementation (it's an implementation of HTTP server, but the core looks quite similar).

Other than that, any magic number, like the 1024 bytes buffer size, should be exposed somehow, or at least be injected in the constructor.

Update

You use a setter to the buffer size without validation. I'd throw an exception if the new value is non-positive.

In your HandleSocket method, you build a byte array in a non-effective technique: you reallocate memory in each iteration. I'd use a List and invoke its AddRange method. After the loop is done - invoke ToArray.

As for the async pattern, it looks non-blocking what so ever, I missed the point of handling the socket after re-invoking BeginAcceptSocket.

StackExchange Code Review Q#8935, answer score: 3