zpl-c / enet

⚡️ ENet reliable UDP networking library
https://blog.zpl.pw
MIT License
676 stars 61 forks source link

Server stops handling packets when 1 peer disconnects #46

Closed rytc closed 9 months ago

rytc commented 9 months ago

I run the server then have two clients connect to it. Both clients and server are sending "hello world", which works fine. Then I harshly disconnect one client by hitting ctl-c. What ends up happening is that the server eventually stops receiving incoming packets at all.

I added printfs to the header to see what's happening, and it looks like it gets stuck trying to send to the disconnected peer and ignores everything else.

Frame output:

--- host_service ---
Send outgoing 1 // First call to enet_protocol_send_outgoing_commands
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
peer 0
No commands...continuing
peer 1
Sending packet to peer 1
Receive incoming // Call to enet_protocol_receive_incoming_commands
send outgoing 2  // second call to enet_protocol_send_outgoing_commands
peer 0
No commands...continuing
peer 1
No commands...continuing
dispatch incoming // Top of enet_host_service
Returning because host service time is lequal to timeout
--- host_service --- // next frame

Normal frames with both peers connected look like a loop of this:

--- host_service ---
Send outgoing 1
peer 0
Sending packet to peer 0
peer 1
Sending packet to peer 1
Receive incoming
Received incoming packet of length 21
Peer 1: adding to dispatch list
Received incoming packet of length 10
Received incoming packet of length 21
Peer 0: adding to dispatch list
Received incoming packet of length 10
send outgoing 2
peer 0
Sending packet to peer 0
peer 1
Sending packet to peer 1
dispatch incoming
Peer 1 received
Message recieved! HELLO WORLD
--- host_service ---
Peer 0 received
Message recieved! HELLO WORLD
--- host_service ---
Send outgoing 1
peer 0
No commands...continuing
peer 1
No commands...continuing
Receive incoming
send outgoing 2
peer 0
No commands...continuing
peer 1
No commands...continuing
dispatch incoming
Returning because host service time is lequal to timeout
--- host_service ---

Repro example:

#include <cstdio>
#define ENET_IMPLEMENTATION
#include "enet.h"
#define WIN32_LEAN_AND_MEAN
#include <Windows.h>

void run_server();
void run_client();

int main (int argc, char** argv) {
    if (enet_initialize() < 0) {
        printf("failed to initialize enet\n");
    };

    bool server;
    if (argc == 2 && !strcmp(argv[1], "-server")) {
        server = true;
    } else {
        server = false;
    }

    if (server) {
        run_server();
    } else {
        run_client();
    }

    printf("done\n");
    enet_deinitialize();

    return 0;
}

void run_server() {
    ENetAddress address = {};
    address.host = ENET_HOST_ANY;
    address.port = 1234;

    ENetHost *host = enet_host_create(&address, 4, 1, 0, 0);

    if (!host) {
        printf("Failed to create server\n");
    }

    printf("Server started...");

    while (true) {
        ENetEvent event;
        while (enet_host_service(host, &event, 0)) {
            switch(event.type) {
                case ENET_EVENT_TYPE_CONNECT: {
                    printf("A client has connected!\n");
                } break;
                case ENET_EVENT_TYPE_RECEIVE: {
                    printf("Message recieved! %s\n",  event.packet->data);
                } break;
                case ENET_EVENT_TYPE_DISCONNECT:
                case ENET_EVENT_TYPE_DISCONNECT_TIMEOUT:
                    printf("A client has disconnected\n");
                    break;
            }
        }

        for (int i = 0; i < 4; i++) {
            ENetPeer* peer = &host->peers[i];
            if(peer->state == ENET_PEER_STATE_CONNECTED) {
                auto packet = enet_packet_create("HELLO WORLD", strlen("HELLO WORLD"), ENET_PACKET_FLAG_RELIABLE);
                enet_peer_send(peer, 0, packet);
            }
        }
        Sleep(16);
    }

    enet_host_destroy(host);
}

void run_client() {
    ENetAddress address = {};
    address.host = ENET_HOST_ANY;
    address.port = 1234;

    ENetHost *host = enet_host_create(NULL, 4, 1, 0, 0);

    enet_address_set_host(&address, "127.0.0.1");
    address.port = 1234;
    printf("Connecting to server\n");
    auto peer = enet_host_connect(host, &address, 1, 0);

    if (!host) {
        printf("Failed to create client\n");
    }

    bool connected = false;

    while (true) {
        ENetEvent event;
        while (enet_host_service(host, &event, 0)) {
            switch(event.type) {
                case ENET_EVENT_TYPE_CONNECT: {
                    printf("Connected to server\n");
                    connected = true;
                } break;
                case ENET_EVENT_TYPE_RECEIVE: {
                    printf("Message recieved! %s\n", event.packet->data);
                } break;
                case ENET_EVENT_TYPE_DISCONNECT:
                case ENET_EVENT_TYPE_DISCONNECT_TIMEOUT:
                    printf("A client has disconnected\n");
                    break;
            }
        }

        if (connected) {
            if(peer->state == ENET_PEER_STATE_CONNECTED) {
                auto packet = enet_packet_create("HELLO WORLD", strlen("HELLO WORLD"), ENET_PACKET_FLAG_RELIABLE);
                enet_peer_send(peer, 0, packet);
            }
        }
        Sleep(16);
    }

    enet_host_destroy(host);
}
inlife commented 9 months ago

Hey there!

enet_host_service function returns both positive and negative values, and negative (usually -1) are used to notify you about errors related to the data not being correctly sent.

Could you try to modify your code so that the calls would look something like this, and see if that helps:

while (enet_host_service(host, &event, 0) > 0) {
inlife commented 9 months ago

Also, I noticed you are not destroying the data packets in the ENET_EVENT_TYPE_RECEIVE switch cases, which could lead to additional memory overconsumption.

It's advised you destroy the packet after you are done working with it like so:

enet_packet_destroy(event.packet);
rytc commented 9 months ago

Thanks for the quick reply! I updated the repro example with the suggested changes, but am still seeing the same results.

#include <cstdio>
#define ENET_IMPLEMENTATION
#include "enet.h"
#define WIN32_LEAN_AND_MEAN
#include <Windows.h>

void run_server();
void run_client();

int main (int argc, char** argv) {

    if (enet_initialize() < 0) {
        printf("failed to initialize enet\n");
    };

    bool server;
    if (argc == 2 && !strcmp(argv[1], "-server")) {
        server = true;
    } else {
        server = false;
    }

    if (server) {
        run_server();
    } else {
        run_client();
    }

    printf("done\n");
    enet_deinitialize();

    return 0;
}

void run_server() {
    ENetAddress address = {};
    address.host = ENET_HOST_ANY;
    address.port = 1234;

    ENetHost *host = enet_host_create(&address, 4, 1, 0, 0);

    if (!host) {
        printf("Failed to create server\n");
    }

    printf("Server started...");

    while (true) {
        ENetEvent event;
        while (enet_host_service(host, &event, 0) > 0) {
            switch(event.type) {
                case ENET_EVENT_TYPE_CONNECT: {
                    printf("A client has connected!\n");
                } break;
                case ENET_EVENT_TYPE_RECEIVE: {
                    printf("Message recieved! %s\n",  event.packet->data);
                    enet_packet_destroy(event.packet);
                } break;
                case ENET_EVENT_TYPE_DISCONNECT:
                case ENET_EVENT_TYPE_DISCONNECT_TIMEOUT:
                    printf("A client has disconnected\n");
                    break;
            }
        }

        for (int i = 0; i < 4; i++) {
            ENetPeer* peer = &host->peers[i];
            if(peer->state == ENET_PEER_STATE_CONNECTED) {
                auto packet = enet_packet_create("HELLO WORLD", strlen("HELLO WORLD"), ENET_PACKET_FLAG_RELIABLE);
                enet_peer_send(peer, 0, packet);
            }
        }
        Sleep(16);
    }

    enet_host_destroy(host);
}

void run_client() {
    ENetAddress address = {};
    address.host = ENET_HOST_ANY;
    address.port = 1234;

    ENetHost *host = enet_host_create(NULL, 4, 1, 0, 0);

    enet_address_set_host(&address, "127.0.0.1");
    address.port = 1234;
    printf("Connecting to server\n");
    auto peer = enet_host_connect(host, &address, 1, 0);

    if (!host) {
        printf("Failed to create client\n");
    }

    bool connected = false;

    while (true) {
        ENetEvent event;
        while (enet_host_service(host, &event, 0) > 0) {
            switch(event.type) {
                case ENET_EVENT_TYPE_CONNECT: {
                    printf("Connected to server\n");
                    connected = true;
                } break;
                case ENET_EVENT_TYPE_RECEIVE: {
                    printf("Message recieved! %s\n", event.packet->data);
                    enet_packet_destroy(event.packet);
                } break;
                case ENET_EVENT_TYPE_DISCONNECT:
                case ENET_EVENT_TYPE_DISCONNECT_TIMEOUT:
                    printf("A client has disconnected\n");
                    break;
            }
        }

        if (connected) {
            if(peer->state == ENET_PEER_STATE_CONNECTED) {
                auto packet = enet_packet_create("HELLO WORLD", strlen("HELLO WORLD"), ENET_PACKET_FLAG_RELIABLE);
                enet_peer_send(peer, 0, packet);
            }
        }
        Sleep(16);
    }

    enet_host_destroy(host);
}
inlife commented 9 months ago

I tested your code, and so far, I did achieve a similar behavior only for this specific use case:

  1. Launch server
  2. Launch client 1
  3. Launch client 2
  4. Launch client 3
  5. Launch client 4
  6. Kill client 4
  7. Launch client 4 again - at this stage, the client will be hanging in the connecting state until the previous peer will be dropped due to 30-ish second timeout on the server side. (all 4 peer slots are taken until timeout happens and one slot will be freed since we provide 4 as an option to host for max clients value)
  8. (In the meantime, all 3 other clients are receiving and printing HELLO WORLD without any delays)

Other than that, I don't see any issues similar to what you've described.

Must say, I tested it on macOS, so there could be differences related to the underlying socket library. Does the situation change when you try increasing channels from 1 to 2 or more? Also does the situation change if you try with a bigger max client value?

I'm looking forward to hearing your comments!

rytc commented 9 months ago

I've done some more investigation but am still having a hard time gleaning what is going on. I'm able to reproduce this with just two peers and the server, primarily looking at the server not receiving any packets. Adjusting the number of clients or channels doesn't seem to change anything.

Here is a pastebin of the output of the server: https://pastes.io/cuz8twm2yo It's a lot of output, but in the first quarter things are normal, then Peer 1 is terminated and it keeps trying to send to peer 1. More and more attempts are made, almost exponentially. Then it finally decides that peer 1 has timed out, so it catches up with peer 0. Which then causes peer 0 to time out, either because the socket is flooded or some other reason.

If I let it run until the server determines that Peer 1 has timed out, the output looks like this:

Sending packet to peer 1
Sending packet to peer 1
Peer 1 has timed out!
[39430] A client has disconnected
Sending packet to peer 0
Received packet of length 29
Received incoming packet of length 29
Sending packet to peer 0
Received packet of length 54
Received incoming packet of length 54
Sending packet to peer 0
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Sending packet to peer 0
Sending packet to peer 0
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Received packet of length 10
Received incoming packet of length 10
Received packet of length 21
Received incoming packet of length 21
Received packet of length 37
Received incoming packet of length 37
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Received packet of length 21
Received incoming packet of length 21
Received packet of length 70
Received incoming packet of length 70
Received packet of length 229
Received incoming packet of length 229
Received packet of length 55
Received incoming packet of length 55
Received packet of length 38
Received incoming packet of length 38
Received packet of length 18
Received incoming packet of length 18
Received packet of length 38
Received incoming packet of length 38
Received packet of length 18
Received incoming packet of length 18
Received packet of length 38
Received incoming packet of length 38
Received packet of length 18
Received incoming packet of length 18
Received packet of length 38
Received incoming packet of length 38
Received packet of length 18
Received incoming packet of length 18
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 18
Received incoming packet of length 18
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Sending packet to peer 0
Sending packet to peer 0
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Sending packet to peer 0
Received packet of length 37
Received incoming packet of length 37
Sending packet to peer 0
Received packet of length 10
Received incoming packet of length 10
Received packet of length 54
Received incoming packet of length 54
Sending packet to peer 0
Sending packet to peer 0
Sending packet to peer 0
Sending packet to peer 0
Sending packet to peer 0
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Received packet of length 21
Received incoming packet of length 21
Received packet of length 37
Received incoming packet of length 37
Received packet of length 21
Received incoming packet of length 21
Received packet of length 10
Received incoming packet of length 10
Received packet of length 21

It does start receiving hello worlds, but vary sporadically. eventually peer 0 times out

Received packet of length 10
Received incoming packet of length 10
Received packet of length 18
Received incoming packet of length 18
Received packet of length 38
Received incoming packet of length 38
Sending packet to peer 0
Peer 0 has timed out!
[76130] A client has disconnected
Received packet of length 55
Received incoming packet of length 55
Received packet of length 18
Received incoming packet of length 18
Received packet of length 21
zpl-zak commented 9 months ago

I was able to reproduce it on Windows, and there are two issues in the example given:

  1. The HELLO WORLD packet is sent without a null-terminator and then printed as is on the other side, causing an unterminated string to be printed in the console.
  2. enet_host_service's last argument is timeout. Please set it to at least 2ms so that enet has enough time to process all the packets by all the peers. With 0, re-connecting the client would eventually cause the server not to receive any packets, reproducing your issue. enet_host_service(host, &event, 2) ensures stable connections even after numerous peer reconnections.
rytc commented 9 months ago

Thank you for taking the time to look into this, that seems to have resolved the weird behavior I was seeing. I appreciate your time!