multithreading - C++ Socket recv mixed messages

Question

Hello I am having a problem with a socket server and client.

The problem is that the messages get mixed up when I send them really fast. When I send them lets say 1 message per second everything runs good, but when I send them 1 message per 40ms they get mixed up.

here is my code for receiving:

    std::string* AteneaClient::readSocket () {

    std::string finalString = std::string("");
    int size = MSG_SIZE;

    bool receiving = true;
    int timesBufferInc=0;
    while (receiving) {
        std::string temporalString;

        //create an empty buffer
        char* RCV_BUFFER = (char*) malloc (size* sizeof(char));
        for(int i=0;i<size;i++){
            RCV_BUFFER[i]=' ';
        }
        RCV_BUFFER[size-1]='';

        int result =  recv(sock,RCV_BUFFER,size-1,NULL);
        if ( result== SOCKET_ERROR ) {
            free(RCV_BUFFER);
            return NULL;
        }
        else if(result<size-1){
            receiving=false;
        }

        temporalString = std::string(RCV_BUFFER);
        finalString+=temporalString;
    }
    return new std::string(finalString);
}

and here is my code for sending:

    int sendThread(void* data){
    SND_THREAD_DATA* parameters =(SND_THREAD_DATA*)data;
    SOCKET* individualSocket = parameters->individualSocket;
    std::string * message = parameters->message;

    char RCV_BUFFER[MSG_SIZE];
    std::string converter;
    std::cout <<"(!)Thread: Iniciando sendThread Individual.."<<std::endl;
    SOCKET default_socket = *individualSocket;

    bool running=true;

    while(running){


        int length=message->length();
        char *cstr = new char[length + 1];
        strcpy(cstr, message->c_str());
        if(::send(*individualSocket,cstr,length + 1,NULL)==SOCKET_ERROR){
            logSendError();
            running=false;
        }
        delete cstr;
        Sleep(SLEEPTIME);
    }

}

and here is the code when I set up the socket:

    void AteneaClient::startUp(){
    int iResult = 0;
    iResult = WSAStartup(MAKEWORD(2, 2), &WinSockData);
    if (iResult != NO_ERROR) {
        wprintf(L"(!)Main:WSAStartup() failed with error: %d
", iResult);
        return;
    }

    ADDR.sin_addr.s_addr= inet_addr(IP);
    ADDR.sin_family = AF_INET;
    ADDR.sin_port = htons(PORT);
    sock = socket(AF_INET,SOCK_STREAM,0);
    running=true;
}

Anyone has any idea why socket messages get mixed up?

Thanks!

---

EDIT:

this is my current receive method with the improvements from Maxim comments:

    std::string* AteneaClient::readSocket () {

    int HEADER_SIZE=4;
    std::string finalString = std::string("");
    int sizeFirstBuffer = HEADER_SIZE*sizeof(char);
    char* RCV_BUFFER=(char*) malloc(sizeFirstBuffer+1);

    //clean new buffer
    for(int i=0;i<HEADER_SIZE;i++){
        RCV_BUFFER[i]=' ';
        }
    RCV_BUFFER[sizeFirstBuffer]='';



    int result =  recv(sock,RCV_BUFFER,sizeFirstBuffer,NULL);
    //cout << "The Size to read is:" <<RCV_BUFFER << endl;


    //now i create a buffer with that size
    int sizeThatIHaveToRead= atoi(RCV_BUFFER);
    int sizeSecondBuffer = sizeThatIHaveToRead*sizeof(char);
    char* RCV_BUFFER_SECOND=(char*) malloc(sizeSecondBuffer+1);

    //clean new buffer
    for(int i=0;i<sizeSecondBuffer;i++){
        RCV_BUFFER_SECOND[i]=' ';
        }
    RCV_BUFFER_SECOND[sizeSecondBuffer]='';



    result =  recv(sock,RCV_BUFFER_SECOND,sizeSecondBuffer,NULL);
    //cout << "RCV_BUFFER_SECOND:" <<RCV_BUFFER_SECOND << endl;
    finalString+=RCV_BUFFER_SECOND;
    return new std::string(finalString);
}

Answer 1

You are sending strings through stream sockets and expect them to be sent and received atomically, e.g. either nothing is sent/received or the entire string is sent/received. This is not how stream sockets work.

Stream sockets often send only part of your data, so you need to keep sending until all data has been sent. Same is for receiving.

You also need to delimit the messages somehow, otherwise when receiving you won't know when a message ends and the next one starts. The two most common ways are a) prefix messages with their size, b) use a message delimiter (e.g. new-line symbol).

ZeroMQ can do both of these tasks for you: your applications end up sending and receiving complete messages, without you having to implement message framing and sending/receiving on byte level.

---

The updated code still does not correctly use send and recv calls.

Here is correct usage with functions to send and receive a std::string:

#include <stdexcept>
#include <stdint.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <arpa/inet.h>

ssize_t recv_all(int fd, void* buf, size_t buf_len) {
    for(size_t len = buf_len; len;) {
        ssize_t r = ::recv(fd, buf, len, 0);
        if(r <= 0)
            return r;
        buf = static_cast<char*>(buf) + r;
        len -= r;
    }
    return buf_len;
}

ssize_t send_all(int fd, void const* buf, size_t buf_len) {
    for(size_t len = buf_len; len;) {
        ssize_t r = ::send(fd, buf, len, 0);
        if(r <= 0)
            return r;
        buf = static_cast<char const*>(buf) + r;
        len -= r;
    }
    return buf_len;
}

void send_string(int fd, std::string const& msg) {
    ssize_t r;
    // Send message length.
    uint32_t len = msg.size();
    len = htonl(len); // In network byte order.
    if((r = send_all(fd, &len, sizeof len)) < 0)
        throw std::runtime_error("send_all 1");
    // Send the message.
    if((r = send_all(fd, msg.data(), msg.size())) < 0)
        throw std::runtime_error("send_all 2");
}

std::string recv_string(int fd) {
    ssize_t r;
    // Receive message length in network byte order.
    uint32_t len;
    if((r = recv_all(fd, &len, sizeof len)) <= 0)
        throw std::runtime_error("recv_all 1");
    len = ntohl(len);
    // Receive the message.
    std::string msg(len, '');
    if(len && (r = recv_all(fd, &msg[0], len)) <= 0)
        throw std::runtime_error("recv_all 2");
    return msg;
}