Threading issue when sending binary arrays to multiple clients.

Jan 26, 2012 at 2:05 PM

First, I have to say that this is an excellent library. I tested it mainly with Microsoft Websockets library from their HTML5labs site. It works fine until when I connected two clients (console apps using the MS library). Both clients sends some binary array as requests. The server basically responses back some other binary arrays. When the server sends many (a few hundred) messages (about a few KB each) concurrently to both clients, some messages were actually send to the wrong client session. Even though the trace showed that the session object and even endpoints are correct. I confirm the problem by using the wireshack to capture all network traffic. To work around the issue, I have to use a lock to eliminate the concurrency of sending messages to more than one client session. Can you please take a look at the issue? Below are some server side code:

This is how I setup the server:

_webSocketServer = new WebSocketServer(new BasicSubProtocol("Basic", new List<Assembly> { this.GetType().Assembly }));
_webSocketServer.Setup(new RootConfig(), new ServerConfig
{
    Port = 8080,
    Ip = "Any",
    MaxConnectionNumber = 100,
    Mode = SocketMode.Sync,
    Name = "SuperWebSocket"
}, SocketServerFactory.Instance);

_webSocketServer.NewSessionConnected += new SessionEventHandler<WebSocketSession>(OnNewSessionConnected);
_webSocketServer.SessionClosed += new SessionEventHandler<WebSocketSession, CloseReason>(OnSessionClosed);
_webSocketServer.NewDataReceived += new SessionEventHandler<WebSocketSession, byte[]>(OnNewDataReceived);
_webSocketServer.NewMessageReceived += new SessionEventHandler<WebSocketSession, string>(OnNewMessageReceived);

This is how I send back the response after receiving the request. Sometimes, I need to send a few responses back after receiving a request.

private void OnNewDataReceived(WebSocketSession session, byte[] e)
{
  if (e == null || e.Length == 0)
    return;
  for(int i = 0; i < 50; i ++)
  {
    // simulate to send a few responses back
    byte[] response = ....
    session.SendResponse(response);
  }
}
Then I connect multiple clients to the same server. The client is a single console app, but it creates two sessions to the websocket server. What happened was that some of the responses pushed back from the server were received by the wrong session on the client side. I used Wireshack to confirm this error. I put some debug code right before session.SendResponse to inspect the RemoteEndPoint and that was correct, but the Wireshack recorded different endpoint (a wrong dst port).

Jan 27, 2012 at 9:54 PM

I have a theory here, believing this might be a problem of just using one ProtocolProcessor for all WebSocketSessions:

When sending binary data, the  DraftHybi10Processor would call "EnqueueSend", passing in the current WebSocketSession and the data to be sent.

That method appends the passed in data to the end of a Queue, and then calls "DequeueSend", passing on the WebSocketSession.

DequeueSend checks, whether the ProcotolProcessor is already sending data from the queue.

If not, it starts calling SocketSession.SendResponse() on the WebSocketSession, until either the something turns wrong with the connection, or the queue is emptied.

The check for an empty queue and the actual dequeue are protected by a lock, that is shared by the call to Enqueue() within EnqueueSend().

Imagine now a scenario, where one thread adds a message for WebSocketSession "A" to the then empty queue, and starts the dequeue-send loop within DequeueSend.

Someanother thread calls EnqueueSend for a different WebSocketSession "B", while DequeueSend is still inside the loop.

If the loop is currently checking the queue or dequeuing an item, the lock will ensure the Enqueue will not interfere.

There is though a slight possibility, that thread 2 will actually get to Enqueue its data after thread 1 called dequeue and exited the lock and before thread 1 reenters the lock and checks the queuelength again.

In that case, DequeueSend shall eventually dequeue the data enqueued by thread 2, and send it to the WebSocketSession it was originally passed (i.e. "A"), and not "B", as would have been the intention. 

Coordinator
Jan 28, 2012 at 6:52 AM

You are correct!

I have uploaded a new version download to fix some bugs including this issue.

Jan 29, 2012 at 3:43 PM

Hey kerry, thank you.

I have slight suspicion though there might be something wrong with the implementation of IWebSocketSession.EnqueueSend(ArraySegment<byte> data) inside WebSocketSession.cs now.

It seems it would enqueue a single ArraySegment multiple times now, depending on it's length:

        void IWebSocketSession.EnqueueSend(ArraySegment<byte> data)
        {
            lock (m_SendingQueue)
            {
                for (var i = 0; i < data.Count; i++)
                {
                    m_SendingQueue.Enqueue(data);
                }
            }

            DequeueSend();
        }

Enjoy your vacation though.

Coordinator
Jan 30, 2012 at 7:52 AM
Edited Jan 30, 2012 at 7:53 AM

I feel very sorry for my mistake.

Because I was in vacation on past days, I didn't take much time to verify this change.

I just come back to work today, developing will go back to normal.