Help about creating server which handles many client connections

[Clyx]

Hi, I'm working on a game working in multiplayer mode, but I'm facing some problems trying to set up a working server which can accepts more than one connection.

How the server should work to my mind

The server starts, the he wait for new client connection, So I have a loop that is active until the server stop.
For each new connection I start a new thread which willl handle comunication with the client, each client are stored in a map width an id and an object representing the connection. So in the thread method I have a loop which runs while the connection is still alive, in this method I handle pakets.

I have to use packets to tranfers data because the project it is more easier. I use TCP packet.

So I would like to know how this kind of server should be done ? How handle connection/disconnection, receiving packets ...

My problem
My server receive the packet but use almost 20% of the processor, I receive 0 in the packet but it shouldn't because I send 42.

This is what I've done (which doesn't work):

Client

bool Client::connect() {
    bool success = m_socket.connect(m_host, m_port) == sf::Socket::Done;

    if(success){
        std::cout << "Connected !" << std::endl;
        m_active = true;
        std::thread thread(sendData, this);
        thread.detach();
    }else{
        std::cerr << "Unable to connect to server" << std::endl;
    }

    return success;
}

//Method which send data
void Client::sendData() {
    sf::Packet packet;
    int i = 42;
    packet << i;
    m_socket.send(packet);
}
 

Server

void Server::start() {

    if(m_port == 0){
        std::cerr << "Error the server must have a port !" << std::endl;
        return;
    }

    if(m_listener.listen(m_port) != sf::Socket::Done){
        std::cerr << "Error while starting server, port is already in use" << std::endl;
        return;
    }

    m_active = true;
    std::cout << "Server started on port " << m_port << " waiting for new client..." << std::endl;

    sf::TcpSocket clientSocket;

    while(m_active){

        if(m_listener.accept(clientSocket) != sf::Socket::Done){
            std::cerr << "Error while receiving new connection" << std::endl;
        }

        unsigned short id = generateClientId();
        ClientConnection *clientConnection = new ClientConnection(0, clientSocket);
        clientConnection->setActive(true);
        addClient(id, clientConnection);
        clientConnection->listenPackets();
        std::cout << "New client connected, starting thread !" << std::endl;
        std::thread thread(&ClientConnection::listenPackets, clientConnection);
        thread.detach();
    }

    std::cout << "Server stopped" << std::endl;

}

//Method which receive packets
void ClientConnection::listenPackets() {
    sf::Packet packet;
    int a;
    while(m_active){
        m_socket->receive(packet);
        while(!packet.endOfPacket()){
            packet >> a;
            std::cout << "Recu " << a << std::endl;
        }
    }
}
 

So here are most interesting parts, any help is welcomed.

Thanks

Clyx.

[Mario]

Recently there's been a similar question on Reddit.

Maybe my sample code from there could help you figuring it out? Note that I'm saving my connections/clients in an array and I don't reuse/invalidate old connections. Instead of using an array you could just add clients to a linked list or something similar.

std::vector<sf::TcpSocket> clients(100);
std::size_t numClients = 0;

while (listener.accept(clients[numClients]) == sf::Socket::Done) {
    // Do something while connecting

    ++numClients; // increase the client count
}

for (sf::TcpSocket &client : clients) {
    // Do something with connected clients
}

In your case you could just create a new sf::TcpSocket as a "client" and pass it to a new thread once a connection is successful.

[Laurent]

Maybe my sample code from there could help you figuring it out?

His code looks better, because he's creating/adding clients dynamically rather than using a fixed-size array and keeping on using disconnected clients That's how I would do it too.

I see two problems in the code. First, you're correctly handling receiving in a thread (one thread per client is questionnable but it works, at least on small scales), but you're calling the function synchronously too (before creating the thread). So you're blocking the listener's loop. Second problem is that you're trying to read from a packet without checking the return value of receive(). And you should not use the endOfPacket() function this way, by the way.

There are also important parts of your code that you're not showing, like the rest of the ClientConnection class.

[Clyx]

Thanks for your answer,

Yeah I forgot to comment the line which call the function synchronously, what do you mean by checking the value of the receive() ? Could you show me an example on how to handle packet correctly ?

I'm troubled, tou say that starting a thread for each client is questionnable, but how to do it otherwise ? I thought it was a must to keep the connexion with the client alive. Could you explain ?

The ClientConnection class is not really important it has just some getter for now.

Thanks.

Clyx

[Laurent]

what do you mean by checking the value of the receive() ?

The receive function returns a status. If it's not Done, it's useless to check the packet content. Cehck the doc for more details.

I'm troubled, tou say that starting a thread for each client is questionnable, but how to do it otherwise ?

One thread for all clients, with a SocketSelector.

[Clyx]

Thanks for your answer ! That works ! I have just a little problem how to properly disconnect a client from the server ?

When I have to remove a client I do this :

//NB : ClientConnection class is just a class that holds an id, the socket and some methods.
ClientConnection *client = m_activesConnections.at(id);
//m_activesConnections is a map<unsigned short, ClientConnection*>
delete client;
 

I have done this in the destructor of ClientConnection:

ClientConnection::~ClientConnection() {
    std::cout << "Destroyed ClientConnection " << m_id <<  std::endl;
    m_socket->disconnect();
}
 

But when I remove a client, and the I try to reconnect a new client it doesn' work it doesn't detect new client..

Here is a part of my server (inspired from the doc):

void Server::start() {

    if(m_port == 0){
        std::cerr << "Error the server must have a port !" << std::endl;
        return;
    }

    if(m_listener.listen(m_port) != sf::Socket::Done){
        std::cerr << "Error while starting server, port is already in use" << std::endl;
        return;
    }

    m_selector.add(m_listener);

    m_active = true;
    std::cout << "Server started on port " << m_port << " waiting for new client..." << std::endl;

    while(m_active){

        if(m_selector.wait()){
            if(m_selector.isReady(m_listener)){
                sf::TcpSocket *client = new sf::TcpSocket;
                if(m_listener.accept(*client) == sf::Socket::Done){
                    ClientConnection *clientConnection = new ClientConnection(*client);
                    clientConnection->setActive(true);
                    addClient(clientConnection);
                    m_selector.add(*clientConnection->getSocket());
                    std::cout << "new client detected, waiting for handshaking.." << std::endl;
                }else{
                    delete client;
                }
            }else{
                for(std::map<unsigned short, ClientConnection*>::iterator it = m_activesConnections.begin(); it != m_activesConnections.end(); it++) {
                    if(&it == nullptr){
                        std::cerr << "Error" << std::endl;
                        continue;
                    }
                    if(it->second != nullptr) {
                        sf::TcpSocket *client = it->second->getSocket();
                        if (m_selector.isReady(*client)) {
                            it->second->listenPackets();
                        }
                    }
                }
            }
        }
    }
}
 

What am I doing wrong ?

Thanks you!

Clyx

[eXpl0it3r]

And what the client do when it gets disconnected?

Btw you should really make use of smart pointers like unique_ptr.