Async callback usage in TCP server application

Metal76

I'm developing an application in which multiple clients exchange data on a TCP link with a server using a custom protocol. I've chosen the asynchronous approach to implement the server. Below I've attached an excerpt of my code, showing the points discussed in my questions. I have several doubts about correct asynchronous callback usage, and I could not find anything in the docs: 1) Suppose you have several clients connected. Do all the callbacks (OnClientConnect, OnClientRead...) "live" in the same thread? That is, while I'm inside OnClientRead to serve one of the clients, can my code be interrupted by an OnClientRead from another client? I performed a simple investigation, and it seems that OnClientRead for different clients are called from the same thread. So, it seems that OnClientRead execution never gets interrupted. Am I right? Is this always true? If so, I think I can safely remove all the lock statements for clientList access. 2) Is it correct to always launch a new AsyncCallback every time? e.g. new AsyncCallback(OnClientRead) inside OnClientRead. Does this waste resources? Should I keep a different AsyncCallback for each client and reuse it? Regards, Andrea

class ClientState
{
// Holds client state: I/O buffer, socket, IP endpoint etc...
}

class NetworkServer
{
// Holds list of client states
private List clientList;
private object lockObject = new object();

// Start server node
public void Start(int backlog)
{
    clientList = new List();

   // Listen to incoming connections asynchronously
    serverSocket.Listen(backlog);
    serverSocket.BeginAccept(new AsyncCallback(OnClientConnect), null);   
}

// Callback on client connection
private void OnClientConnect(IAsyncResult state)
{
    // Accepts incoming connection
    Socket clientSocket = serverSocket.EndAccept(state);

    // Create client state and fill it with state values    
    ClientState clientState = new ClientState();
    ...

    // Add to client list
    lock(lockObject)
    {
        clientList.Add(clientState);
    }
    
    // Start receiving from client
    clientSocket.BeginReceive(clientState.Buffer, 
                              0,

Nicholas Butler

Hi again Andrea. That's looking good.

Metal76 wrote:

Suppose you have several clients connected. Do all the callbacks (OnClientConnect, OnClientRead...) "live" in the same thread? That is, while I'm inside OnClientRead to serve one of the clients, can my code be interrupted by an OnClientRead from another client? I performed a simple investigation, and it seems that OnClientRead for different clients are called from the same thread. So, it seems that OnClientRead execution never gets interrupted. Am I right? Is this always true? If so, I think I can safely remove all the lock statements for clientList access.

The callbacks happen on a thread pool thread. This means that each callback owns it's thread while it is executing and will not be 'interrupted'. However, the thread pool has many threads! Each callback is handled separately, so it may or may not execute on the same thread pool thread each time. If multiple clients are active at the same time, you may have many threads from the pool, each executing OnClientConnect or OnClientRead for a particular client. This means that you do have to protect your shared state ( clientList ) by using a lock, just as you have done.

Metal76 wrote:

Is it correct to always launch a new AsyncCallback every time? e.g. new AsyncCallback(OnClientRead) inside OnClientRead. Does this waste resources? Should I keep a different AsyncCallback for each client and reuse it?

Yes, you have to start a new BeginXXX call each time to continue accepting clients and receiving data. The AsyncCallback is just a delegate, so doesn't waste much. You can declare an instance in your class, instantiate it in a constructor, and reuse it each time. It doesn't really matter, though. Just as a matter of style, I would put the calls to BeginAccept and BeginReceive in a private method each, just so you don't have two copies of each call. Nick

---------------------------------- Be excellent to each other :)

Metal76

Hi Nick! By the way, I also considered the ReceiveAsync pattern you suggested in your last post, but as a newbie I found it more difficult to approach :-) I'll consider it for future enhancements.

Nick Butler wrote:

Just as a matter of style, I would put the calls to BeginAccept and BeginReceive in a private method each, just so you don't have two copies of each call.

Do you mean something like the following?

private void StartReceiving(ClientState clientState)
{
clientSocket.BeginReceive(clientState.Buffer,
0,
clientState.Buffer.Length,
SocketFlags.None,
new AsyncCallback(OnClientRead),
clientState);
}

A couple of additional points (just to increase the amount of beer pints I already owe you): 1) Which is the correct way to gracefully shutdown the server app? I know for sure that I have to close all the client sockets; are there any other resources to shutdown in the async pattern I'm following? 2) I noticed that the ClientState instance I create inside OnClientConnect for each of the clients seems to be internally maintained by the async pattern, without any intervention on my side. That is: when OnClientRead fires from one of the clients, the input IAsyncResult state is the right ClientState instance associated with that client. So, it seems to me that keeping a clientList is only useful when I want to shutdown the server (so that I can dispose of each client socket) and if I want to offer to the users of my class the possibility to perform actions on connected clients (e.g. check state of client A, forcibly shutdown connection with client B etc.). Is this right? Thanks in advance and best regards, Andrea

Nicholas Butler

Metal76 wrote:

Do you mean something like the following?

Yes, that's it.

Metal76 wrote:

Which is the correct way to gracefully shutdown the server app? I know for sure that I have to close all the client sockets; are there any other resources to shutdown in the async pattern I'm following?

If your protocol doesn't have any command messages to send when closing, all you have to do is call Shutdown and then Dispose.

Metal76 wrote:

So, it seems to me that keeping a clientList is only useful when I want to shutdown the server (so that I can dispose of each client socket) and if I want to offer to the users of my class the possibility to perform actions on connected clients (e.g. check state of client A, forcibly shutdown connection with client B etc.). Is this right?

Yes, that's right. Nick

---------------------------------- Be excellent to each other :)

Metal76

Thanks Nick! As usual, very useful info. Bye, Andrea