How to fix "Error reading from socket - bad file descriptor" while in thread on Linux using C?

Question

I am setting up a server-client application(chat) and i have encountered a problem with sockets/threads. Both server and client run on Linux.

I want the server to be able to serve multiple users, and to do it I want to use threads from pthread.h, so every logged user has his own thread.

This is the server: main.c

#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <pthread.h>
#include "manazerUctov.h"

int pocetVlakienZOP = 0;

int main(int argc, char *argv[])
{
    pthread_t* vlaknaZiadosti = 
    (pthread_t*)malloc(sizeof(pthread_t)*POCET_UZIVATELOV);

    int sockfd, newsockfd;
    socklen_t cli_len;
    struct sockaddr_in serv_addr, cli_addr;
    int n;

    // Program
    if (argc < 2) {
        fprintf(stderr, "usage %s port\n", argv[0]);
        return 1;
    }

    bzero((char*) &serv_addr, sizeof (serv_addr));
    serv_addr.sin_family = AF_INET;
    serv_addr.sin_addr.s_addr = INADDR_ANY;
    serv_addr.sin_port = htons(atoi(argv[1]));

    sockfd = socket(AF_INET, SOCK_STREAM, 0);
    if (sockfd < 0) {
        perror("Error creating socket\n");
        return 1;
    }
    printf("Socket OK\n");

    if (bind(sockfd, (struct sockaddr*) &serv_addr, sizeof (serv_addr)) < 0) {
        perror("Error binding socket address\n");
        return 2;
    }
    printf("Bind OK\n");

    listen(sockfd, 5);
    while (1) {

        cli_len = sizeof (cli_addr);

        newsockfd = accept(sockfd, (struct sockaddr*) &cli_addr, &cli_len);
        if (newsockfd < 0) {
            perror("ERROR on accept\n");
            return 3;
        }
        printf("Accept OK\n");
        client_data* cdata = (client_data*) malloc(sizeof (client_data));
        cdata->socket = newsockfd;

        pthread_t vlakno;
        vlaknaZiadosti[pocetVlakienZOP++] = vlakno;
        pthread_create(&vlakno, NULL, &serveClient, &cdata);
    }

    close(sockfd);
    return 0;
}

manazerUctov.h:

#ifndef MANAZERUCTOV_H
#define MANAZERUCTOV_H

#ifndef POCET_UZIVATELOV
#define POCET_UZIVATELOV 256
#endif

#ifdef __cplusplus
extern "C" {
#endif

typedef struct {
    char *buffer;
    char* msg;                         
    int socket;
} client_data;

void* serveClient(void* pdata);

#ifdef __cplusplus
}
#endif

#endif /* MANAZERUCTOV_H */

manazerUctov.c Here, the read function is giving error:

#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include "manazerUctov.h"

void* serveClient(void *pdata) {
    char buffer[256];
    client_data* client = (client_data*)pdata;
    bzero(buffer, 256);

    int n = read(client->socket, buffer, 255);
    if (n < 0) {
        perror("Error reading from socket\n");
        return NULL;
    }

    printf("Read from socket\n");
    close(client->socket);
}

When the client sends socket, the server accepts it and creates a thread, but I get Error reading from socket: bad file descriptor. And the program is stuck, it won't even exit. And as far as I am concerned the sockets aren't closed yet. Before I implemented threads, reading from sockets worked correctly.

EDIT: Original code is 500 lines long, so I trimmed it and the former error was no more, although another error was &cdata in pthread_create . Removing & solved the problem in both trimmed and original code and server is working.

Answer 1

Your pthread_create function call is incorrect.

This passes a pointer to the client_data pointer and not what the pointer points to:

pthread_create(&my_thread, NULL, &my_function, &cdata);

What you want is:

pthread_create(&my_thread, NULL, &my_function, cdata);

So, instead of getting cdata->socket, you'll get the lower 32 bits of the pointer value as your file descriptor.

---

Some additional notes:

Because you have a loop scoped pthread_t my_thread;, this disappears on each iteration of the loop, so the main thread won't be able to do a pthread_join on it.

So, when your thread function exits, it will be in an unjoined (i.e. zombie) state.

Also, on each iteration, each pthread_create call may point to my_thread at the same address. I'm not sure what effect this will have on other thread primitives, but I'd be wary.

You may want to add the pthread_t mythread; to your allocated struct. That way, it is guaranteed to have a unique address. And, the main thread can keep track of the structs it has allocated.

Another way to deal with this is to have your thread function do a pthread_detach at the top [or add the appropriate attributes to the pthread_create call]

This may be easier cleaner.

Also, be sure that your thread function closes its socket and does a free on the struct pointer just before exiting.