Reconceptualize when a DataChannel becomes Closed.

OrchidTechnologies · Mar 4, 2024 · f4e92f7 · f4e92f7
1 parent d2595b2
commit f4e92f7
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 114 deletions.
diff --git a/p2p/source/channel.cpp b/p2p/source/channel.cpp
@@ -61,7 +61,6 @@ Channel::Channel(BufferDrain &drain, const S<Peer> &peer, rtc::scoped_refptr<web
     channel_(std::move(channel))
 {
     channel_->RegisterObserver(this);
-    peer_->channels_.insert(this);
 }
 
 Channel::Channel(BufferDrain &drain, const S<Peer> &peer, int id, const std::string &label, const std::string &protocol) :
@@ -91,23 +90,18 @@ task<Socket> Channel::Wire(BufferSunk &sunk, S<Base> base, Configuration configu
 }
 
 void Channel::OnStateChange() noexcept {
+    const auto state(channel_->state());
+    if (Verbose)
+        Log() << "OnStateChange(" << webrtc::DataChannelInterface::DataStateString(state) << ")" << std::endl;
     switch (channel_->state()) {
         case webrtc::DataChannelInterface::kConnecting:
-            if (Verbose)
-                Log() << "OnStateChange(kConnecting)" << std::endl;
             break;
         case webrtc::DataChannelInterface::kOpen:
-            if (Verbose)
-                Log() << "OnStateChange(kOpen)" << std::endl;
             opened_();
             break;
         case webrtc::DataChannelInterface::kClosing:
-            if (Verbose)
-                Log() << "OnStateChange(kClosing)" << std::endl;
             break;
         case webrtc::DataChannelInterface::kClosed:
-            if (Verbose)
-                Log() << "OnStateChange(kClosed)" << std::endl;
             Stop();
             break;
     }
@@ -136,21 +130,19 @@ task<void> Channel::Open() noexcept {
 task<void> Channel::Shut() noexcept {
     co_await Post([&]() {
         channel_->Close();
+        channel_->UnregisterObserver();
 
-        // XXX: this should be checking if Peer has a data_transport
-        if (channel_->id() == -1)
+        const auto state(channel_->state());
+        if (Verbose)
+            Log() << "closed channel in state " << webrtc::DataChannelInterface::DataStateString(state) << std::endl;
+        if (state != webrtc::DataChannelInterface::kClosed)
             Stop();
-    });
 
-    co_await Pump::Shut();
-
-    co_await Post([&]() {
-        channel_->UnregisterObserver();
         channel_ = nullptr;
-
-        peer_->channels_.erase(this);
         peer_ = nullptr;
     });
+
+    co_await Pump::Shut();
 }
 
 template <typename Type_, Type_ Pointer_>

diff --git a/p2p/source/peer.cpp b/p2p/source/peer.cpp
@@ -99,102 +99,17 @@ void Peer::OnRenegotiationNeeded() noexcept {
 }
 
 void Peer::OnIceConnectionChange(webrtc::PeerConnectionInterface::IceConnectionState state) noexcept {
-    switch (state) {
-        case webrtc::PeerConnectionInterface::kIceConnectionNew:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionNew)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionChecking:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionChecking)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionConnected:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionConnected)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionCompleted:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionCompleted)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionDisconnected:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionDisconnected)" << std::endl;
-            break;
-
-        case webrtc::PeerConnectionInterface::kIceConnectionFailed:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionFailed)" << std::endl;
-            // this should be handled in OnStandardizedIceConnectionChange
-        break;
-
-        case webrtc::PeerConnectionInterface::kIceConnectionClosed:
-            if (Verbose)
-                Log() << "OnIceConnectionChange(kIceConnectionClosed)" << std::endl;
-            closed_();
-        break;
-
-        case webrtc::PeerConnectionInterface::kIceConnectionMax:
-        default:
-            orc_insist(false);
-        break;
-    }
+    if (Verbose)
+        Log() << "OnIceConnectionChange(" << webrtc::PeerConnectionInterface::AsString(state) << ")" << std::endl;
+    if (state == webrtc::PeerConnectionInterface::kIceConnectionClosed)
+        closed_();
 }
 
 void Peer::OnStandardizedIceConnectionChange(webrtc::PeerConnectionInterface::IceConnectionState state) noexcept {
-    switch (state) {
-        case webrtc::PeerConnectionInterface::kIceConnectionNew:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionNew)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionChecking:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionChecking)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionConnected:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionConnected)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionCompleted:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionCompleted)" << std::endl;
-            break;
-        case webrtc::PeerConnectionInterface::kIceConnectionDisconnected:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionDisconnected)" << std::endl;
-            break;
-
-        case webrtc::PeerConnectionInterface::kIceConnectionFailed:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionFailed)" << std::endl;
-
-            // you can't close a PeerConnection if it is blocked in a signal
-            Spawn([self = shared_from_this()]() mutable noexcept -> task<void> {
-                co_await Post([self = std::move(self)]() mutable {
-                    for (auto current(self->channels_.begin()); current != self->channels_.end(); ) {
-                        auto next(current);
-                        ++next;
-                        (*current)->Stop("kIceConnectionClosed");
-                        current = next;
-                    }
-
-                    self->Stop();
-                    self = nullptr;
-                });
-            }, __FUNCTION__);
-        break;
-
-        case webrtc::PeerConnectionInterface::kIceConnectionClosed:
-            if (Verbose)
-                Log() << "OnStandardizedIceConnectionChange(kIceConnectionClosed)" << std::endl;
-            // this is (annoyingly) only signaled via OnIceConnectionChange
-            orc_insist(false);
-        break;
-
-        case webrtc::PeerConnectionInterface::kIceConnectionMax:
-        default:
-            orc_insist(false);
-        break;
-    }
+    if (Verbose)
+        Log() << "OnStandardizedIceConnectionChange(" << webrtc::PeerConnectionInterface::AsString(state) << ")" << std::endl;
+    // this is (annoyingly) only signaled via OnIceConnectionChange
+    orc_insist(state != webrtc::PeerConnectionInterface::kIceConnectionClosed);
 }
 
 void Peer::OnIceGatheringChange(webrtc::PeerConnectionInterface::IceGatheringState state) noexcept {

diff --git a/p2p/source/peer.hpp b/p2p/source/peer.hpp
@@ -81,9 +81,6 @@ class Peer :
     const S<Base> base_;
     const rtc::scoped_refptr<webrtc::PeerConnectionInterface> peer_;
 
-    // XXX: do I need to lock this?
-    std::set<Channel *> channels_;
-
     Event gathered_;
     std::vector<std::string> gathering_;
     std::vector<std::string> candidates_;