TCP Socket select server segmentation fault when client disconnect

Question

This is a simple client server system. Server uses select to handle different client request. But the problem is: When I shut down the client, server will get segmentation fault. I don't know how to deal with the situation. Thanks for your help.

Client Side:

#include <stdio.h>
#include <iostream>
#include <stdlib.h>
#include <queue>
#include <cstdlib>
#include <string.h>
#include <mutex>
#include <thread>
#include <pthread.h>
#include <sys/socket.h> /* for socket(), connect(), send(), and recv() */
#include <arpa/inet.h>  /* for sockaddr_in and inet_addr() */
#include <stdlib.h>     /* for atoi() and exit() */
#include <string.h>    /* for memset() */
#include <unistd.h> /* for close() */
#include <ctype.h>
#define SIZESTACKSPACE 1000000;
#define RCVBUFSIZE 32   /* Size of receive buffer */
using namespace std;

void *sendRequest(void *);
void *receiveRequest(void *);

//#define TEMPPORTNO "40868";
//#define IP1 "10.10.154.41";
//#define IP1 "192.168.37.166";


int sock;                        /* Socket descriptor */
bool running1 = true, running2 = true;

//queue1 is used for sending message
//queue2 is used by recieve to display message
queue<char*> queue1, queue2;

//create two mutex for two queue
pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER;
pthread_mutex_t mutex2 = PTHREAD_MUTEX_INITIALIZER;

//create conditional variables
pthread_cond_t cond1 = PTHREAD_COND_INITIALIZER;
pthread_cond_t cond2 = PTHREAD_COND_INITIALIZER;

pthread_attr_t attr;
int main(void){
    pthread_t thread2, thread1;
    int rc2, rc1;

    size_t stacksize;

    //initialize attributes
    pthread_attr_init(&attr);
    pthread_attr_getstacksize (&attr, &stacksize);
    stacksize = sizeof(double)*1000+SIZESTACKSPACE;

    pthread_attr_setstacksize(&attr, stacksize);

    //create thread1, with function sendRequest()
    rc1 = pthread_create(&thread1, &attr, sendRequest, NULL);
    if(rc1){
        cout<<" ERROR; return code from pthread_create() is " << rc1;
        exit(-1);
    }
    //create thread2, with the function recieveRequest()
    rc2 = pthread_create(&thread2, &attr, receiveRequest, NULL);
    if(rc2){
        cout<<" ERROR; return code from pthread_create() is " << rc2;
        exit(-1);
    }

    int chunksize = 4; /* use BUFSIZ-1 for (probably) more efficient program */
    char *s;
    char *temp;
    int buffersize;
    int nPos;
    char c;
    char *str;

    while(1){
        buffersize = chunksize+1;

        s = (char*) malloc(buffersize); /* get the first chunk */
        if (s == NULL) {
            printf("Can't allocate %d bytes\n", buffersize);
            exit(1);
        }

        if((c=getchar()) != '\n' && c != EOF){
            nPos = 1;
            s[0] = c;
            while((c=getchar()) != '\n' && c != EOF){
                s[nPos] = c;
                if(nPos>=buffersize){
                    buffersize += chunksize;
                    temp = (char*)realloc(s, buffersize);
                    if (temp == NULL) {
                        printf("Can't realloc %d bytes\n", chunksize);
                        free(s); /* clean up the previously allocated stuff */
                        s = NULL;
                        exit(1);
                    }
                    s=temp;
                }
            nPos++;
            }

            int k, j;

            /*The last character is null*/
            //nPos is the length of the string, aside of the null character
            s[nPos] = '\0';

            //to store the new string, allocation
            str = (char*) malloc(nPos+6);

            j = nPos;
            //each byte contains a value of the number
            k = 3;
            while(k>=0){
                str[k] = nPos % 10;
                nPos = nPos/10;
                k--;
            }

            str[4] = '\0';

            k = 0;
            while(k<=j){
                str[k+5] = s[k];
                k++;
            }

            str[k+5] = '\0';

            free(s);

            //add mutex here
            pthread_mutex_lock(&mutex1);
            queue1.push(str);
            //signal sendRequest
            if(!queue1.empty())
                pthread_cond_signal(&cond1);
            pthread_mutex_unlock(&mutex1);

            //signal recvRequest
            pthread_mutex_lock(&mutex2);
            queue2.push(str);
            if(!queue2.empty())
                pthread_cond_signal(&cond2);
            pthread_mutex_unlock(&mutex2);
        }else if(c==EOF){
            break;
        }
    }
    //wait for thread 2
    while(!queue2.empty());
    //signal for threads

    close(sock);

    /* Clean up and exit */
    pthread_attr_destroy(&attr);
    pthread_exit(NULL);

    pthread_mutex_destroy(&mutex1);
    pthread_mutex_destroy(&mutex2);
    pthread_cond_destroy(&cond1);
    pthread_cond_destroy(&cond2);

    return 0;
}

void *sendRequest(void *){


    struct sockaddr_in echoServAddr; /* Echo server address */
    unsigned short servPort;     /* server port */
    char* servIP;                    /* Server IP address (dotted quad) */
    char* echoString;                /* String to send to echo server */
    unsigned int stringLen;
    char* tempPort;


    //fetch environment variables from the system
    servIP = getenv ("SERVER_ADDRESS");
    tempPort = getenv("SERVER_PORT");

//    servIP = IP1;
//    tempPort = TEMPPORTNO;
    if(strlen(servIP) == 0 || strlen(tempPort) == 0){
        perror("DOES NOT SET ENVIRONMENT VARIABLES FOR SERVER_ADDRESS OR SERVER_PORT\n");
        exit(-1);
    }
    servPort = atoi(tempPort);
    /* Create a reliable, stream socket using TCP */
    if ((sock = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0){
        printf("socket() failed\n");
        exit(1);
    }

    /* Construct the server address structure */
    memset(&echoServAddr, 0, sizeof(echoServAddr));     /* Zero out structure */
    echoServAddr.sin_family      = AF_INET;             /* Internet address family */
    echoServAddr.sin_addr.s_addr = inet_addr(servIP);   /* Server IP address */
    echoServAddr.sin_port        = htons(servPort);     /* Server port */

    /* Establish the connection to the echo server */
    if (connect(sock, (struct sockaddr *) &echoServAddr, sizeof(echoServAddr)) < 0){
        perror("connect() failed\n");
        exit(1);
    }
    pthread_mutex_lock(&mutex1);
    while(true){
        pthread_cond_wait(&cond1, &mutex1);

        echoString = queue1.front();
        queue1.pop();

        pthread_mutex_unlock(&mutex2);

        //send a request to the server
        //determine the length that is going to be sent
        stringLen = 5;
        while(echoString[stringLen])
            stringLen++;


        /* Send the string to the server */
        if (send(sock, echoString, stringLen, 0) != (int)stringLen){
            perror("send() sent a different number of bytes than expected\n");
            exit(1);
        }



        sleep(2);//sleep for 2 seconds

    }
    return (void *) 0;
}
void *receiveRequest(void *){
    char* temp;

    char *echoBuffer;     /* Buffer for echo string */
    unsigned int echoStringLen;      /* Length of string to echo */
    unsigned int bytesRcvd;   /* Bytes read in single recv()]*/
    char  *partTemp;
    unsigned int stringLen;
    pthread_mutex_lock(&mutex2);
    while(true){
        //get the toppest value from the queue
        pthread_cond_wait(&cond2, &mutex2);
        temp = queue2.front();
        pthread_mutex_unlock(&mutex2);

        //get the length from the queue
        //cp the first four bytes
        int k = 0;

        stringLen = 0;
        while(k<4){
            stringLen = stringLen * 10 + temp[k];
            k++;
        }

        //wait for response

        echoBuffer = (char *)malloc(5);

        if((bytesRcvd = recv(sock, echoBuffer,5, 0)) <= 0){
            printf("recv() failed or connection closed prematurely\n");
            exit(1);
        }


        //totalBytesRcvd += bytesRcvd;
        //get the length of the string recv
        k=0;
        echoStringLen = 0;
        while(k<4){
            echoStringLen = echoStringLen * 10 + echoBuffer[k];
            k++;
        }

        echoBuffer = (char *)realloc(echoBuffer, echoStringLen + 1);

        //recive the rest of the string, which is a length of echoStringLen+1
        bytesRcvd = recv(sock, echoBuffer, echoStringLen+1, 0);
        echoBuffer[echoStringLen] = '\0';

        //escape the first 5 bytes for the printing
        partTemp = temp;

        k=0;
        while(k<5){

            partTemp ++;
            k++;
        }

        printf("%s\nServer: %s \n", partTemp, echoBuffer);

        free(echoBuffer);

        //pop the toppest value from the queue
        //this is useful for closing the threads
        //it can ensure all allocations are freed
        pthread_mutex_lock(&mutex2);
        queue2.pop();
        free(temp);
        pthread_mutex_unlock(&mutex2);


    }
    return (void *) 0;
}

Server Side:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>
#include <sys/wait.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <ifaddrs.h>

#define PORT 0

#define MAXPENDING 5


int main(){
    struct sockaddr_in serverAddress, clientAddress; // These stores the network settings
    socklen_t serverAddressLength = sizeof(serverAddress), clientAddressLength = sizeof(clientAddress);
    int serverSocketID, clientSocketID;


    if ((serverSocketID = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0) {
        perror("socket() failed");
        exit(1);
    }

    // Specifying preference for IP address and port number lookup
    memset(&serverAddress, 0, sizeof(serverAddress)); // Initialize memory for
    serverAddress.sin_family = AF_INET;
    serverAddress.sin_addr.s_addr = htonl(INADDR_ANY);
    serverAddress.sin_port = htons(PORT);

    if (bind(serverSocketID, (struct sockaddr *) &serverAddress, serverAddressLength) != 0) {
        perror("bind() failed");
        close(serverSocketID);
        exit(1);
    }

    // Server starts to listen
    if (listen(serverSocketID, MAXPENDING) == -1) {
        perror("listen() failed");
        close(serverSocketID);
        exit(1);
    }

    //The following code is to obtain IP address from ifaddr info from Linux
    getsockname(serverSocketID, (struct sockaddr*) &serverAddress, &serverAddressLength);
    struct ifaddrs * ifAddrStruct=NULL;
    struct ifaddrs * ifa=NULL;
    void * tmpAddrPtr=NULL;

    getifaddrs(&ifAddrStruct);

    for (ifa = ifAddrStruct; ifa != NULL; ifa = ifa->ifa_next) {
        if (ifa ->ifa_addr->sa_family==AF_INET) { // check it is IP4
            // is a valid IP4 Address
            tmpAddrPtr=&((struct sockaddr_in *)ifa->ifa_addr)->sin_addr;
            char addressBuffer[INET_ADDRSTRLEN];
            inet_ntop(AF_INET, tmpAddrPtr, addressBuffer, INET_ADDRSTRLEN);
            if(ifa->ifa_name[0] == 'e' ){
                printf("SERVER_ADDRESS %s\nSERVER_PORT %d\n", addressBuffer, (int) ntohs(serverAddress.sin_port));
                break;
            }
        } else if (ifa->ifa_addr->sa_family==AF_INET6) { // check it is IP6
            // is a valid IP6 Address
            tmpAddrPtr=&((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr;
            char addressBuffer[INET6_ADDRSTRLEN];
            inet_ntop(AF_INET6, tmpAddrPtr, addressBuffer, INET6_ADDRSTRLEN);
            if(ifa->ifa_name[0] == 'e' ){

                printf("SERVER_ADDRESS %s\nSERVER_PORT %d\n", addressBuffer, (int) ntohs(serverAddress.sin_port));
                break;
            }
        }
    }

    if (ifAddrStruct!=NULL) freeifaddrs(ifAddrStruct);

    // Select attributes
    int largestFileDescriptorIndex = serverSocketID;
    // We will add clients to the master list, select will use a worker copy of our master list
    fd_set master, worker;
    //initialize the file descriptor list
    FD_ZERO(&master);
    FD_ZERO(&worker);
    FD_SET(serverSocketID, &master);
    // Add keyboard to allow control over server
    FD_SET(STDIN_FILENO, &master);

    // Specify how long to block and wait for a client to do something
    struct timeval fileDescriptorWaitTime;
    // Wait for 1 second to check if there is data coming in
    fileDescriptorWaitTime.tv_sec = 1;
    fileDescriptorWaitTime.tv_usec = 0;

    int running = 1, i;
    while(running) { // This is the big red switch that makes the server run
        worker = master; // Resets the select list
        if (select(largestFileDescriptorIndex + 1, &worker, NULL, NULL, &fileDescriptorWaitTime) == -1) {
            perror("select() failed");
            close(serverSocketID);
            exit(1);
        }
        // Loop through the state of all file descriptors
        for (i = 0; i <= largestFileDescriptorIndex; i++) {
            // Check if any file descriptor changed state
            if (FD_ISSET(i, &worker)) {
                // A new client is trying to connect
                if (i == serverSocketID) {
                    // Client connect successfully
                    if ((clientSocketID = accept(serverSocketID,
                            (struct sockaddr*) &clientAddress, &clientAddressLength)) != -1) {
                        // Register client into master list
                        FD_SET(clientSocketID, &master);
                        if (clientSocketID > largestFileDescriptorIndex) {
                            // Update length of list to loop
                            largestFileDescriptorIndex = clientSocketID;
                        }
                    }

                }
                else if (i == STDIN_FILENO) { // Check keyboard input
                    fprintf(stdout, "Server is Shutting down\n");
                    getchar();
                    running = 0;
                    continue;
                }else
                {
                    char *echoBuffer;        /* Buffer for echo string */
                    int recvMsgSize;                    /* Size of received message */
                    int j;

                    echoBuffer = (char *)malloc(5);
                    /* Receive message from client, get the first 5 bytes first to know the length of the string*/
                    if ((recvMsgSize = recv(clientSocketID, echoBuffer, 5, 0)) < 0){
                        perror("recv() failed");
                        close(clientSocketID);
                        FD_CLR(clientSocketID, &master);
                    }

                    int stringLen=0, k = 0;//the length of the string

                    /*convert the char * into an int*/
                    while(k<4){
                        stringLen = stringLen*10 + (int)echoBuffer[k];
                        k++;
                    }

                    char *str; // store the string
                    //string size + 4 bytes + '\0'+strlen(string)+'\0'
                    str = (char *)malloc(stringLen + 6);
                    //put the first 5 bytes into the echo string
                    k = 0;
                    while(k<5){
                        str[k] = echoBuffer[k];
                        k++;
                    }
                    free(echoBuffer);

                    //recieve string of a length of stringLen+1, which is char num + '\0'
                    echoBuffer = (char *)malloc(stringLen+1);
                    if ((recvMsgSize = recv(clientSocketID, echoBuffer, stringLen+1, 0)) < 0){
                        perror("recv() failed");
                        close(clientSocketID);
                        FD_CLR(clientSocketID, &master);
                    }

                    //set the last char to be null
                    echoBuffer[stringLen]='\0';
                    printf("%s\n", echoBuffer);


                    //deal with the data here
                    if(echoBuffer[0] <= 'z' && echoBuffer[0]>='a')
                        echoBuffer[0] = echoBuffer[0] + 'A'-'a';

                    //operations on data except the first one
                     for( j = 1; j<stringLen; j++)
                    {
                        if(echoBuffer[j]<='z' && echoBuffer[j]>='a' && echoBuffer[j-1] == ' ')
                            echoBuffer[j] = echoBuffer[j] + 'A'-'a';
                        else if(echoBuffer[j]<='Z' && echoBuffer[j]>='A' && echoBuffer[j-1] != ' ')
                            echoBuffer[j] = echoBuffer[j] + 'a'-'A';
                    }

                     //store the data into str
                    k= 0;
                    while(k<=stringLen){
                        str[k+5] = echoBuffer[k];
                        k++;
                    }
                    str[stringLen+5] = '\0';
                    free(echoBuffer);

                    recvMsgSize = stringLen+6;
                    /* Send received string */
                    /* Echo message back to client */
                    if (send(clientSocketID, str, recvMsgSize, 0) != recvMsgSize){
                        perror("send() failed");
                        close(clientSocketID);
                        FD_CLR(clientSocketID, &master);
                    }
                    free(str);

                }//operations on the data finishes
            }//if the client socket descriptor is in the list
        }//loop through all the file descriptors
    }//busy waiting

    close(serverSocketID);
    return 0;
}

Answer 1

When a client disconnects, the client's socket will select as ready-for-read, and then all subsequent attempts to recv() on that socket will return 0, to indicate EOF. Note that this situation is the only time recv() will return 0. It looks like your code is expecting recv() to return -1 in that scenario instead, so it isn't handling that case correctly.

Also it looks like you try to use echoBuffer after free()-ing it, which is undefined behavior and should be avoided. (In fact, why use malloc() and free() at all? Just declare echoBuffer on the stack with a large-enough size, and you won't have to worry about when to free() it)