0

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]='\0';

        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\n", 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]='\0';



    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]='\0';



    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);
}
j.doe
  • 662
  • 4
  • 19
chelo_c
  • 1,739
  • 3
  • 21
  • 34
  • Is the socket a TCP (`SOCK_STREAM`) or UDP (`SOCK_DGRAM`) socket? Is it a blocking or non-blocking socket? – Adam Rosenfield Nov 27 '13 at 16:17
  • it is a TCP socket. SOCK_STREAM – chelo_c Nov 27 '13 at 16:33
  • You are not using the return value of recv which is a common error. – usr Nov 27 '13 at 16:37
  • what do you mean @usr ? – chelo_c Nov 27 '13 at 16:39
  • 2
    The return value signifies the exact number of bytes received. http://linux.die.net/man/2/recv Without using that number you can't know how many bytes you got. What if you just got the first byte? – usr Nov 27 '13 at 16:40
  • I have added the code where I init the socket.@AdamRosenfield @ChrisDesjardins – chelo_c Nov 27 '13 at 16:41
  • 1
    I am not exactly sure what the definition of "mixed up" is, but to do message based I/O on TCP (as opposed to stream based I/O) you need to frame the messages, here is a very simple example of how to do it: http://blogs.msdn.com/b/joncole/archive/2006/03/20/555721.aspx – Chris Desjardins Nov 27 '13 at 16:54
  • 1
    Consider a [`std::vector` recv buffer](http://stackoverflow.com/q/4223711/132382) rather than a manually managed buffer. – pilcrow Nov 27 '13 at 16:55
  • @Chris link's broken. :-( – Harith Jan 29 '23 at 12:24

1 Answers1

6

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, '\0');
    if(len && (r = recv_all(fd, &msg[0], len)) <= 0)
        throw std::runtime_error("recv_all 2");
    return msg;
}
Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
  • i will try sending a prefix message as you say and get back to you. Thanks @Maxim Yegorushkin – chelo_c Nov 27 '13 at 17:19
  • @chelo_c Another option for you is to try using [zeromq](http://zeromq.org/). It frames messages for you over stream sockets and more. – Maxim Egorushkin Nov 27 '13 at 19:01
  • your solution seemed to fix it. The messages arrive very good now. But as the buffer increases the recv method reads a larger buffer and takes longer, so it starts delaying the program. is that normal? – chelo_c Nov 27 '13 at 19:14
  • @chelo_c Probably not. Please show the new code of sending/receiving. – Maxim Egorushkin Nov 27 '13 at 19:21
  • I have pasted my new code in the question, it doesnt lose messages and they dont get mixed up. But as buffer gets larger it starts to delay. – chelo_c Nov 27 '13 at 19:45