Server program stuck on read() function but write() function works well in the client program

Question

I am working on a simple chatroom program using socket programming method in C. I have implemented that clients can connect to the server and send messages to the server, and the server should be distributed messages to the others.

But when I am trying to test my program and trying to make two clients communicate, something strange happens. The strange thing happens if I follow the following steps.

1. run server program and waiting for connections.
2. open one client program and connecting to the server, then send two messages, both work successfully.
3. open another client and do the same, the first client can receive the message that the second client sent perfectly.
4. But when I switch to the first server and try to write a new message and send to the server to be distributed, it fails. And neither the second client nor the server could receive this message.
5. After some debugging, it seems that the server program stuck on read() function. By the way, I did these all on my Uubuntu18.04 virtual machine in different terminals.

More points:

1. I have tried to debug and print all the values I can get, it seems that the write() function of the client program working properly, but the server program stops at the second function read() inside the Receive thread.
2. I think that this might be a question about the blocking state of read() function but I am new to networking programming and quite don't understand.

This is a screenshot of my problem (Chat-room:Left is a server, right are two clients):

Here are my client.c and server.c codes, I have indicated the position that the issue occurs.

CLIENT.C

/* tcp-client.c */
#include <stdio.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <strings.h>
#include <arpa/inet.h>
#include <unistd.h>
#include <string.h>
#include <pthread.h>
#include <stdlib.h>

//send the message to the server any time terminal get input
void*  Send(void* Socked)
{
    char sender[80];
    //save the socked into a int pointer
    int *SockedCopy = Socked;
    while(fgets(sender, sizeof(sender), stdin)){
        //whenever enter a string, send it
        int messageSize = strlen(sender) + 1;
        write(*SockedCopy, &messageSize, sizeof(int));
        int i = write(*SockedCopy, sender, messageSize);      //both work when sending message
        printf("write over! the write() returns %d\n", i);
        //check whether this is a quit message
        if(strcmp(sender, "q!") == 0)
            exit(1);

    }
}


//receive message from server
void* Receive(void* Socked)
{
    int *SockedCopy = Socked;
    char Receiver[80];

    while(1){
        //read message continuosly
        int reveiverEnd = 0;
        reveiverEnd  = read (*SockedCopy, Receiver, 1000);
        Receiver[reveiverEnd] = '\0';   
        fputs(Receiver, stdout);
        Receiver[0] = '\0';
    }
}

int main ()
{
    int sockfd, n;
    pthread_t threadSend;
    pthread_t threadReceive;
    struct sockaddr_in serv, cli;
    char rec[1000];
    char send[80];

    //input UserName
    printf("Input Username:" );
    fgets(send, sizeof(send), stdin);
    send[strlen(send) - 1] = '\0';
    int MessageSize = strlen(send);

    //create socked
    sockfd = socket (PF_INET, SOCK_STREAM, 0);

    //create server information
    bzero (&serv, sizeof (serv));
    serv.sin_family = PF_INET;
    serv.sin_port = htons (8888);
    serv.sin_addr.s_addr = inet_addr ("127.0.0.1" /*local machine*/);

    //connect to the server
    connect (sockfd, (struct sockaddr *) &serv, sizeof (struct sockaddr));

    //send the user name to the server
    write(sockfd, &MessageSize, sizeof(int));
    write (sockfd, send, sizeof(send));

    //get successfully connecting message
    n = read (sockfd, rec, 1000);//n marks real length
    rec[n] = '\0';  
    fputs(rec, stdout);

    send[0] = '\0';

    //open send thread  
    pthread_create(&threadSend, 0, Send, &sockfd);

    //open receiving message thread
    pthread_create(&threadReceive, 0, Receive, &sockfd);

    //close socked and close two threads
    for(int i = 0; i < 100; ++i)
        sleep(100000);
    pthread_exit(&threadSend);
    pthread_exit(&threadReceive);
    close(sockfd);

    return 0;
}

SERVER.C

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <pthread.h>
#include <sys/socket.h>
#include <linux/in.h>
#include <unistd.h>
#include <unistd.h>

pthread_t thread;
pthread_t threadClient[100];
int ServerSock;

//Every client's information
typedef struct
{
    int sock;
    char UserName[16]; 
    struct sockaddr address;
    int addr_len;
} connection_t;
static connection_t conn[100];

//this function distributes the messsage/status of single client to the other
//Info is the message needed to be distributed
int SendInfo(void* Info)
{
    char *info = Info;
    for(int i = 0; i < 100; ++i)
        //send to the client that exists and doesn't quit room
        if(conn[i].addr_len != -1 && conn[i].addr_len != 0){
            if(send (conn[i].sock, info , strlen(info) + 1, 0) == -1)
                printf("error occured, send to %s fail", conn[i].UserName);
            printf("send %s to %s successfully!\n", info, conn[i].UserName);
        }   
    return 0;   
}


//This function deals with single client, aim to receive message from this client
//and then send them to another using SendIinfo
void* Receive(void* clientnumber)
{
    int* Clientnumber = clientnumber;
    while(1)
    {
        //read the message from the client
        char *Buffer;
        int messageLen = 0;
        read(conn[*Clientnumber].sock, &messageLen, sizeof(int));
        printf("receive from %d\n", messageLen);
        if(messageLen > 0)
        {
            Buffer = (char *)malloc((messageLen+1)*sizeof(char));
            read(conn[*Clientnumber].sock, Buffer, messageLen);   // the program stucks here and don't know why

            if(Buffer[0] != ':') continue;
            Buffer[messageLen] = '\0';
            //whether the client want to quit
            if( Buffer[1] == 'q' && Buffer[2] == '!' )
            {
                //constitute quit message and delete this client
                char quit[] = " quit the chat room";
                char quitMessage[20];       
                quitMessage[0] = '\0';
                strcat(conn[*Clientnumber].UserName, quit); 
                SendInfo(quitMessage);
                conn[*Clientnumber].addr_len = -1;
                pthread_exit(&threadClient[*Clientnumber]);
            }
            else{
                //constitute the message
                char begin[] = " says";
                char messageDistribute[200];
                messageDistribute[0] = '\0';
                strcat(messageDistribute, conn[*Clientnumber].UserName);
                strcat(messageDistribute, begin);
                strcat(messageDistribute, Buffer);
                SendInfo(messageDistribute);
            }
            free(Buffer);
        }
        else
            continue;
    }
}



//aim to accept whenever there is a client trying to connect
void * process(void * ptr)
{
    pthread_t clientThread[100];
    char * buffer;
    int len;
    //the number of the client connecting now
    int clientNumber = 0;      
    long addr = 0;
    while(1){
        //waiting to be connected
        if(clientNumber < 100)
        {
            conn[clientNumber].sock = accept(ServerSock, &conn[clientNumber].address, &conn[clientNumber].addr_len);
        }
        else
            break;


        //the length of the message
        read(conn[clientNumber].sock, &len, sizeof(int));
        if (len > 0)
        {

            //multiple information of a client
            addr = (long)((struct sockaddr_in *)&conn[clientNumber].address)->sin_addr.s_addr;
            buffer = (char *)malloc((len+1)*sizeof(char));
            buffer[len] = 0;
            read(conn[clientNumber].sock, buffer, len);


            //send success message to the client
            send (conn[clientNumber].sock, "You have entered the chatroom, Start CHATTING Now!\n", 51, 0);


            //save client's nick name
            strcpy(conn[clientNumber].UserName, buffer);


            printf("User <%s> has entered the Chatroom!\n", conn[clientNumber].UserName);
            printf("There are %d people in Chatroom now!\n",clientNumber+1);
            free(buffer);

            //create a thread dealing the messages from a single client
            int number = clientNumber;
            pthread_create(&threadClient[clientNumber], 0, Receive, &number);
        }
        clientNumber += 1;

    }
    pthread_exit(0);
}

int main(int argc, char ** argv){
    struct sockaddr_in address;
    int port = 8888;
    connection_t * connection;

    //create socked
    ServerSock = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

    //bind the socked to a port
    address.sin_family = AF_INET;
    address.sin_addr.s_addr = INADDR_ANY;
    address.sin_port = htons(port);
    if (bind(ServerSock, (struct sockaddr *)&address, sizeof(struct sockaddr_in)) < 0)
    {
        fprintf(stderr, "error: cannot bind socket to port %d\n", port);
        return -4;
    }


    //listen for connections
    listen(ServerSock, 100);
    printf("the server is ready and listening\n");


    //creating a thread dealing with connections
    pthread_create(&thread, 0, process, (void *)connection);


    //keep this program working
    for(int i = 0; i < 100; ++i)
        sleep(10000);


    //close socked and thread
    pthread_detach(thread);
    close(ServerSock);
    return 0;
}

This is for an academic homework.

Answer 1

Your problem isn't a networking problem at all, but rather an error in how you are passing your clientnumber argument to your Receive thread:

 if (len > 0)
 {
    [...]

    int number = clientNumber;
    pthread_create(&threadClient[clientNumber], 0, Receive, &number);
 }  // number gets destroyed here

Note that in the above code, number is a local variable, which means it will be destroyed as soon as the scope exits (i.e. as soon as execution reaches the } line, above).

However, you are passing a pointer to number (i.e. &number) to your Receive thread, and your Receive thread tries to use that pointer to look up data about the client. That means the Receive thread is dereferencing a dangling pointer, which invokes undefined behavior. (In this case, what I see is that the program sort of works for a short time, and then stops working, because the memory location Clientnumber points to has been overwritten by some other data, and therefore the next time the Receive thread tries to dereference the pointer, it ends up looking at the wrong data, and therefore calls read() on the wrong socket fd ... but in principle anything could happen, since once you've invoked undefined behavior, all bets are off)

In any case, one easy way to fix the problem would be to pass a pointer to the connection_t object directly, instead of passing an index, i.e.:

pthread_create(&threadClient[clientNumber], 0, Receive, &conn[clientNumber] );

.... and then you can use that pointer directly in your Receive thread:

void* Receive(void* arg)
{
   connection_t * conn = (connection_t *) arg; 

   [...]

   int r = read(conn->sock, &messageLen, sizeof(int));

Doing it this way won't invoke undefined behavior, because the connection_t conn[100]; array is declared as a static/global, and therefore the objects inside it are guaranteed not to be destroyed until the process exits.

If you still need to access the clientNumber index value from within Receive(), you can add that value as another field in the connection_t struct definition.