c sockets - not receiving all sent data

Question

I'm having some problems with two programs I wrote, a server and a client. To keep it easy and chronological, I first wrote the server and tested it with both telnet and netcat and everything works fine (except for a difference in the return value of read() / recv(), as it looks like the normal telnet program adds an additional char at the end of the string to be sent, but anyway...).

Now I have written the client program too, but I am not getting all the data which was correctly received by the other two clients, in particular the row[i] strings I get from the MySQL query. Things change when I introduce a usleet() call after each send() function and all data is received correctly.

Now I was thinking about an incompatible buffer size problem (?), but after playing for a while and checking the dimensions, I was not able to find out anything.

You will find the code below, please if you have any advice don't hesitate to tell me.

Tnx

/* CLIENT CODE */

#define BUFSIZE 1000
...
void *send_handler(void *);
...
int main(int argc, char *argv[]) {
    char buf[BUFSIZE];
    ...
    socket stuff...
    ...
    connect
    ...
    /* receive string from server - working */
    bzero(buf, BUFSIZE);
    n = read(sockfd, buf, BUFSIZE);
    if (n < 0) 
        error("ERROR reading from socket");
    buf[n] = '\0';
    printf("%s", buf);
    ...
    /* send username to server - working */
    bzero(buf, BUFSIZE);
    fgets(buf, BUFSIZE, stdin);
    n = write(sockfd, buf, strlen(buf));
    if (n < 0) 
        error("ERROR writing to socket");
    ...
    /* start receiving handler */
    if( pthread_create( &thread_id , NULL , send_handler , (void*) &sockfd) < 0) {
    perror("could not create thread");
        return 1;
    }

    /* main thread for reading data */
    while(1) {
        bzero(buf, BUFSIZE);
        n = read(sockfd, buf, BUFSIZE);
        if (n < 0) 
    error("ERROR reading from socket");
    buf[n] = '\0';
    printf("%s", buf);
    }

    close(sockfd);
    return 0;
}

void *send_handler(void *socket_desc) {
    //Get the socket descriptor
    int sock = *(int*)socket_desc;
    char buf[BUFSIZE];
    int n;

    while (1) {
        bzero(buf, BUFSIZE);
        fgets(buf, BUFSIZE, stdin);
        n = write(sock, buf, strlen(buf));
        if (n < 0) 
            error("ERROR writing to socket");
    }
}

/* SERVER CODE */

void *connection_handler(void *);

int main(int argc , char *argv[]) {
    ...
    /* socket variables */
    ...
    pthread_t thread_id;
    ...
    socket stuff...
    ...
    while((client_sock = accept(socket_desc, (struct sockaddr *)&client_addr, (socklen_t*)&client_len))) {
        if( pthread_create( &thread_id , NULL , connection_handler , (void*) &client_sock) < 0) {
        perror("could not create thread");
            return 1;
    }
    }
    return 0;
}

void *connection_handler(void *socket_desc) {
//Get the socket descriptor
int sock = *(int*)socket_desc;
    ...
    /* mysql variables */
char cmd[1000];
...
MYSQL_RES *result;
MYSQL_ROW row;
MYSQL *con;
...
/* connection variables */
int read_size, i;
char *message;
char client_message[2000];
char buffer[1000];
    ...
    //clear the buffers
memset(client_message, '\0', 2000);
    ...
    snprintf(cmd, 999, "SELECT field1, field2, field3, field4 FROM files WHERE key='%s' ORDER BY id DESC", var);
if (mysql_query(con, cmd)) {
    error checks...
}
result = mysql_store_result(con);
if (result == NULL) {
    error checks...
}
else {
    num_rows = mysql_num_rows(result);
}
if (num_rows == 0) {
    message = "Nothing found\n";
    send(sock , message , strlen(message), 0);  
}
    else {
    num_fields = mysql_num_fields(result);
    num_rows = mysql_num_rows(result);
        snprintf(buffer, 999, "Number of rows: %d\n", num_rows);
    send(sock , buffer , sizeof(buffer), 0);
    //usleep(10000); // commented, but necessary to work properly...
        memset(buffer, '\0', sizeof(buffer));

        while ((row = mysql_fetch_row(result))) { 
    for(i = 0; i < num_fields; i++) {
        snprintf(buffer, 999, "%s\t", row[i] ? row[i] : "NULL");
        send(sock , buffer , sizeof(buffer), 0);
        //usleep(10000);
        memset(buffer, '\0', sizeof(buffer));
        }
        message = "\n";
        send(sock , message , strlen(message), 0);
        //usleep(10000);
    }
    message = "\n";
    send(sock , message , strlen(message), 0);
        //usleep(10000);
    mysql_free_result(result);
}
...
}

EDIT: I changed printf("%s", buf);

with printf("Bytes read: %d\n", n); in the client code and I obtained following output:

with commented usleep():

Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 36
Bytes read: 1000
Bytes read: 31
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 2
(17 lines)

with usleep(0) slowing down the send flow (correct output obtained):

Bytes read: 1000
Bytes read: 33
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1
Bytes read: 1
Bytes read: 1000
Bytes read: 31
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1000
Bytes read: 1
Bytes read: 1
(21 lines)

Any hint?

SOLVED: just by replacing

sizeof(buffer);

with

strlen(buffer);

in the server part and everything works fine even without usleep(), the output is correct/complete.

Thanks anyway.

Answer 1

You can't assume that a single read reads an entire message. There are no messages in TCP, only bytes, and any given read may return as few as one byte, or the result of several writes at the peer all at once. You have to loop and parse.

Answer 2

You have a race condition in your code, you are passing a pointer to a local variable to the thread function. Since a filedescriptor (int) is not larger than a void pointer (I'm pretty sure that is guaranteed, but add an assertion nonetheless), you can also convert the descriptor value to a pointer instead of passing the address of the local filedescriptor:

int s = accept(...);
if(int e = pthread_create(.., &connection_handler, (void*)s, ..))
    error(..);

BTW: pthread_create returns zero on success and otherwise an error code, which is not negative. But it's very unlikely that your code failed at that point.