socket programming: 'accept: Bad file descriptor'

Question

I'm trying to write a game that will let multiple clients connect and play - below is the relevant code (it's very messy - cleaning up later):

Edit: I realized it's a lot of scrolling... the crash occurs towards the end of the game during:

    std::cout << black_hits << " black hits & " << white_hits
                << " white hits.\n";

    if (black_hits == 4) {
        std::cout << "you won!\n";
        std::cin.ignore().get();
        close(client); //<<<< CRASH HERE
        return 0;
    }

Not really a crash I guess... but close enough :)

---

#include <iostream>
#include <vector>
#include <algorithm>
#include <cstdlib>
#include <ctime>
#include <string>
#include <sstream>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <string.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netdb.h>
#include <arpa/inet.h>
#include <sys/wait.h>
#include <signal.h>

#define BACKLOG 10
#define MAXDATASIZE 100

typedef enum {RED,GREEN,BLUE,YELLOW,ORANGE} color;
int StartMasterMind(int client, sockaddr_storage addr_in); 

struct msgstruct {
        int length;
        char* send_data;
};

void sigchld_handler(int s)
{
    while(waitpid(-1, NULL, WNOHANG) > 0);
}

// get sockaddr, IPv4 or IPv6:
void *get_in_addr(struct sockaddr *sa)
{
    if (sa->sa_family == AF_INET) {
        return &(((struct sockaddr_in*)sa)->sin_addr);
    }

    return &(((struct sockaddr_in6*)sa)->sin6_addr);
}

int tcp_connect(const char *serv, const char *host = NULL)
{
    int sockfd, new_fd;  // listen on sock_fd, new connection on new_fd
    struct addrinfo hints, *servinfo, *p;
    struct sockaddr_storage their_addr; // connector's address information
    socklen_t sin_size;
    struct sigaction sa;
    int yes=1;
    char s[INET6_ADDRSTRLEN];
    int rv;

    memset(&hints, 0, sizeof hints);
    hints.ai_family = AF_UNSPEC;
    hints.ai_socktype = SOCK_STREAM;
    hints.ai_flags = AI_PASSIVE; // use my IP

    if ((rv = getaddrinfo(host, serv, &hints, &servinfo)) != 0) {
        fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rv));
        return 1;
    }

    // loop through all the results and bind to the first we can
    for(p = servinfo; p != NULL; p = p->ai_next) {
        if ((sockfd = socket(p->ai_family, p->ai_socktype,
                p->ai_protocol)) == -1) {
            perror("server: socket");
            continue;
        }

        if (setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &yes,
                sizeof(int)) == -1) {
            perror("setsockopt");
            exit(1);
        }

        if (bind(sockfd, p->ai_addr, p->ai_addrlen) == -1) {
            close(sockfd);
            perror("server: bind");
            continue;
        }

        break;
    }

    if (p == NULL)  {
        fprintf(stderr, "server: failed to bind\n");
        return 2;
    }

    freeaddrinfo(servinfo); // all done with this structure

    if (listen(sockfd, BACKLOG) == -1) {
        perror("listen");
        exit(1);
    }

    sa.sa_handler = sigchld_handler; // reap all dead processes
    sigemptyset(&sa.sa_mask);
    sa.sa_flags = SA_RESTART;
    if (sigaction(SIGCHLD, &sa, NULL) == -1) {
        perror("sigaction");
        exit(1);
    }

    printf("server: waiting for connections...\n");

    while(1) {  // main accept() loop
        sin_size = sizeof their_addr;
        new_fd = accept(sockfd, (struct sockaddr *)&their_addr, &sin_size);
        if (new_fd == -1) {
            perror("accept");
            continue;
        }

        inet_ntop(their_addr.ss_family,
            get_in_addr((struct sockaddr *)&their_addr),
            s, sizeof s);
        printf("server: got connection from %s\n", s);

        if (!fork()) { // this is the child process
            close(sockfd); // child doesn't need the listener
            //if (send(new_fd, "Hello, world!", 13, 0) == -1)
            //    perror("send");
            //close(new_fd);
            StartMasterMind(new_fd,their_addr);
           // exit(0);
        }
        close(new_fd);  // parent doesn't need this
    }

    return 0;
}

void InitializeGame(const char* port)
{
    tcp_connect(port);
}

std::vector<color> GetInputAsColorMap(char* input)
{
[...]//redacted for clarity
}

int StartMasterMind(int client, sockaddr_storage addr_in) 
{
    struct msgstruct message;   
    struct sockaddr_storage their_addr = addr_in;
    socklen_t addr_len;

    message.send_data = "Welcome to ... M A S T E R M I N D.\n";
    message.length = strlen(message.send_data);
    send(client, message.send_data, message.length, 0);

[...]//redacted for clarity

    if (strcmp(theValue, "random") == 0 || strcmp(theValue, "Random") == 0)
    {
[...]//redacted for clarity
    }
    else
    {
[...]//redacted for clarity
    }


    char* buf;

    for (int i = 0; i < 8; ++i) {
        std::vector<color> current_try(4);
        int black_hits = 0, white_hits = 0;
        std::vector<int> correctColorIndex;
        std::vector<int> correctColor;

        bool exclude[4] = {false};
        std::cout << "test\n"; 
        message.send_data = "Please enter your guess:  ";
        message.length = strlen(message.send_data);
        send(client, message.send_data, message.length, 0);

        addr_len = sizeof their_addr;
        std::cout << "addr_len: " << addr_len << std::endl;     

        recvfrom(client, buf, MAXDATASIZE-1, 0, (struct sockaddr *)&their_addr, &addr_len);

        current_try = GetInputAsColorMap(buf);
        std::cout << "the buffer: " << buf << std::endl;
        std::cout << "current_try: " << current_try[0] << current_try[1] << current_try[2] << current_try[3] << std::endl;

[...]//redacted for clarity

        std::cout << black_hits << " black hits & " << white_hits
                    << " white hits.\n";

        if (black_hits == 4) {
            std::cout << "you won!\n";
            std::cin.ignore().get();
            close(client); //<<<< CRASH HERE
            return 0;
        }
    }   

[...]//redacted for clarity
}

int main(int argc, char** argv)
{
    InitializeGame(argv[1]);
    return 0;
}

---

Here is sample output:

server: waiting for connections...
server: got connection from 127.0.0.1
value or random: 
1122
test
addr_len: 128
the buffer: 1123�
current_try: 1123
3 black hits & 0 white hits.
test
addr_len: 128
the buffer: 1223�
current_try: 1223
2 black hits & 1 white hits.
test
addr_len: 128
the buffer: 1122�
current_try: 1122
4 black hits & 0 white hits.
you won!
accept: Bad file descriptor
accept: Bad file descriptor
accept: Bad file descriptor
... // continuously, hundreds of times

---

I'm very new to socket programming; could someone give me a hand? This crashes with or without trying to close(client) at the end of the game.

Solution 1

I think when the child process is wrapping back to start of while(1) loop, it tries to accept a connection with server socket descriptor = "sockfd" which you already closed for the child:

if (!fork()) { // this is the child process
            close(sockfd);
....
}

Try this link to read as how to terminate the child process after its work is complete.

Solution 2

That message means that you're calling accept() on an invalid file descriptor, i.e. probably one that you've closed.

Comments

[Don't You Worry Child]

If it shows accept: Bad file descriptor, How can "crash" be at close(client). It surely means problem at accept.

[MrDuk]

You're right, sorry - I'm super new to this, and haven't yet figured out how to debug effectively under gdb -- however, accept: should have given it away I suppose.. :)

[MrDuk]

[edited] -- Sike, I lied. Looks like it's totally that I was closing the sockfd as @MadHatter suggested.

[pmverma]

What I think is same with above answer, probably you may left the server socket opened. I this case, all of your children will try to accept the connections. Or you may like to exit the child process once it has finished.

[MrDuk]

I just tested commenting out close(sockfd) and it seems to have worked! However, this seems bad... at what point do I want to close the sockfd?

[pmverma]

because the server socket is accepting the connection, you should close the socket in last part of the code. like before returning from function.

[MrDuk]

Ok, so would it be safe to assume that for an application this small I don't need to worry about closing the socket (because at that point, my program is going to exit most likely)?

[pmverma]

As far as I know, if you do not close a file descriptor-socket in your case, the kernel will close all those on program exiting. But it is always good to close them.