Asynchronous C client for a multiclient C server

Question

I have a client which is working fine, but whenever I run a new client, sometimes I don't receive the sent message on the other client already running, while using telnet it works flawlessly, the message "broadcasts" to all connected clients, and I want whenever a message is received to one of the clients to show even if I didn't already send a message. Should I use select on clients ? and what should be changed ?

client.c:

#include <stdio.h> //printf
#include <string.h>    //strlen
#include <sys/socket.h>    //socket
#include <arpa/inet.h> //inet_addr
#include <unistd.h> 

int main(int argc , char *argv[]){
    int sock;
    struct sockaddr_in server;
    char message[256] , server_reply[256];

    //Create socket
    sock = socket(AF_INET , SOCK_STREAM , 0);
    if (sock == -1)
    {
        printf("Could not create socket");
    }
    puts("Socket created");

    server.sin_addr.s_addr = inet_addr("127.0.0.1");
    server.sin_family = AF_INET;
    server.sin_port = htons( 9034 );

    //Connect to remote server
    if (connect(sock , (struct sockaddr *)&server , sizeof(server)) < 0){
        perror("connect failed. Error");
        return 1;
    }

    puts("Connected\n");    

    //keep communicating with server
    for(;;){

    printf("Enter message: ");
    memset(message, 0, 256);
    fgets(message, 256,stdin);
    // scanf("%s" , message);

        //Send some data
        if( send(sock , message , strlen(message) , 0) < 0)
        {
            puts("Send failed");
            return 1;
        }

        //Receive a reply from the server
        if( recv(sock , server_reply , 256 , 0) < 0)
        {
            puts("recv failed");
            break;
        }

    printf("Server Reply: %s\n", server_reply);
    server_reply[0]='\0'; 
    }

    close(sock);
    return 0;
}

server.c:

/*
** selectserver.c -- a cheezy multiperson chat server
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>

#define PORT "9034"   // port we're listening on

// get sockaddr, IPv4 or IPv6:
void *get_in_addr(struct sockaddr *sa)
{
    if (sa->sa_family == AF_INET) {
        return &(((struct sockaddr_in*)sa)->sin_addr);
    }

    return &(((struct sockaddr_in6*)sa)->sin6_addr);
}

int main(void){
    fd_set master;    // master file descriptor list
    fd_set read_fds;  // temp file descriptor list for select()
    int fdmax;        // maximum file descriptor number

    int listener;     // listening socket descriptor
    int newfd;        // newly accept()ed socket descriptor
    struct sockaddr_storage remoteaddr; // client address
    socklen_t addrlen;

    char buf[256];    // buffer for client data
    int nbytes;

    char remoteIP[INET6_ADDRSTRLEN];

    int yes=1;        // for setsockopt() SO_REUSEADDR, below
    int i, j, rv;

    struct addrinfo hints, *ai, *p;

    FD_ZERO(&master);    // clear the master and temp sets
    FD_ZERO(&read_fds);

    // get us a socket and bind it
    memset(&hints, 0, sizeof hints);
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_flags = AI_PASSIVE;
    if ((rv = getaddrinfo(NULL, PORT, &hints, &ai)) != 0) {
        fprintf(stderr, "selectserver: %s\n", gai_strerror(rv));
        exit(1);
    }

    for(p = ai; p != NULL; p = p->ai_next) {
        listener = socket(p->ai_family, p->ai_socktype, p->ai_protocol);
        if (listener < 0) { 
            continue;
        }

        // lose the pesky "address already in use" error message
        setsockopt(listener, SOL_SOCKET, SO_REUSEADDR, &yes, sizeof(int));

        if (bind(listener, p->ai_addr, p->ai_addrlen) < 0) {
            close(listener);
            continue;
        }

        break;
    }

    // if we got here, it means we didn't get bound
    if (p == NULL) {
        fprintf(stderr, "selectserver: failed to bind\n");
        exit(2);
    }

    freeaddrinfo(ai); // all done with this

    // listen
    if (listen(listener, 10) == -1) {
        perror("listen");
        exit(3);
    }

    // add the listener to the master set
    FD_SET(listener, &master);

    // keep track of the biggest file descriptor
    fdmax = listener; // so far, it's this one

    // main loop
    for(;;) {
        read_fds = master; // copy it
        if (select(fdmax+1, &read_fds, NULL, NULL, NULL) == -1) {
            perror("select");
            exit(4);
        }

        // run through the existing connections looking for data to read
        for(i = 0; i <= fdmax; i++) {
            if (FD_ISSET(i, &read_fds)) { // we got one!!
                if (i == listener) {
                    // handle new connections
                    addrlen = sizeof remoteaddr;
                    newfd = accept(listener,
                        (struct sockaddr *)&remoteaddr,
                        &addrlen);

                    if (newfd == -1) {
                        perror("accept");
                    } else {
                        FD_SET(newfd, &master); // add to master set
                        if (newfd > fdmax) {    // keep track of the max
                            fdmax = newfd;
                        }
                        printf("selectserver: new connection from %s on "
                            "socket %d\n",
                            inet_ntop(remoteaddr.ss_family,
                                get_in_addr((struct sockaddr*)&remoteaddr),
                                remoteIP, INET6_ADDRSTRLEN),
                            newfd);
                    }
                } else {
                    // handle data from a client
                    memset(buf, 0, 256);
                    if ((nbytes = recv(i, buf, sizeof buf, 0)) <= 0) {
                        // got error or connection closed by client
                        if (nbytes == 0) {
                            // connection closed
                            printf("selectserver: socket %d hung up\n", i);
                        } else {
                            perror("recv");
                        }
                        close(i); // bye!
                        FD_CLR(i, &master); // remove from master set
                    } else {
                        // we got some data from a client
                        for(j = 0; j <= fdmax; j++) {
                            // send to everyone!
                            if (FD_ISSET(j, &master)) {
                                // except the listener and ourselves
                                if (j != listener && j != i) {
                                    if (send(j, buf, nbytes, 0) == -1) {
                                        perror("send");
                                    }
                                }
                            }
                        }
                    }
                } // END handle data from client
            } // END got new incoming connection
        } // END looping through file descriptors
    } // END for(;;)--and you thought it would never end!

    return 0;
}

Answer 1

The reason a client can't receive a message until they send one is because.

fgets(message, 256,stdin);

Will keep "reading" (and will therefore block) until an EOF or a newline character has been read from the input stream

Also, note that

if( recv(sock , server_reply , 256 , 0) < 0)

blocks if there is nothing to read, which will prevent that user from sending more messages to the server until there is something new to read from the server. Assuming that you've played online games before, I hope that you can see that such a setup would be rather annoying!

So, we have to find someway of checking to see if we can read from STDIN and the server socket without incurring a block. Using select() will prevent us blocking on the sever socket, but it wouldn't work for STDIN whilst using fgets() to read input from the user. This is because, as mentioned above, fgets() blocks until an EOF or newline is detected.

The main solution I have in mind is to replace fgets with a method buffer_message() that will only read from STDIN when it won't block on read (we'll use select() to implement this). We'll then place what is read into a buffer. If there is a full message, this message will then be written to the server. Otherwise, we'll let the control keep going through the program until there is something to read or write.

This is code from a recent university assignment I did and so a small portion of the code isn't mine

Declarations:

//directives are above (e.g. #include ...)

//message buffer related delcartions/macros
int buffer_message(char * message);
int find_network_newline(char * message, int inbuf);
#define COMPLETE 0
#define BUF_SIZE 256

static int inbuf; // how many bytes are currently in the buffer?
static int room; // how much room left in buffer?
static char *after; // pointer to position after the received characters
//main starts below

Main:

//insert the code below into main, after you've connected to the server
puts("Connected\n");    

//set up variables for select()
fd_set all_set, r_set;
int maxfd = sock + 1;
FD_ZERO(&all_set);
FD_SET(STDIN_FILENO, &all_set); FD_SET(sock, &all_set);
r_set = all_set;
struct timeval tv; tv.tv_sec = 2; tv.tv_usec = 0;

//set the initial position of after
after = message;

puts("Enter message: ");
//keep communicating with server
for(;;){

    r_set = all_set;
    //check to see if we can read from STDIN or sock
    select(maxfd, &r_set, NULL, NULL, &tv);

    if(FD_ISSET(STDIN_FILENO, &r_set)){

        if(buffer_message(message) == COMPLETE){
            //Send some data
            if(send(sock, message, strlen(message) + 1, 0) < 0)//NOTE: we have to do strlen(message) + 1 because we MUST include '\0'
            {
                puts("Send failed");
                return 1;
            }

            puts("Enter message:");
        }
    }

    if(FD_ISSET(sock, &r_set)){
        //Receive a reply from the server
        if( recv(sock , server_reply , 256 , 0) < 0)
        {
            puts("recv failed");
            break;
        }

        printf("\nServer Reply: %s\n", server_reply);
        server_reply[0]='\0';

    }
}

close(sock);
return 0;
//end of main

Buffer functions:

int buffer_message(char * message){

    int bytes_read = read(STDIN_FILENO, after, 256 - inbuf);
    short flag = -1; // indicates if returned_data has been set 
    inbuf += bytes_read;
    int where; // location of network newline

    // Step 1: call findeol, store result in where
    where = find_network_newline(message, inbuf);
    if (where >= 0) { // OK. we have a full line

        // Step 2: place a null terminator at the end of the string
        char * null_c = {'\0'};
        memcpy(message + where, &null_c, 1); 

        // Step 3: update inbuf and remove the full line from the clients's buffer
        memmove(message, message + where + 1, inbuf - (where + 1)); 
        inbuf -= (where+1);
        flag = 0;
    }

    // Step 4: update room and after, in preparation for the next read
    room = sizeof(message) - inbuf;
    after = message + inbuf;

    return flag;
}

int find_network_newline(char * message, int bytes_inbuf){
    int i;
    for(i = 0; i<inbuf; i++){
        if( *(message + i) == '\n')
        return i;
    }
    return -1;
}

P.S.

if( send(sock , message , strlen(message) , 0) < 0)

The above can also block if there's no space to write to the server, but there's no need to worry about that here. Also, I'd like to point out a few things you should implement for your client and your server:

1. Whenever you send data over a network, the standard newline is \r\n, or carriage return / newline, or simply the network newline. All messages sent between the client and the server should have this appended at the end.
2. You should be buffering all data sent between the server and the client. Why? Because you're not guaranteed to receive all packets in a message in a single read of a socket. I don't have time to find a source, but when using TCP/IP, packets for a message/file don't have to arrive together, meaning that if you do read, you may not be reading all of the data you intend to read. I'm not well versed in this, so please investigate this more. Open to having this edited / corrected