Thread: Unix Server reads wrongly

Hey,

given this 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>


#define BUFLEN 255
#define MAX_CONNECTIONS 128
#define TRUE 1
#define FALSE 0


void* job_read(void * p);
void* job_write(void*);


//Global Variables
FILE* plogfile;
int socket_ids[MAX_CONNECTIONS];
char endprogramm = FALSE;
int open_cnncts = 0;
pthread_mutex_t mutex;


void error(const char* msg){
    perror(msg);
    exit(1);
}


int main(int argc, char* argv[]) {
    if(argc < 2){
        fprintf(stderr, "You must provide a port number");
        exit(EXIT_FAILURE);
    }
    if(argc == 3){
        plogfile = fopen(argv[2], "w");
    } else {
        plogfile = fopen("logfile.txt", "w");
    }
    stderr = plogfile;
    int sockfd, portnum;
    
    //Create nmutthread
    if(pthread_mutex_init(&mutex, NULL)<0){
        error("Could not initialize Mutex");
    }
    //Initialzing threads and create writethread
    pthread_t readthreads[MAX_CONNECTIONS];
    pthread_t writethread;
    pthread_create(&writethread, NULL, job_write, NULL);
    
    //Setup for connections
    struct sockaddr_in serv_add, cli_adr;
    socklen_t clilen;
    clilen = sizeof(cli_adr);
    bzero((char*)&serv_add, sizeof(struct sockaddr_in));
    
    portnum = atoi(argv[1]);
    serv_add.sin_family = AF_INET;
    serv_add.sin_addr.s_addr = INADDR_ANY;
    serv_add.sin_port = htons(portnum);
    
    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    if(sockfd < 0){
        error("Error opening socket.");
    }


    //Bind listening
    if(bind(sockfd, (struct sockaddr*) (&serv_add), sizeof(serv_add)) < 0){
        error("Binding failed.");
    }
    for(open_cnncts = 0; (!endprogramm) & (open_cnncts < MAX_CONNECTIONS); open_cnncts++){
        fprintf(plogfile,"Listening....");
        listen(sockfd, MAX_CONNECTIONS);
        socket_ids[open_cnncts] = accept(sockfd, (struct sockaddr*) &cli_adr, &clilen);
        fprintf(plogfile,"Client connected.\n");
        pthread_create(&readthreads[open_cnncts] , NULL, job_read, (void*)&socket_ids[open_cnncts]);
    }
    endprogramm = TRUE;
    close(sockfd);
    for(; open_cnncts != 0; open_cnncts--){
        close(socket_ids[open_cnncts]);
        pthread_join(readthreads[open_cnncts], NULL);
    }
    pthread_join(writethread, NULL);
    pthread_mutex_destroy(&mutex);
    return 0;


}


void* job_read(void * p){
    int* socketp = (int*)p;
    int newsockfd = (*socketp);
    size_t n;
    char buffer[BUFLEN];
    while(!endprogramm){
        bzero(buffer, BUFLEN);
        n = read(newsockfd, buffer, BUFLEN);
        if(!n){
            error("Reading Failed");
        }
        pthread_mutex_lock(&mutex);
        for(int i = 0; i < open_cnncts; i++){
            if(socket_ids[i] == newsockfd)continue;
            n = write(socket_ids[i], buffer, strlen(buffer));
            if(n < 0){
                error("Writing failed");
            }
        }
        pthread_mutex_unlock(&mutex);
        printf("Client: %s\n", buffer);
    }
    return NULL;
}


void* job_write(void* args){
    fprintf(plogfile, "Started writing thread...\n");
    size_t n;
    char buffer[BUFLEN];
    while(!endprogramm) {
        bzero(buffer, BUFLEN);
        fgets(buffer, BUFLEN, stdin);


        pthread_mutex_lock(&mutex);
        for(int i = 0; i < open_cnncts; i++){
            n = write(socket_ids[i], buffer, strlen(buffer));
            if(n < 0){
                error("Writing failed");
            }
        }
        printf("Server: %s\n",buffer);
        pthread_mutex_unlock(&mutex);
        if(strcmp("Bye", buffer) == 0){
            break;
        }
    }
    endprogramm = TRUE;
    return NULL;
}

and this 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>


#define BUFLEN 255
#define TRUE 1
#define FALSE 0


char endprogram = 0;
int sockfd;




void error(const char* msg){
    perror(msg);
    exit(1);
}


void* job_read(void* p){
    char buffer[BUFLEN];
    while(!endprogram){
        bzero(buffer, BUFLEN);
        size_t n = read(sockfd, buffer, (BUFLEN));
        if(n < 0){
            error("Error on reading");
        }
        printf("Server: %s", buffer);
        int i = strncmp("Bye", buffer, 3);
        if(i == 0){
            endprogram = TRUE;
            return NULL;
        }
    }
    return NULL;
}


void* job_write(void* p){
    return NULL;
}


int main(int argc, const char * argv[]) {
    pthread_t readt;
    pthread_t writet;
    
    int sockfd, 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, (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);
    pthread_create(&writet, NULL, &job_write, NULL);
    
    size_t n;
    char buffer[BUFLEN];
    while(!endprogram){
        bzero(buffer, BUFLEN);
        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;
}

something unexplainable happens. If the Server reads a message from the client, either the terminal output is empty and it correctly prints down:

"Client: message" in the stdout, without sending back the message OR the stdout is not empty and he is sending back the message without printing down: "Client: message"

I suppose the error is somewhere here:

Code:

void* job_read(void * p){    int* socketp = (int*)p;
    int newsockfd = (*socketp);
    size_t n;
    char buffer[BUFLEN];
    while(!endprogramm){
        bzero(buffer, BUFLEN);
        n = read(newsockfd, buffer, BUFLEN);
        if(!n){
            error("Reading Failed");
        }
        pthread_mutex_lock(&mutex);
        for(int i = 0; i < open_cnncts; i++){
            if(socket_ids[i] == newsockfd)continue;
            n = write(socket_ids[i], buffer, strlen(buffer));
            if(n < 0){
                error("Writing failed");
            }
        }
        pthread_mutex_unlock(&mutex);
        printf("Client: %s\n", buffer);
    }
    return NULL;
}

EXAMPLE:

Terminal 1: Terminal 2:
./Server 9999 | ./Client 127.0.0.1 9999
... | Hi
Client Hi | ...
... | Hi again!
... | Server: Hi again!

Thank you!

I recommend not using read() and write() with sockets. Instead, use recv() and send(). And pay attention to the behavior of the functions... read(), for example, can return -1 in case of errors.

Originally Posted by flp1969

I recommend not using read() and write() with sockets. Instead, use recv() and send(). And pay attention to the behavior of the functions... read(), for example, can return -1 in case of errors.

To the -1:

I am wondering, cause my compiler demands size_t for n and as far as I know this cannot become smaller -1...

To the recv/send:

Which flags should I use?

Originally Posted by SuchtyTV

To the -1:

I am wondering, cause my compiler demands size_t for n and as far as I know this cannot become smaller -1...

The read() prototype:

Code:

ssize_t read(int fildes, void *buf, size_t nbyte);

Notice the return type: ssize_t (with two 's').

Originally Posted by SuchtyTV

To the recv/send:

Which flags should I use?

Read the manpages for recv and send.

The way it is, this will never report an error:

Code:

size_t n = read(sockfd, buffer, (BUFLEN));
if(n < 0){
  error("Error on reading");
}

That's why I said to you to pay attention to the functions (and types) behaviors.

And there are some errors as well... In client, sockfd is inicialized inside main() - locally, but your job_read() thread function uses the global, uninitialized, var.... Where are the synchronization calls when dealing with 'endprogram' global var between threads? Do you know that fgets() will insert '\n' at the end of the buffer if there is space enough? Where are the error checking for calls like pthread_create()? Why are you using obsolete functions as gethostbyname() and atoi()?

And, please, learn how to use a debugger (gdb) instead of simply saying: "the bug still exists".