Thread: Posix-Server behaves unexpected

  1. #1
    Registered User
    Join Date
    Feb 2019
    Posts
    67

    Posix-Server behaves unexpected

    Hey Guys,

    I have encountered a bug, which I do not fully understand, nor I know why this happens. Also I do not understand how exactly the output which is printed was generated.

    If you be so kind and compile these files with me, just to reconstruct the bug.

    SERVER:

    Code:
    #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 <pthread.h>
    #include <stdbool.h>
    
    
    #define BUFLEN 255
    #define MAX_CONNECTIONS 128
    #define MAX_NAME_LENGTH 30
    
    
    struct JOB_INFO {
    	int socket_ids[MAX_CONNECTIONS];
    	bool end_of_program;
    	int open_cnncts;
    	pthread_mutex_t socketids_changingMutex;
    };
    
    
    struct SERVER_INFO {
        struct sockaddr_in serv_add;
        struct sockaddr_in cli_adr;
        socklen_t cli_len;
        pthread_t write_thread;
    };
    //Global Variables
    FILE* plog_file;
    
    
    void delete_socket(int socketid, struct JOB_INFO* pjob_data);
    void* job_read(void*);
    void* job_write(void*);
    void error(const char* msg);
    void setUpFileStream(int argc, char* argv[]);
    int setUpConnections(struct SERVER_INFO* pserver_data, char* port_numc);
    void enterConnectingLoop(struct SERVER_INFO* pserver_data, struct JOB_INFO* pjob_data, int sockfd);
    
    
    void error(const char* msg){
        perror(msg);
        exit(1);
    }
    
    
    int main(int argc, char* argv[]) {
    	setUpFileStream(argc, argv);
    
    
    	int sock_fd;
        struct JOB_INFO job_data;
        job_data.end_of_program = false;
        job_data.open_cnncts = 0;
    
    
        struct SERVER_INFO server_data;
        //Create mutex
        if(pthread_mutex_init(&(job_data.socketids_changingMutex), NULL) < 0){
            error("Could not initialize Mutex");
        }
        //Initialzing threads and create write_thread
        pthread_create(&server_data.write_thread, NULL, job_write, (void*)&job_data);
    
    
        //Setup for connections
        sock_fd = setUpConnections(&server_data, argv[1]);
        fprintf(plog_file,"Listening...");
        fflush(stdout); //For unknown reason this is needed.
        enterConnectingLoop(&server_data, &job_data, sock_fd);
    
    
        job_data.end_of_program = true;
        close(sock_fd);
    
    
        pthread_join(server_data.write_thread, NULL);
        pthread_mutex_destroy(&job_data.socketids_changingMutex);
        return EXIT_SUCCESS;
    }
    
    
    void* job_read(void * p){
    
    
    	char** pc = (char**)p; //allow pointer arithmetic
        int new_sock_fd = *((int*) (pc[1]));  //Casting
        struct JOB_INFO* pjob_data = ((struct JOB_INFO*) (pc[0])); //Casting
    
    
        ssize_t n; //Error catching variable
        ssize_t m; //Error catching variable
        char buffer[BUFLEN];
        char name[MAX_NAME_LENGTH];
        sprintf(name, "Client %d: ", new_sock_fd);
        fprintf(plog_file, "Reading from %s\n", name);
    
    
        while(!pjob_data->end_of_program){
            bzero(buffer, BUFLEN);
            n = read(new_sock_fd, buffer, BUFLEN);
    
    
            if(n<0){
                printf("Buffer: %s", buffer);
                error("Reading Failed");
            }
            if(n == 0){
                delete_socket(new_sock_fd, pjob_data);
                pthread_exit(NULL);
            }
    
    
            pthread_mutex_lock(&pjob_data->socketids_changingMutex);
            for(int i = 0; i < pjob_data->open_cnncts; i++){
                if(pjob_data->socket_ids[i] == new_sock_fd){
                    continue;
                }
                m = write(pjob_data->socket_ids[i], name, strlen(name));
                if((m < 0)){
                    error("Writing name failed");
                }
                n = write(pjob_data->socket_ids[i], buffer, strlen(buffer));
                if((n < 0)){
                    error("Writing message failed");
                }
            }
            pthread_mutex_unlock(&pjob_data->socketids_changingMutex);
            printf("%s%s", name, buffer);
            fflush(stdout); //For unknown reason this is needed.
        }
        delete_socket(new_sock_fd, pjob_data);
        pthread_exit( NULL );
    }
    
    
    void* job_write(void* args){
        struct JOB_INFO* pjob_data = (struct JOB_INFO*)(args);
        fprintf(plog_file, "Started writing thread...\n");
        ssize_t n; //Error catching variable
        ssize_t m; //Error catching variable
        char buffer[BUFLEN];
        char* name = "Server: \0";
    
    
        while(!pjob_data->end_of_program) {
            fgets(buffer, BUFLEN, stdin);
    
    
            pthread_mutex_lock(&pjob_data->socketids_changingMutex);
            for(int i = 0; i < pjob_data->open_cnncts; i++){
                m = write(pjob_data->socket_ids[i], name, strlen(name));
                if((m < 0)){
                    error("Writing name failed");
                }
                n = write(pjob_data->socket_ids[i], buffer, strlen(buffer));
                if((n < 0)){
                    error("Writing message failed");
                }
            }
            pthread_mutex_unlock(&pjob_data->socketids_changingMutex);
            if(strcmp("Bye\n", buffer) == 0){
                exit(EXIT_SUCCESS);
            }
        }
        pjob_data->end_of_program = true;
        pthread_exit( NULL );
    }
    
    
    void enterConnectingLoop(struct SERVER_INFO* pserver_data, struct JOB_INFO* pjob_data, int sockfd){
        listen(sockfd, MAX_CONNECTIONS);
    
    
        for(pjob_data->open_cnncts = 0; (!pjob_data->end_of_program); /*mutex needs to be set*/ ){
        	void** p = malloc(2*sizeof(void*));
        	p[0] = (void*)pjob_data;
        	p[1] = (void*)&(pjob_data->socket_ids[pjob_data->open_cnncts]);
        	pjob_data->socket_ids[pjob_data->open_cnncts] =
        			accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
        	printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->open_cnncts]));
            pthread_mutex_lock(&(pjob_data->socketids_changingMutex));
            fprintf(plog_file,"Client connected.\n");
            pthread_t thread;
            pthread_create(&thread , NULL, job_read, p);
            pjob_data->open_cnncts++;
            pthread_mutex_unlock(&(pjob_data->socketids_changingMutex));
        }
    }
    
    
    void delete_socket(int socketid, struct JOB_INFO* pjob_data){
        bool found = false;
        for(int i = 0; i < pjob_data->open_cnncts; i++){
            if(found){
            	pjob_data->socket_ids[i-1] = pjob_data->socket_ids[i];
            }
            if(pjob_data->socket_ids[i] == socketid){
                close(socketid);
                found = true;
            }
        }
        if(found){
        	pjob_data->open_cnncts--;
        }
    }
    
    
    inline void setUpFileStream(int argc, char* argv[]){
        printf("Server started...\n");
        if(argc < 2){
            fprintf(stderr, "You must provide a port number");
            exit(EXIT_FAILURE);
        }
        if(argc == 3){
            plog_file = fopen(argv[2], "w");
        } else {
            plog_file = fopen("logfile.txt", "w");
        }
        plog_file = stdout;
        stderr = plog_file;
    }
    
    
    inline int setUpConnections(struct SERVER_INFO* pserver_data, char* port_numc){
        pserver_data->cli_len = sizeof(pserver_data->cli_adr);
        memset(&pserver_data->serv_add, 0, sizeof((pserver_data->serv_add)));
    
    
        uint16_t port_num = (uint16_t)atoi(port_numc);
        pserver_data->serv_add.sin_family = AF_INET;
        pserver_data->serv_add.sin_addr.s_addr = INADDR_ANY;
        pserver_data->serv_add.sin_port = htons(port_num);
    
    
        int sockfd = socket(AF_INET, SOCK_STREAM, 0);
        if(sockfd < 0){
            error("Error opening socket.");
        }
    
    
        if(bind(sockfd, (struct sockaddr*) (&pserver_data->serv_add), sizeof(pserver_data->serv_add)) < 0){
            error("Binding failed.");
        }
        return sockfd;
    }
    And the a Client:

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <unistd.h>
    #include <string.h>
    #include <sys/types.h>
    #include <sys/socket.h>
    #include <netinet/in.h>
    #include <netdb.h>
    #include <pthread.h>
    #include <stdbool.h>
    
    
    #define BUFLEN 255
    
    
    bool endprogram = false;
    int sockfd;
    
    
    void error(const char* msg){
        perror(msg);
        exit(1);
    }
    
    
    void* job_read(void* p){
        char buffer[BUFLEN];
        while(!endprogram){
            bzero(buffer, BUFLEN);
            ssize_t n = read(sockfd, buffer, (BUFLEN));
            if(n < 0){
                error("Error on reading");
            }
            printf("%s", buffer);
            int i = strncmp("Bye", buffer, 3);
            if(i == 0){
                endprogram = true;
                pthread_exit(NULL);
            }
        }
       pthread_exit(NULL);
    }
    
    
    int main(int argc, const char * argv[]) {
        pthread_t readt;
        
        int16_t portnum;
        struct sockaddr_in serveraddr;
        struct hostent* server;
        
        if(argc < 3){
            perror("You shall provide a port and a ip adress");
        }
        portnum = atoi(argv[2]);
        sockfd = socket(AF_INET, SOCK_STREAM, 0);
        if(sockfd < 0){
            error("Error opening socket");
        }
        
        server = gethostbyname(argv[1]);
        if(!server){
            error("No such host");
        }
        
        bzero((char*)&serveraddr, sizeof(serveraddr));
        serveraddr.sin_family = AF_INET;
        bcopy((char *)server->h_addr_list[0], (char *)&serveraddr.sin_addr.s_addr, sizeof(server->h_length));
        serveraddr.sin_port = htons(portnum);
        
        if(connect(sockfd, (struct sockaddr *)&serveraddr, sizeof(serveraddr))<0){
            error("Connection failed");
        }
        
        pthread_create(&readt, NULL, &job_read, NULL);
        
        ssize_t n;
        char buffer[BUFLEN];
        while(!endprogram){
            fgets(buffer, BUFLEN, stdin);
            n = write(sockfd, buffer, strlen(buffer));
            if(n < 0){
                error("Error on writing");
            }
            n = strcmp(buffer, "Bye");
            if(n == 0){
                endprogram = true;
            }
        }
        pthread_join(readt, NULL);
        close(sockfd);
        return 0;
    }
    First let us compile both files and name them Server and Client.

    After compiling start Server like:

    ./Server 127.0.0.1 9999

    and start the Client (in the Terminal ) with:

    ./Client 9999

    Optional: Now start another Client.

    As you see the communication works good. Just type some text and enter.

    If you now close one Client with STRG/CMD+C everything works as wanted.
    But let us instead close the terminal using Mouse...


    WTF WTF

    Presumably the Server reads the last message of the Client over and over...
    But not only this: It also executes code, which is unexplainble. After some time:
    He stops writing the Clients Name and just writes the message, which is completely unexplainable since there is no line of code which does only write a message. Furthermore all other active Clients notice the scenario. After some time, the Server realizes the Connections does not longer exists and loses the connection.

    WHY?

  2. #2
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,537
    There are multiple issues.

    1. You call delete_socket() outside the scope of the mutex.
    Code:
            if(n == 0){
                delete_socket(new_sock_fd, pjob_data);
                pthread_exit(NULL);
            }
    
            //!! oops, too late for delete
            pthread_mutex_lock(&pjob_data->socketids_changingMutex);
    2. You assign a new socket outside the scope of the mutex.
    Code:
        	pjob_data->socket_ids[pjob_data->numConnections] =
        			accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
        	printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->numConnections]));
            //!! oops, too late
            pthread_mutex_lock(&(pjob_data->socketids_changingMutex));
    3. Your threads leak the memory of the supplied parameter
    > void** p = malloc(2*sizeof(void*));
    The thread should call free on this pointer.

    4. Rather than messing with voids and lots of casts, declare a struct.
    Code:
    struct read_thread_params {
        struct JOB_INFO *info;
        int socket;
    };
    
    ///
    void* job_read(void * p){
        struct read_thread_params *rp = p;
    
        // Now you can get say the socket with rp->socket instead of
        // int new_sock_fd = *((int*) (pc[1]));  //Casting
    
        while(!pjob_data->end_of_program){
            ////
        }
    
        free(p);
        pthread_exit( NULL );
    }
    
    
    /// enterConnectingLoop
    s = accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
    struct read_thread_params *rp = malloc( sizeof(*rp) );
    rp->info = pjob_data;
    rp->socket = s;
    
    pthread_mutex_lock(&(pjob_data->socketids_changingMutex));
    pjob_data->socket_ids[pjob_data->numConnections++] = s;
    pthread_mutex_unlock(&(pjob_data->socketids_changingMutex));
    
    pthread_t thread;
    pthread_create(&thread , NULL, job_read, rp);
    5. Possible buffer overflow.
    Code:
            bzero(buffer, BUFLEN);
            n = read(new_sock_fd, buffer, BUFLEN);
    If read() completely fills the buffer with BUFLEN bytes, there won't be a \0 to mark the end of the string.

    Also, bzero is a heavy handed way of doing it.

    Code:
    n = read(new_sock_fd, buffer, BUFLEN-1);  // leave room for a \0
    if ( n > 0 ) buffer[n] = '\0';  // add a \0 on success
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  3. #3
    Registered User
    Join Date
    Feb 2019
    Posts
    67
    Quote Originally Posted by Salem View Post
    There are multiple issues.

    1. You call delete_socket() outside the scope of the mutex.
    Code:
            if(n == 0){
                delete_socket(new_sock_fd, pjob_data);
                pthread_exit(NULL);
            }
    
            //!! oops, too late for delete
            pthread_mutex_lock(&pjob_data->socketids_changingMutex);
    2. You assign a new socket outside the scope of the mutex.
    Code:
            pjob_data->socket_ids[pjob_data->numConnections] =
                    accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
            printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->numConnections]));
            //!! oops, too late
            pthread_mutex_lock(&(pjob_data->socketids_changingMutex));
    3. Your threads leak the memory of the supplied parameter
    > void** p = malloc(2*sizeof(void*));
    The thread should call free on this pointer.

    4. Rather than messing with voids and lots of casts, declare a struct.
    Code:
    struct read_thread_params {
        struct JOB_INFO *info;
        int socket;
    };
    
    ///
    void* job_read(void * p){
        struct read_thread_params *rp = p;
    
        // Now you can get say the socket with rp->socket instead of
        // int new_sock_fd = *((int*) (pc[1]));  //Casting
    
        while(!pjob_data->end_of_program){
            ////
        }
    
        free(p);
        pthread_exit( NULL );
    }
    
    
    /// enterConnectingLoop
    s = accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
    struct read_thread_params *rp = malloc( sizeof(*rp) );
    rp->info = pjob_data;
    rp->socket = s;
    
    pthread_mutex_lock(&(pjob_data->socketids_changingMutex));
    pjob_data->socket_ids[pjob_data->numConnections++] = s;
    pthread_mutex_unlock(&(pjob_data->socketids_changingMutex));
    
    pthread_t thread;
    pthread_create(&thread , NULL, job_read, rp);
    5. Possible buffer overflow.
    Code:
            bzero(buffer, BUFLEN); 
            n = read(new_sock_fd, buffer, BUFLEN);
    If read() completely fills the buffer with BUFLEN bytes, there won't be a \0 to mark the end of the string.

    Also, bzero is a heavy handed way of doing it.

    Code:
    n = read(new_sock_fd, buffer, BUFLEN-1);  // leave room for a \0
    if ( n > 0 ) buffer[n] = '\0';  // add a \0 on success
    Thank you for your support!
    To 2.:

    That is it. I cannot mutex accept() - because everything will be hold back until a client enters

    EDIT: After I fixed everything as you suggested - except the struct-suggestion and the accept() - the bug still exists...
    Code:
    #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 <pthread.h>
    #include <stdbool.h>
    
    
    #define BUFLEN 255
    #define MAX_CONNECTIONS 128
    #define MAX_NAME_LENGTH 30
    
    
    struct JOB_INFO {
        int socket_ids[MAX_CONNECTIONS];
        bool end_of_program;
        int open_cnncts;
        pthread_mutex_t socketids_changingMutex;
    };
    
    
    struct SERVER_INFO {
        struct sockaddr_in serv_add;
        struct sockaddr_in cli_adr;
        socklen_t cli_len;
        pthread_t write_thread;
    };
    //Global Variables
    FILE* plog_file;
    
    
    void delete_socket(int socketid, struct JOB_INFO* pjob_data);
    void* job_read(void*);
    void* job_write(void*);
    void error(const char* msg);
    void setUpFileStream(int argc, char* argv[]);
    int setUpConnections(struct SERVER_INFO* pserver_data, char* port_numc);
    void enterConnectingLoop(struct SERVER_INFO* pserver_data, struct JOB_INFO* pjob_data, int sockfd);
    
    
    void error(const char* msg){
        perror(msg);
        exit(1);
    }
    
    
    int main(int argc, char* argv[]) {
        setUpFileStream(argc, argv);
    
    
        int sock_fd;
        struct JOB_INFO job_data;
        job_data.end_of_program = false;
        job_data.open_cnncts = 0;
    
    
        struct SERVER_INFO server_data;
        //Create mutex
        if(pthread_mutex_init(&(job_data.socketids_changingMutex), NULL) < 0){
            error("Could not initialize Mutex");
        }
        //Initialzing threads and create write_thread
        pthread_create(&server_data.write_thread, NULL, job_write, (void*)&job_data);
    
    
        //Setup for connections
        sock_fd = setUpConnections(&server_data, argv[1]);
        fprintf(plog_file,"Listening...");
        fflush(stdout); //For unknown reason this is needed.
        enterConnectingLoop(&server_data, &job_data, sock_fd);
    
    
        job_data.end_of_program = true;
        close(sock_fd);
    
    
        pthread_join(server_data.write_thread, NULL);
        pthread_mutex_destroy(&job_data.socketids_changingMutex);
        return EXIT_SUCCESS;
    }
    
    
    void* job_read(void * p){
    
    
        char** pc = (char**)p; //allow pointer arithmetic
        int new_sock_fd = *((int*) (pc[1]));  //Casting
        struct JOB_INFO* pjob_data = ((struct JOB_INFO*) (pc[0])); //Casting
    
    
        ssize_t n; //Error catching variable
        ssize_t m; //Error catching variable
        char buffer[BUFLEN];
        char name[MAX_NAME_LENGTH];
        sprintf(name, "Client %d: ", new_sock_fd);
        fprintf(plog_file, "Reading from %s\n", name);
    
    
        while(!pjob_data->end_of_program){
          
                     n = read(new_sock_fd, buffer, BUFLEN-1);        if ( n > 0 ) buffer[n] = '\0';  // add a \0 on success
            if(n<0){
                printf("Buffer: %s", buffer);
                error("Reading Failed");
            }
            pthread_mutex_lock(&pjob_data->socketids_changingMutex);
            if(n == 0){
                delete_socket(new_sock_fd, pjob_data);
                pthread_exit(NULL);
            }
    
    
            for(int i = 0; i < pjob_data->open_cnncts; i++){
                if(pjob_data->socket_ids[i] == new_sock_fd){
                    continue;
                }
                m = write(pjob_data->socket_ids[i], name, strlen(name));
                if((m < 0)){
                    error("Writing name failed");
                }
                n = write(pjob_data->socket_ids[i], buffer, strlen(buffer));
                if((n < 0)){
                    error("Writing message failed");
                }
            }
            pthread_mutex_unlock(&pjob_data->socketids_changingMutex);
            printf("%s%s", name, buffer);
            fflush(stdout); //For unknown reason this is needed.
        }
        delete_socket(new_sock_fd, pjob_data);
        free(p);
        pthread_exit( NULL );
    }
    
    
    void* job_write(void* args){
        struct JOB_INFO* pjob_data = (struct JOB_INFO*)(args);
        fprintf(plog_file, "Started writing thread...\n");
        ssize_t n; //Error catching variable
        ssize_t m; //Error catching variable
        char buffer[BUFLEN];
        char* name = "Server: \0";
    
    
        while(!pjob_data->end_of_program) {
            fgets(buffer, BUFLEN, stdin);
    
    
            pthread_mutex_lock(&pjob_data->socketids_changingMutex);
            for(int i = 0; i < pjob_data->open_cnncts; i++){
                m = write(pjob_data->socket_ids[i], name, strlen(name));
                if((m < 0)){
                    error("Writing name failed");
                }
                n = write(pjob_data->socket_ids[i], buffer, strlen(buffer));
                if((n < 0)){
                    error("Writing message failed");
                }
            }
            pthread_mutex_unlock(&pjob_data->socketids_changingMutex);
            if(strcmp("Bye\n", buffer) == 0){
                exit(EXIT_SUCCESS);
            }
        }
        free(args);
        pjob_data->end_of_program = true;
        pthread_exit( NULL );
    }
    
    
    void enterConnectingLoop(struct SERVER_INFO* pserver_data, struct JOB_INFO* pjob_data, int sockfd){
        listen(sockfd, MAX_CONNECTIONS);
    
    
        for(pjob_data->open_cnncts = 0; (!pjob_data->end_of_program); /*mutex needs to be set*/ ){
            void** p = malloc(2*sizeof(void*));
            p[0] = (void*)pjob_data;
            p[1] = (void*)&(pjob_data->socket_ids[pjob_data->open_cnncts]);
            pjob_data->socket_ids[pjob_data->open_cnncts] =
                    accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
            pthread_mutex_lock(&(pjob_data->socketids_changingMutex)); //Note you cannot mutex the accept()
            printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->open_cnncts]));
            fprintf(plog_file,"Client connected.\n");
            pthread_t thread;
            pthread_create(&thread , NULL, job_read, p);
            pjob_data->open_cnncts++;
            pthread_mutex_unlock(&(pjob_data->socketids_changingMutex));
        }
    }
    
    
    void delete_socket(int socketid, struct JOB_INFO* pjob_data){
        bool found = false;
        for(int i = 0; i < pjob_data->open_cnncts; i++){
            if(found){
                pjob_data->socket_ids[i-1] = pjob_data->socket_ids[i];
            }
            if(pjob_data->socket_ids[i] == socketid){
                close(socketid);
                found = true;
            }
        }
        if(found){
            pjob_data->open_cnncts--;
        }
    }
    
    
    inline void setUpFileStream(int argc, char* argv[]){
        printf("Server started...\n");
        if(argc < 2){
            fprintf(stderr, "You must provide a port number");
            exit(EXIT_FAILURE);
        }
        if(argc == 3){
            plog_file = fopen(argv[2], "w");
        } else {
            plog_file = fopen("logfile.txt", "w");
        }
        plog_file = stdout;
        stderr = plog_file;
    }
    
    
    inline int setUpConnections(struct SERVER_INFO* pserver_data, char* port_numc){
        pserver_data->cli_len = sizeof(pserver_data->cli_adr);
        memset(&pserver_data->serv_add, 0, sizeof((pserver_data->serv_add)));
    
    
        uint16_t port_num = (uint16_t)atoi(port_numc);
        pserver_data->serv_add.sin_family = AF_INET;
        pserver_data->serv_add.sin_addr.s_addr = INADDR_ANY;
        pserver_data->serv_add.sin_port = htons(port_num);
    
    
        int sockfd = socket(AF_INET, SOCK_STREAM, 0);
        if(sockfd < 0){
            error("Error opening socket.");
        }
    
    
        if(bind(sockfd, (struct sockaddr*) (&pserver_data->serv_add), sizeof(pserver_data->serv_add)) < 0){
            error("Binding failed.");
        }
        return sockfd;
    }
    Last edited by SuchtyTV; 07-17-2019 at 11:52 AM.

  4. #4
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,537
    1. Sloppy formatting at line 104.

    2. At line 111, you exit the thread whilst still holding the mutex.

    3. You're still writing outside the mutex.
    Code:
            pjob_data->socket_ids[pjob_data->open_cnncts] =
                    accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
            pthread_mutex_lock(&(pjob_data->socketids_changingMutex)); //Note you cannot mutex the accept()
    "You cannot mutex the accept".
    True, but you could use a temporary variable like I showed you, to hold the new socket until you're ready to lock/write/unlock.

    4. inline....
    Yeah, inlining things just once doesn't buy you much at all.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  5. #5
    Registered User
    Join Date
    Feb 2019
    Posts
    67
    Quote Originally Posted by Salem View Post
    "You cannot mutex the accept".
    True, but you could use a temporary variable like I showed you, to hold the new socket until you're ready to lock/write/unlock.
    Why did not I came up with this idea myself...

    Code:
    void enterConnectingLoop(struct SERVER_INFO* pserver_data, struct JOB_INFO* pjob_data, int sockfd){
        listen(sockfd, MAX_CONNECTIONS);
    
    
        for(pjob_data->open_cnncts = 0; (!pjob_data->end_of_program); /*mutex needs to be set*/ ){
            void** p = malloc(2*sizeof(void*));
            int temp = accept(sockfd, (struct sockaddr*) &pserver_data->cli_adr, &pserver_data->cli_len);
            pthread_mutex_lock(&(pjob_data->socketids_changingMutex)); //Note you cannot mutex the accept()
            pjob_data->socket_ids[pjob_data->open_cnncts] = temp;
            p[0] = (void*)pjob_data;
            p[1] = (void*)&(pjob_data->socket_ids[pjob_data->open_cnncts]);
            printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->open_cnncts]));
            fprintf(plog_file,"Client connected.\n");
            pthread_t thread;
            pthread_create(&thread , NULL, job_read, p);
            pjob_data->open_cnncts++;
            pthread_mutex_unlock(&(pjob_data->socketids_changingMutex));
        }
    }
    Unfortunately, the bug still exists... I know that inline does not buys much, but it does not cost much either, does it?
    Last edited by SuchtyTV; 07-18-2019 at 01:38 AM.

  6. #6
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,537
    Code:
            pjob_data->socket_ids[pjob_data->open_cnncts] = temp;
            p[0] = (void*)pjob_data;
            p[1] = (void*)&(pjob_data->socket_ids[pjob_data->open_cnncts]);
            printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->open_cnncts]));
            fprintf(plog_file,"Client connected.\n");
            pthread_t thread;
            pthread_create(&thread , NULL, job_read, p);
            pjob_data->open_cnncts++;
    Do you really want the thread to be running between adding the socket to socket_ids, and then incrementing open_cnncts?
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

  7. #7
    Registered User
    Join Date
    Feb 2019
    Posts
    67
    Quote Originally Posted by Salem View Post
    Code:
            pjob_data->socket_ids[pjob_data->open_cnncts] = temp;
            p[0] = (void*)pjob_data;
            p[1] = (void*)&(pjob_data->socket_ids[pjob_data->open_cnncts]);
            printf("SOCKETFD %d\n", (pjob_data->socket_ids[pjob_data->open_cnncts]));
            fprintf(plog_file,"Client connected.\n");
            pthread_t thread;
            pthread_create(&thread , NULL, job_read, p);
            pjob_data->open_cnncts++;
    Do you really want the thread to be running between adding the socket to socket_ids, and then incrementing open_cnncts?
    What is your intension, I want to "assign" to socket_ids[0] and then after all this done, there is one connection more?

  8. #8
    and the hat of int overfl Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    37,537
    Well I see your difficulty, because you have this.

    > p[1] = (void*)&(pjob_data->socket_ids[pjob_data->open_cnncts]);

    Maybe implement point 4 in #2
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Scanf behaves weirdly in for loop.Please help.
    By zeusvj in forum C Programming
    Replies: 9
    Last Post: 12-24-2015, 02:04 AM
  2. Getch behaves as key is previous key.
    By #define whatevr in forum C Programming
    Replies: 13
    Last Post: 03-13-2011, 04:15 PM
  3. ReadConsoleInput behaves bad.
    By antex in forum Windows Programming
    Replies: 2
    Last Post: 10-25-2005, 12:02 PM
  4. MSVC behaves strange
    By ripper079 in forum C++ Programming
    Replies: 4
    Last Post: 10-28-2003, 08:34 PM
  5. char* behaves strangely
    By Seron in forum C++ Programming
    Replies: 7
    Last Post: 01-12-2002, 04:04 PM

Tags for this Thread