accept function loop in thread c++ socket

Question

I have 2 questions:

first - I try to connect one client to server each time. (the accept function is in thread) but the connection fail in accept - return always -1 (instead of zero).

the second question - how I can add timeout for to accepting client?

server.h

class Server{
public:
    void open(int port, object&);
    void startThreadOPeration();
    void stop(){};
    ~Server();
private:
    sockaddr_in address;
    int socketfd;
    Object* obj; 
    void startThread();
    std::thread acceptClient;
};

server.cpp

void Server::open(int port,Object& obj) {
    this->obj = &obj;
    int socketfd = socket(AF_INET, SOCK_STREAM, 0);
    if (socketfd == -1) {
        throw "Could not create a socket";
    }
    this->socketfd = socketfd;

    sockaddr_in address;
    address.sin_family = AF_INET;
    address.sin_addr.s_addr = INADDR_ANY; 
    address.sin_port = htons(port);
    this->address = address;

    if (bind(socketfd, (struct sockaddr *) &this->address, sizeof(this->address)) == -1) {
        throw "Could not bind the socket to an IP";
    }


    if (listen(this->socketfd, 5) == -1) { 
        throw "Error during listening command";
    } else {
        std::cout<<"Server is now listening ..."<<std::endl;
    }


    startThread();
}


void Server::startThreadOPeration() {
    while (!side_server::stop) {
        // accepting a client
        int client_socket = accept(this->socketfd, (struct sockaddr *) &this->address,
                                   (socklen_t *) &this->address); // Return -1
        if (client_socket == -1) {
            throw "Error accepting client";
        }
        //do operation
    }



void Server::startThread() {



 acceptClient = std::thread(&Server::startThreadOPeration,this);
}

Server::~Server(){
    close(this->socketfd);
    if(acceptClient.joinable()){
        this->acceptClient.join();
    }
}

thank you for help!

Answer 1

accept() takes 3 parameters - a socket, a pointer to a socket address structure, and a pointer to a socklen_t containing the maximum length of the given socket address structure. On success, the socklen_t will be updated with the actual length of the socket structure.

Unfortunately, your code calls accept() with the address of the this->address variable in the 3rd parameter rather than the address of a valid socklen_t variable. Odds are good this causes this->address to contain what will appear to be complete nonsense when accept() updates what it thinks is a socklen_t but really isn't.

void Server::startThreadOPeration() {
    while (!side_server::stop) {
        // accepting a client
        socklen_t len = sizeof(this->address); // max size of address structure
        int client_socket = accept(this->socketfd, 
                                   (struct sockaddr *) &this->address, 
                                   &tlen); // supply valid max length
        if (client_socket == -1) {
            throw "Error accepting client";
        }
        //do operation
    }

accept() doesn't do timeouts easily, but select() does:

void Server::startThreadOPeration() {
    while (!side_server::stop) {
        fd_set readfds;
        struct timeval tv;
        int result;

        FD_ZERO(&readfds);

        tv.tv_sec = timeout_s; // number of seconds to wait here
        tv.tv_usec = timeout_us; // number of additional us to wait here

        FD_SET(this->socketfd, &readfds);
        result = select(this->socketfd + 1, &readfds, NULL, NULL, &tv);
        if (result > 0) 
        {
            // cheating a bit here since the only thing in readfds will be this->socketfd
            // normally you should check what of many file descriptors is set.
            socklen_t len = sizeof(this->address);
            int client_socket = accept(this->socketfd, 
                                       (struct sockaddr *) &this->address, 
                                        &tlen); // supply valid max length
            if (client_socket == -1) {
                throw "Error accepting client";
            }
            //do operation
        }
        else
        {
            // handle any failure except timeout here 
        }
    }
}