Stack overflow when using the System.Net.Sockets.S

Question

With respect to C# and .NET's System.Net.Sockets.Socket.AcceptAsync method, one would be required to handle a return value of "false" in order to handle the immediately available SocketAsyncEventArgs state from the synchronously processed connection. Microsoft provides examples (found on the System.Net.Sockets.SocketAsyncEventArgs class page) which will cause a stack overflow if there are a large amount of pending connections, which can be exploited on any system that implements their handling model.

Other ideas for getting around this issue are to make a loop that calls the handler method, with the condition being that the value Socket.AcceptAsync returns is equal to false, and to break the loop (to allow deferred processing) if the value is indicating that the operation is being completed asynchronously (true). However, this solution also causes a stack overflow vulnerability because of the fact that the callback associated with the SocketAsyncEventArgs passed to Socket.AcceptAsync has at the end of the method, a call to Socket.AcceptAsync, which also has a loop for immediately available, synchronously accepted, connections.

As you can see, this is a pretty solid problem, and I've yet to find a good solution that does not involve System.Threading.ThreadPool and creating tons of other methods and scheduling processing. As far as I can see, the asynchronous socket model relating to Socket.AcceptAsync requires more than what is demonstrated in the examples on MSDN.

Does anyone have a clean and efficient solution to handling immediately pending connections that are accepted synchronously from Socket.AcceptAsync without going into creating separate threads to handle the connections and without utilizing recursion?

Answer 1

newSocket.Send(Encoding.ASCII.GetBytes("Hello socket!")); 
newSocket.Disconnect(false); 
newSocket.Close(); 

The problem with this snippet above is that this will block your next accept operation.

A better way is like this:

while (true)
{
   if (e.SocketError == SocketError.Success)
   {
      //ReadEventArg object user token
      SocketAsyncEventArgs readEventArgs = m_readWritePool.Pop();
      Socket socket = ((AsyncUserToken)readEventArgs.UserToken).Socket = e.AcceptSocket;

      if (!socket.ReceiveAsync(readEventArgs))
         ThreadPool.QueueUserWorkItem(new WaitCallback(ProcessReceiveEx), readEventArgs); .
    }
    else
    {
       HadleBadAccept(e);
    }

    e.AcceptSocket = null;

    m_maxNumberAcceptedClients.WaitOne();
    if (listenSocket.AcceptAsync(e))
       break;
}

Answer 2

I haven't looked carefully, but it smells like this might be helpful (see the section called "stack dive"):

http://blogs.msdn.com/b/mjm/archive/2005/05/04/414793.aspx

Answer 3

I wouldn't use AcceptAsync, but rather BeginAccept/EndAccept, and implement the common async pattern correctly, that is, checking for CompletedSynchronously to avoid callbacks in the callback thread on operations which completed .

See also AsyncCallBack CompletedSynchronously

---

Edit regarding the requirement to use AcceptAsync:

The MSDN documentation explicitly says that the callback will NOT be invoked for operations which completed synchronously. This is different to the common async pattern where the callback is always invoked.

Returns true if the I/O operation is pending. The SocketAsyncEventArgs.Completed event on the e parameter will be raised upon completion of the operation. Returns false if the I/O operation completed synchronously. The SocketAsyncEventArgs.Completed event on the e parameter will not be raised and the e object passed as a parameter may be examined immediately after the method call returns to retrieve the result of the operation.

I currently don't see how a loop would not solve the stack overflow issue. Maybe you can be more specific on the code that causes the problem?

---

Edit 2: I'm thinking of code like this (only in regard to AcceptAsync, the rest was just to get a working app to try it out with):

static void Main(string[] args) {
    Socket listenSocket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
    listenSocket.Bind(new IPEndPoint(IPAddress.Loopback, 4444));
    listenSocket.Listen(100);
    SocketAsyncEventArgs e = new SocketAsyncEventArgs();
    e.Completed += AcceptCallback;
    if (!listenSocket.AcceptAsync(e)) {
        AcceptCallback(listenSocket, e);
    }
    Console.ReadKey(true);
}

private static void AcceptCallback(object sender, SocketAsyncEventArgs e) {
    Socket listenSocket = (Socket)sender;
    do {
        try {
            Socket newSocket = e.AcceptSocket;
            Debug.Assert(newSocket != null);
            // do your magic here with the new socket
            newSocket.Send(Encoding.ASCII.GetBytes("Hello socket!"));
            newSocket.Disconnect(false);
            newSocket.Close();
        } catch {
            // handle any exceptions here;
        } finally {
            e.AcceptSocket = null; // to enable reuse
        }
    } while (!listenSocket.AcceptAsync(e));
}

Answer 4

I have resolved this problem by simply changing the placement of the loop. Instead of recursively calling the accept handler from within itself, wrapping the code in a do-while loop with the condition being "!Socket.AcceptAsync(args)" prevents a stack overflow.

The reasoning behind this is that you utilize the callback thread for processing the connections which are immediately available, before bothering to asynchronously wait for other connections to come across. It's re-using a pooled thread, effectively.

I appreciate the responses but for some reason none of them clicked with me and didn't really resolve the issue. However, it seems something in there triggered my mind into coming up with that idea. It avoids manually working with the ThreadPool class and doesn't use recursion.

Of course, if someone has a better solution or even an alternative, I'd be happy to hear it.