how to handle packets in multi-threaded server client program?

I currently have a client app that works but it is single threaded.

my packets look like this: < len_of_data>|< data>"

"|" is used as a separator for my data.

< len_of_data> is always 4 digits long followed.

< data> looks like: |< transaction id>|< command>|< buflen>|< buf>|< checksum>|

my code to create the packets is:

_snprintf_s(data_buffer, WS_MAX_DATA_PACKET_SIZE, 
               WS_MAX_DATA_PACKET_SIZE - 1,
               "%s%d%s%d%s%d%s%s%s%d%s", 
               WS_PACKET_SEP, pkt->transaction_id, 
 开发者_如何学Go              WS_PACKET_SEP, pkt->command, 
               WS_PACKET_SEP, pkt->bufsize, 
               WS_PACKET_SEP, pkt->buf, 
               WS_PACKET_SEP, pkt->checksum, WS_PACKET_SEP);

buf_len = strlen(data_buffer);

_snprintf_s(send_buffer, WS_MAX_DATA_PACKET_SIZE, 
            WS_MAX_DATA_PACKET_SIZE - 1, "%04d%s%s", 
            buf_len, WS_PACKET_SEP, data_buffer);

buf_len = strlen(send_buffer);
// Send buffer
bytes_sent = send(ConnectSocket, send_buffer, buf_len, 0);

The client thread sends a command to the server, then calls a GetIncomingPackets() function. In GetIncomingPackets(), I call recv() to get 5 bytes, this should be the len of the rest of packet, I parse these 5 bytes and verify that they match my expected format. Then I convert the first 4 bytes to an integer, x. Then I call recv() again to get x bytes more and then parse those out into my packet structure.

The problem happens when I add another thread to do the same thing (send and receive commands). I start my app and fire 2 threads and send them to send different commands and wait for responses. When the threads call GetIncomingPackets(), the data I am getting back is invalid. The first 5 bytes I am expecting are missing sometimes, and I just get the following 5 bytes, therefore I am unable to get my < len_of_data > packet.

I even added a critical section block between the 2 recv() calls in my GetIncomingPackets() so the treads dont interrupt each other while getting a full packet. Without some extra code for error checking, this how the function looks like

#define WS_SIZE_OF_LEN_PACKET 5
bool GetIncomingPackets(SOCKET sd, dev_sim_packet_t *pkt )
{
    char len_str_buf[WS_SIZE_OF_LEN_PACKET + 1] = {0};      // + 1 for NULL char
    char data_buf[WS_MAX_DATA_PACKET_SIZE + 1] = {0};
    int ret = 0;
    int data_len = 0;

    EnterCriticalSection( &recv_critical_section );
    nReadBytes = WS_RecvAll(sd, len_str_buf, WS_SIZE_OF_LEN_PACKET );
    ret = WS_VerifyLenPacket(len_str_buf);
    // Convert data packet lenght string received to int 
    data_len = WS_ConvertNumberFromString(len_str_buf, WS_SIZE_OF_LEN_PACKET );   
    // Get data from packet
    nReadBytes = WS_RecvAll(sd, data_buf, data_len);
    LeaveCriticalSection( &recv_critical_section  );
    ret = ParseMessager(data_buf, data_len, pkt);
}

My question is, what could be causing this problem, and how could I fix it? Or is there better ways to do what i am trying to do. The reason that I'm trying to make it multi-threaded is because my app will communicate with 2 other sources, and I want to have a thread to handle each request that comes in from either source.

thanks in advance and feel free to ask any questions if I didn't explain something well.

Here's the code for WS_RecvAll(). The buffer is a static buffer declared in GetIncomingPackets() like this:

  char data_buf[WS_MAX_DATA_PACKET_SIZE + 1] = {0};   // + 1 for NULL char


int WS_RecvAll(SOCKET socket_handle, char* buffer, int size)
{
    int ret = 0;
    int read = 0;
    int i = 0;
    char err_buf[100] = {0};
    while(size)
    {
        ret = recv(socket_handle, &buffer[read], size, 0);
        if (ret == SOCKET_ERROR)
        {
            printf("***ERROR***: recv failed, error = %d\n", WSAGetLastError());
            return WS_ERROR_RECV_FAILED;
        }
        if (ret == 0) {
            break;
        }
        read += ret;
        size -= ret;
     }
     return read;
 }

It's very difficult to debug MT problems, particularly at one remove, but if you are using astatic buffer, should not:

 LeaveCriticalSection( &recv_critical_section  );
 ret = ParseMessager(data_buf, data_len, pkt);

be:

 ret = ParseMessager(data_buf, data_len, pkt);
 LeaveCriticalSection( &recv_critical_section  );

And why use a static buffer in any case?

Im curious to know whether you have used the same socked descriptor in both the threads to connect to the server.