Closing sockets in asynchronous TCP server

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: i.e. my application listens for incoming connection with BeginAccept, and accepts them with EndAccept, receives data with BeginReceive/EndReceive and so on. I also keep a list of connected client state information, so that the users of my class can read several info about the clients (IP address, connection state etc.) Now, I'm implementing several methods to forcibly disconnect a client, and to shutdown the whole server (disconnect all the clients and release server resources). Unfortunately, simply closeing the clients + server sockets does not work because the pending operations (EndAccept etc.) fail with various kind of exceptions (NullReference, ObjectDisposed etc.) Is there a correct way to forcibly shutdown connections? Regards, Andrea

Nicholas Butler

Hi Andrea, I was hoping that Shutdown would be thread-safe. Obviously not, so you'll have to implement some control synchronization yourself. You can do this with a ReaderWriterLock - or preferably ReaderWriterLockSlim, if you are targeting .NET 3.5. Something like this:

// in your NetworkServer class
private volatile bool quit = false;
private ReaderWriterLock rwl = new ReaderWriterLock();

// in OnClientConnect
Socket clientSocket = null;
rwl.AcquireReaderLock(-1);// -1 == Timeout.Infinite
try
{
if ( quit ) return;
clientSocket = serverSocket.EndAccept(state);
}
finally
{
rwl.ReleaseReaderLock();
}

// same ( reader lock ) in OnClientRead - wrap the call to EndReceive

// in Shutdown
rwl.AcquireWriterLock(-1);
try
{
quit = true;

foreach ( var client in clientList )
{
client.Socket.Shutdown();
client.Socket.Dispose();
}
}
finally
{
rwl.ReleaseWriterLock();
}

Does that make sense? Nick

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

Metal76

As usual you save my day Nick! I'm currently refactoring my server code for better maintainability, as soon as possible I'll try your suggestion and let you know. Thanks! Andrea

Nicholas Butler

You're welcome.

Metal76 wrote:

and let you know

Please do - this is quite interesting :) Nick

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

Metal76

Hi Nick, before trying your suggestion, I was wondering: does using some kind of lock around socket I/O operations actually interfere with data exchange? That is: as well as BeginReceive/EndReceive, in my server I also transmit data with BeginSend/EndSend. Suppose I acquire a lock over the socket while reading, and at the same time I have a trasmission going on over the same socket. What happens? This is a situation I'm going to face in my code, which uses bidirectional full-duplex communications. Regards, Andrea

Nicholas Butler

Hi Andrea, I suggested using a reader/writer lock for this reason: in normal operation, the lock is acquired in reader mode. As you can have as many readers as you like at the same time, this does not effect normal operation. Your problem was that calling Shutdown asynchronously while an EndReceive ( or EndSend ) was executing was throwing nasty exceptions. If you wrap the call to Shutdown in a writer acquire, it is guaranteed that it owns the lock in exclusive mode. This means that the EndXXX methods cannot be executing at the same time as Shutdown, which I think should solve your problem. So, wrap all 'normal' calls on the Socket in reader acquires, and wrap the call to Shutdown in a writer acquire. Actually, now I think about it, you will probably have to wrap the calls to BeginXXX in reader acquires as well. Nick

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

Mark Salsbery

I chose a simpler approach than locks. I keep a queuedIO count incremented for every "Beginxxx" call. In the async callback delegates, the queuedIO count is decremented. When the listener/socket is shutdown, all the pending IO completions fail (as they should) throwing an exception on the "Endxxxx" calls in the async callback delegate. I catch the expected exception and do nothing in the exception handler. queuedIO count is still decremented. The shutdown code waits for the queuedIO count to reach 0 before proceeeding. The only lock required is on the queuedIO count variable, for which I use Interlocked increments and decrements to insure atomic access. Thread.VolatileRead is used to read the count while waiting for it to reach 0. Works good for me :) Mark

Mark Salsbery Microsoft MVP - Visual C++ :java:

Metal76

Hi Mark, at the moment my code implements a similar (but even simpler) approach: the EndXxx calls are wrapped in a try...catch block, only catching the SocketException and ObjectDisposed exception which seem to be the ones which get "normally" fired when I force the shutdown on the socket. Something like:

try
{
...
// Starts receiving data
_Socket.BeginReceive(_ReceiveBuffer,
0,
_ReceiveBuffer.Length,
SocketFlags.None,
new AsyncCallback(ClientReceiveCallback),
null);
...

}
catch (SocketException sockExc)
{
// Process connection errors (e.g. connection closed by peer...)
// by closing the socket
SocketClose((SocketError)sockExc.ErrorCode);
}
catch (ObjectDisposedException objDispExc)
{
// Could fire during forced shutdown because
// socket is disposed while reception is pending
// No need to close the socket here
}

Do you think it could also be a valid approach? I feel a little uneasy to use exceptions to implement normal programming logic, but I could not find a "clean" way to forcibly shutdown sockets, even searching in CodeProject articles! Regards, Andrea

Mark Salsbery

Metal76 wrote:

I feel a little uneasy to use exceptions to implement normal programming logic, but I could not find a "clean" way to forcibly shutdown sockets

Hi Andrea, I'd love to find a cleaner method, but I haven't, which is why I do it that way :) I do include comments in my callback delegate code to remind me that certain exceptions are expected during shutdown. My server code was ported from C++, where I used an IO completion port. The same problem existed there, except the function used by the completion handler threads to wait for a completed IO (GetQueuedCompletionStatus) would indicate an error instead of throwing an exception. I still used the queuedIOcount there, because I wanted to wait for all pending IOs to be flushed from the completion port at shutdown before continuing. I'm pretty sure .NET's asynchronous IO uses IO completion ports as well, or the equivalent. The difference is, .NET handles the pool of threads for you where in straight Win32, one has to handle that manually. From everything I've seen, they chose to implement the shutdown as an exception instead of using an error code. I guess an error code could have been added to the IAsyncResult, but it wouldn't have been generic - IAsyncResults are used for more than just socket accepts and receives. Instead, the specific "Endxxxx" method documentation specifies the exception that will be thrown in error conditions. For example, Socket.EndReceive() states "ObjectDisposedException...The Socket has been closed". I don't see an alternative. The whole idea of asynchronous IO is "fire-and-forget". Once you call "Beginxxxx", the IO request is out of your hands until completion. They chose to implement exceptions instead of error codes - you don't get anything else to work with AFAIK :) Mark

Mark Salsbery Microsoft MVP - Visual C++ :java: