Search code examples
csocketstcpsendrecv

Proper way of breaking a TCP stream into messages (in C )


I'm working on a C project that uses TCP to exchange data between a client and a server. Thanks to the comment to my previous question, and looking at answers to previous questions, I realized that a single send operation may need multiple recv in order to completely send a message, so I wrote the following code.

Sending

    void send_message(int clientSocket, char message [], int length ){
        int sent = 0;
        while(sent < length){
            sent += send(clientSocket, message + sent , length - sent , 0);
        }
    }

Receiving

    void receive_message(int clientSocket, char message [], int length){
        int received = 0;
        while(received < length){
        received += recv(clientSocket, message + received , length - received , 0);
        }
    }

If i'm just trying to send a single message, this works without problems. However, when I try to send multiple messages one after the other, the sending side manages to complete the operations and exit the while loops (which should mean that all the bytes have been sent); but the receiving side doesn't, and it gets stuck when one of the recv keeps reading 0 bytes from the socket. Here's my code.

Client

#include<stdlib.h>
#include<string.h>
#include<inttypes.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include<sys/socket.h>

void send_message(int clientSocket, char message [], int length ){
    int sent = 0;
    while(sent < length){
        sent += send(clientSocket, message + sent , length - sent , 0);
    }
}
void receive_message(int clientSocket, char message [], int length){
    int received = 0;
    while(received < length){
        received += recv(clientSocket, message + received , length - received , 0);
    }
}
int main(){
    //SETTING UP THE SOCKET
    struct sockaddr_in serverAddr;
    socklen_t addr_size;
    int clientSocket = socket(AF_INET, SOCK_STREAM, 0);
    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(7899);
    //REPLACE  SERVER_IP_ADDRESS WITH YOUR ADDRESS
    inet_pton(AF_INET,"SERVER_IP_ADDRESS9999",&serverAddr.sin_addr);
    memset(serverAddr.sin_zero, '\0', sizeof serverAddr.sin_zero);
    addr_size = sizeof serverAddr;
    connect(clientSocket, (struct sockaddr *) &serverAddr, addr_size);

    // Testing
    char message [3000]= "";
    memset(message, 0,3000);
    for(int i = 0 ; i< 20; i ++){
        //Filling the string
        for(int j=0; j<2999; j++){
            message[j] = 'A' + i;
        }
        send_message(clientSocket, message, 3000);
        printf("i = %d \n %s\n",i, message);
    }
}

Server

#include <stdio.h>
#include<stdlib.h>
#include<string.h>
#include<inttypes.h>
#include <netinet/in.h>
#include<sys/socket.h>

void send_message(int clientSocket, char message [], int length ){
    int sent = 0;
    while(sent < length){
        sent += send(clientSocket, message + sent , length - sent , 0);
    }
}
void receive_message(int clientSocket, char message [], int length){
    int received = 0;
    while(received < length){
        received += recv(clientSocket, message + received , length - received , 0);
    }
}
int main(){
    //SETTING UP THE SOCKET
    int welcomeSocket, clientSocket;
    struct sockaddr_in serverAddr;
    struct sockaddr_storage serverStorage;
    socklen_t addr_size;
    welcomeSocket = socket(AF_INET, SOCK_STREAM, 0);
    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(7899);
    serverAddr.sin_addr.s_addr = INADDR_ANY;
    memset(serverAddr.sin_zero, '\0', sizeof serverAddr.sin_zero);
    bind(welcomeSocket, (struct sockaddr *) &serverAddr, sizeof(serverAddr));
    if (listen(welcomeSocket,1)==0)
        printf("Listening\n");
    else
        printf("Error\n");
    addr_size = sizeof serverStorage;
    
    //Accettiamo la connessione sul socket per il Client in arrivo
    clientSocket = accept(welcomeSocket, (struct sockaddr *) &serverStorage, &addr_size);

    printf("accepted");

    // Testing
    char message [3000]= "";
    memset(message, 0,3000);
    for(int i = 0 ; i< 20; i ++){
        receive_message(clientSocket, message, 3000);
        printf("i:= %d\n %s\n",i, message);
    }
} 

Note: if I tell the sender to wait a few moments between each message, the the program works without a problem: what could the reason be? Am I over loading the socket by sending too many bytes at the same time?

EDIT: Included code.

Note 2: From my tests, the code works if ran locally, with client and server on the same machine. It breaks however when i try to run it on a remote server. EDIT 2: To see what i mean about "waiting": if in the sender I include a line that prints the message, then there are no problems. If instead I don't, then the server has the problems that i described above and in the comments.

Edit 3: Solution

Thanks to Serge Ballesta's answer i implemented this solution

Server side

#include <stdio.h>
#include<stdlib.h>
#include<string.h>
#include<inttypes.h>
#include <netinet/in.h>
#include<sys/socket.h>

void receive_message(int clientSocket, char message [], int length){
    int received = 0; int r =0;
    while(received < length){
        r = recv(clientSocket, message + received , length - received , 0);
        if( r<= 0){
            printf("!\n\nERROR- skip \n\n\n!!!!!!!!");
            break;
        }else{
            received += r;
        }
    }
}
int main(){
    //SETTING UP THE SOCKET
    int welcomeSocket, clientSocket;
    struct sockaddr_in serverAddr;
    struct sockaddr_storage serverStorage;
    socklen_t addr_size;
    welcomeSocket = socket(AF_INET, SOCK_STREAM, 0);
    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(7899);
    serverAddr.sin_addr.s_addr = INADDR_ANY;
    memset(serverAddr.sin_zero, '\0', sizeof serverAddr.sin_zero);
    bind(welcomeSocket, (struct sockaddr *) &serverAddr, sizeof(serverAddr));
    if (listen(welcomeSocket,1)==0)
        printf("Listening\n");
    else
        printf("Error\n");
    addr_size = sizeof serverStorage;
    
    //Accettiamo la connessione sul socket per il Client in arrivo
    clientSocket = accept(welcomeSocket, (struct sockaddr *) &serverStorage, &addr_size);

    printf("accepted");

    // Testing
    char message [3000]= "";
    memset(message, 0,3000);
    for(int i = 0 ; i< 30; i ++){
        receive_message(clientSocket, message, 3000);
        printf("i:= %d\n %s\n",i, message);
    }
}

Client Side

#include <stdio.h>
#include<stdlib.h>
#include<string.h>
#include<inttypes.h>
#include <arpa/inet.h>
#include <netinet/in.h>
#include<sys/socket.h>

void send_message(int clientSocket, char message [], int length ){
    int sent = 0;
    while(sent < length){
        sent += send(clientSocket, message + sent , length - sent , 0);
    }
}

int main(){
    //SETTING UP THE SOCKET
    struct sockaddr_in serverAddr;
    socklen_t addr_size;
    int clientSocket = socket(AF_INET, SOCK_STREAM, 0);
    serverAddr.sin_family = AF_INET;
    serverAddr.sin_port = htons(7899);
    //REPLACE  SERVER_IP_ADDRESS WITH YOUR ADDRESS
    inet_pton(AF_INET,"15.161.236.166",&serverAddr.sin_addr);
    memset(serverAddr.sin_zero, '\0', sizeof serverAddr.sin_zero);
    addr_size = sizeof serverAddr;
    connect(clientSocket, (struct sockaddr *) &serverAddr, addr_size);

    // Testing
    printf("Start\n");
    char message [3000]= "";
    memset(message, 0,3000);
    for(int i = 0 ; i< 30; i ++){
        //Filling the string
        for(int j=0; j<2999; j++){
            message[j] = 'A' + (i % 26);//(rand() %20) ;
        }
        message[4] = 0;
        send_message(clientSocket, message, 3000);
    }
    shutdown(clientSocket, 2);
}


Solution

  • You have 2 problems in your code:

    • the sender (client prog) exits as soon as the last send call returns. As you try to send 20*3000 bytes, you can expect (when you use a true network and not the loopback interface on a single machine) than a number of bytes have just been queued for transfer and have not yet been received at that moment. But the end of the client program will abruptly close the socket and the queued bytes will not be sent at all

    • the receiver expects all the 60000 bytes to be received and never tests for an early peer closure on the socket. If it happens (and because of the problem sender side, it is to be expected), the receiver will fall in an endless loop reading 0 bytes from a socket which has already closed by the sender.

    What to do:

    1. the reciever should test for a 0 bytes read. If it happens it means that nothing will ever come from the socket and it should immediately abort with an error message. At least the problem will be easier to diagnose.

    2. the sender should not abruptly close its socket but instead use a graceful shutdown: when everything has been sent, it should use shutdown on the socket notifying the peer that nothing more will be sent, and wait for the receiver to close the socket when everything has correctly be transmitted. It is enough to use a blocking read of a few bytes: the read will nicely block until the peer closes its socket and will then return a 0 bytes read.