Multithreaded server don't switch between threads cpp

Question

I'm trying to make a multithreaded server, but for some reason, the threads of my server aren't switching. Only the last thread that was create is running, the other threads aren't running.

This is the code of the main server:

void Server::serve(int port)
{

    struct sockaddr_in sa = { 0 };

    sa.sin_port = htons(port); // port that server will listen for
    sa.sin_family = AF_INET;   // must be AF_INET
    sa.sin_addr.s_addr = INADDR_ANY;    // when there are few ip's for the machine. We will use always "INADDR_ANY"

    // Connects between the socket and the configuration (port and etc..)
    if (bind(_serverSocket, (struct sockaddr*)&sa, sizeof(sa)) == SOCKET_ERROR)
        throw std::exception(__FUNCTION__ " - bind");

    std::cout << "binded" << std::endl;

    // Start listening for incoming requests of clients
    if (listen(_serverSocket, SOMAXCONN) == SOCKET_ERROR)
        throw std::exception(__FUNCTION__ " - listen");
    std::cout << "Listening on port " << port << std::endl;

    while (true)
    {
        SOCKET client_socket = accept(_serverSocket, NULL, NULL);
        if (client_socket == INVALID_SOCKET)
            throw std::exception(__FUNCTION__);

        std::cout << "Accepting clients..." << std::endl;
        std::cout << "Client accepted" << std::endl;

        std::thread newClientThread(&Server::clientThread, this, std::ref(client_socket));  // Make thread of client
        newClientThread.detach();
    }
}

This is the thread of the client:

void Server::clientThread(SOCKET& clientSocket)
{
    Helper ourHelper;
    std::string msg = "";
    int lengthOfMessage = 0;
    this->_vectorOfSockets.push_back(clientSocket);
    try
    {
        while (true)
        {
            std::cout << clientSocket;
            /*
            // Get message and save into queue
            msg = ourHelper.getStringPartFromSocket(clientSocket, 1024);
            msg = this->returnFullMsg(msg);
            try
            {
                std::cout << clientSocket << " - d\n";
                std::lock_guard<std::mutex> lck(mtx);
                std::cout << clientSocket << " - d\n";
                this->_messagesQueue.push(msg);
                ourConditionVariable.notify_one();
                std::this_thread::sleep_for(std::chrono::seconds(1));  // Wait the main thread to take care for the message
            }
            catch (std::exception e)
            {

            }*/
            std::this_thread::sleep_for(std::chrono::seconds(1));
        }
    }
    catch (const std::exception& e)
    {
        std::cout << "Client logged out\n";
        // this->_users.erase(msg.substr(5, atoi(msg.substr(3, 5).c_str() + 5)));  // Remove the user from the connected users list
        closesocket(clientSocket);
    }
}

And this the code of the main thread:

int main()
{
    Server myServer;
    std::string newMessage;
    std::thread ourConnectorThread (connectorThread, std::ref(myServer));
    ourConnectorThread.join();
    /*std::cout << "Starting...\n";
    while (true)
    {
        std::unique_lock<std::mutex> lckMessages(ourMutex);
        ourConditionVariable.wait(lckMessages);  // Wait for new message
        newMessage = myServer.getQueue().front();  // Get the new message.
        myServer.getQueue().pop();  // Remove the first item
        takeCareForMessage(newMessage, myServer);
        lckMessages.unlock();
    }*/
    return 0;
}

The code in the comments is irrelevant. The result of this code is that the last thread is just printing the number of socket every second, and the other threads aren't printing anything.. What is the problem with my code?

Answer 1

One main error in your code is that client_socket is passed by reference, and then it is modified by the server thread. A fix is to pass it by value.

Another error is that _vectorOfSockets.push_back is modified by multiple threads - a race condition. You need to use a mutex to fix that.

accept may fail when a client has disconnected. That's not an unrecoverable exceptional condition, no need to throw an exception, just retry accept to recover.