paullouisageneau / libdatachannel

C/C++ WebRTC network library featuring Data Channels, Media Transport, and WebSockets
https://libdatachannel.org/
Mozilla Public License 2.0
1.79k stars 361 forks source link

Dynamically opening and closing DataChannels does not work correctly: "DataChannel is closed" #588

Closed LanderN closed 2 years ago

LanderN commented 2 years ago

I'm developing an application where a C++ program interacts with a web client using WebRTC/libdatachannel. I have discovered an issue while implementing functionality that dynamically opens and closes certain datachannels. I was able to reproduce the issue using only C++/libdatachannel 0.16.7.

Relevant parts of the code:

// Side A
[..]
std::string state = "new";
std::shared_ptr<rtc::DataChannel> dc;

for (;;) {
    if (state == "new") {
        state = {};

        dc = pc.createDataChannel("test_1");
        dc->onMessage([](auto) { /* ignore binary data */ },
                      [&](auto text) {
                          state = "test_1 open";
                          std::cout << "received message on test_1: " << text << std::endl;
                      });
    } else if (state == "test_1 open") {
        state = {};

        dc->close();
        dc = pc.createDataChannel("test_2");
        dc->onMessage([](auto) { /* ignore binary data */ },
                      [&](auto text) {
                          state = "test_2 open";
                          std::cout << "received message on test_2: " << text << std::endl;
                      });
    } else if (state == "test_2 open") {
        state = {};

        dc->close();
        dc = pc.createDataChannel("test_3");
        dc->onMessage([](auto) { /* ignore binary data */ },
                      [&](auto text) {
                          state = "test_3 open";
                          std::cout << "received message on test_3: " << text << std::endl;
                          exit(0);
                      });
    }
}
// Side B
[..]
pc.onDataChannel([](const std::shared_ptr<rtc::DataChannel>& dc) {
    std::cout << "got new data channel: " << dc << std::endl;

    dc->onOpen([dc]() {
        try {
            std::cout << "side b: data channel opened:" << dc << std::endl;
            std::stringstream stream;
            stream << "hello from b, dc: " << dc;
            dc->send(stream.str());
        } catch (const std::exception& e) {
            std::cerr << "side b: could not send to dc " << dc << ": " << e.what() << std::endl;
            exit(1);
        }
    });

    dc->onClosed([dc]() {
        std::cout << "side b: data channel closed:" << dc << std::endl;
    });

    dc->onMessage([](auto) { /* ignore binary data */ },
                  [](auto text) {
                      std::cout << "side b: got text: " << text << std::endl;
                  });
});

(Compilable example: https://gist.github.com/LanderN/c4dee5cf0bc46bd54fb6ac8de251713e)

What I expect the output to be:

got new data channel: 0x39f03044
side b: data channel opened:0x39f03044
received message on test_1: hello from b, dc: 0x39f03044
side b: data channel closed:0x39f03044
got new data channel: 0x39f03094
side b: data channel opened:0x39f03094
received message on test_2: hello from b, dc: 0x39f03094
side b: data channel closed:0x39f03094
got new data channel: whatever
side b: data channel opened:whatever
received message on test_3: hello from b, dc: whatever

The actual output:

got new data channel: 0x39f03044
side b: data channel opened:0x39f03044
received message on test_1: hello from b, dc: 0x39f03044
side b: data channel closed:0x39f03044
got new data channel: 0x39f03094
side b: data channel opened:0x39f03094
received message on test_2: hello from b, dc: 0x39f03094
side b: data channel closed:0x39f03094
side b: data channel opened:0x39f03044
side b: could not send to dc 0x39f03044: DataChannel is closed

It seems like libdatachannel is somehow mixing up the already closed test_1 DataChannel with the new test_3 DataChannel, causing the "DataChannel is closed" error.

paullouisageneau commented 2 years ago

Good catch, thank you for the detailed bug report and the example to reproduce!

The issue is that the id of test_1 is reused to open test_3 (as the stream has been closed) and the other side wrongly routes the open message to the still existing test_1 DataChannel rather than opening a new one as it should.

paullouisageneau commented 2 years ago

This is fixed by https://github.com/paullouisageneau/libdatachannel/pull/590

LanderN commented 2 years ago

Awesome, thanks!

paullouisageneau commented 2 years ago

The fix is released in v0.16.8.