1
votes

I've using the examples from MSDN and (mostly) CodeProject to write a socket server. I'm trying to get my head around the thread-safety of the code. All socket events trigger the IO_Completed method which inspects the SAEA for last operation type (send or receive):

void IO_Completed(object sender, SocketAsyncEventArgs e)
{
    // determine which type of operation just completed and call the associated handler
    switch (e.LastOperation)
    {
        case SocketAsyncOperation.Receive:
            ProcessReceive(e);
            break;
        case SocketAsyncOperation.Send:
            ProcessSend(e);
            break;
        default:
            throw new ArgumentException("The last operation completed on the socket was not a receive or send");
    }       
}

Thinking about incoming calls, does ProcessReceive() need to be completely thread-safe as it may be called many times in a short timeframe if there are a lot of clients, or does it block somehow so that it fully completes before the next event calls it again? I am doing more than just bouncing the received message straight back to the client (which is what the examples do).

Even in the examples, ProcessReceive() is quite a long method (see below) and surely must be at risk of corruption from a second thread. By the time I add the code I need to do something sensible (call a WCF service) the chances of the same code being run again must be very high.

What do I need to do to make ProcessReceive() (and the other related methods) generally thread-safe without compromising the performance gained from using SocketAsyncEventArgs?

Example ProcessReceive() method below:

private void ProcessReceive(SocketAsyncEventArgs receiveSendEventArgs)
{
    DataHoldingUserToken receiveSendToken =
                 (DataHoldingUserToken)receiveSendEventArgs.UserToken;

    if (receiveSendEventArgs.SocketError != SocketError.Success)
    {
        receiveSendToken.Reset();
        CloseClientSocket(receiveSendEventArgs);
        return;
    }

    if (receiveSendEventArgs.BytesTransferred == 0)
    {
        receiveSendToken.Reset();
        CloseClientSocket(receiveSendEventArgs);
        return;
    }

    Int32 remainingBytesToProcess = receiveSendEventArgs.BytesTransferred;

    if (receiveSendToken.receivedPrefixBytesDoneCount <
                       this.socketListenerSettings.ReceivePrefixLength)
    {
        remainingBytesToProcess = prefixHandler.HandlePrefix(receiveSendEventArgs,
                  receiveSendToken, remainingBytesToProcess);

        if (remainingBytesToProcess == 0)
        {
            StartReceive(receiveSendEventArgs);
            return;
        }
    }

    bool incomingTcpMessageIsReady = messageHandler
              .HandleMessage(receiveSendEventArgs,
              receiveSendToken, remainingBytesToProcess);

    if (incomingTcpMessageIsReady == true)
    {
        receiveSendToken.theMediator.HandleData(receiveSendToken.theDataHolder);
        receiveSendToken.CreateNewDataHolder();
        receiveSendToken.Reset();
        receiveSendToken.theMediator.PrepareOutgoingData();
        StartSend(receiveSendToken.theMediator.GiveBack());
    }
    else
    {
        receiveSendToken.receiveMessageOffset = receiveSendToken.bufferOffsetReceive;
        receiveSendToken.recPrefixBytesDoneThisOp = 0;
        StartReceive(receiveSendEventArgs);
    }
}
3

3 Answers

1
votes

Just synchronize what needs to be synchronized. The IO_Completed method itself is threadsafe-agnostic and does not need to change.

Assuming that your DataHoldingUserToken (and other variables such as prefixHandler) are not threadsafe, then they'll need to be protected. As far as I can tell, a simple lock should do.

The mental model is this: IO_Completed may be called at any time with different arguments; each of these run on a ThreadPool thread.

0
votes

I recently implemented something like this. It processed messages over tcp connections. I created a single thread that was responsible for accepting incoming connections. That thread would then spawn a new thread to handle each connection. Those threads blocked while waiting on I/O from the network so they didn't eat CPU resources. If your connections don't share anything it doesn't require thread safety.

0
votes

I'd recommend using the Asynchronous Programming Model, basically the clients would call on BeginProcessReceive and pass in a callback, and in the callback execute EndProcessReceive. You can use either tasks or if pre-4.0 call ThreadPool.QueueUserWorkItem. Am guessing here, but it looks like StartReceived or StartSend are blocking methods, that could be executed into their own (thread-pool) thread. Calling a WCF service, as you mentioned, would lend itself to this model.

This model, would allow you to handle a large number of clients, in addition to various other advantages...