Fix potentially dangling pointers in several Connection related tests.

Bug: webrtc:11988
Change-Id: Iba3f41ba4be8c911d6bdc5241f736831c5a74d0f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249983
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35855}

p2p/base/p2p_transport_channel.cc

@@ -1656,6 +1656,17 @@ rtc::ArrayView<Connection*> P2PTransportChannel::connections() const {
                                      res.size());
 }
 
+void P2PTransportChannel::RemoveConnectionForTest(Connection* connection) {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  RTC_DCHECK(FindConnection(connection));
+  connection->SignalDestroyed.disconnect(this);
+  ice_controller_->OnConnectionDestroyed(connection);
+  RTC_DCHECK(!FindConnection(connection));
+  if (selected_connection_ == connection)
+    selected_connection_ = nullptr;
+  connection->Destroy();
+}
+
 // Monitor connection states.
 void P2PTransportChannel::UpdateConnectionStates() {
   RTC_DCHECK_RUN_ON(network_thread_);

p2p/base/p2p_transport_channel.h

@@ -212,6 +212,7 @@ class RTC_EXPORT P2PTransportChannel : public IceTransportInternal {
 
   // Public for unit tests.
   rtc::ArrayView<Connection*> connections() const;
+  void RemoveConnectionForTest(Connection* connection);
 
   // Public for unit tests.
   PortAllocatorSession* allocator_session() const {

p2p/base/p2p_transport_channel_unittest.cc

@@ -2464,10 +2464,12 @@ class P2PTransportChannelMultihomedTest : public P2PTransportChannelTestBase {
 
   void DestroyAllButBestConnection(P2PTransportChannel* channel) {
     const Connection* selected_connection = channel->selected_connection();
-    for (Connection* conn : channel->connections()) {
-      if (conn != selected_connection) {
-        conn->Destroy();
-      }
+    // Copy the list of connections since the original will be modified.
+    rtc::ArrayView<Connection*> view = channel->connections();
+    std::vector<Connection*> connections(view.begin(), view.end());
+    for (Connection* conn : connections) {
+      if (conn != selected_connection)
+        channel->RemoveConnectionForTest(conn);
     }
   }
 };
@@ -3829,7 +3831,7 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
   // Wait for conn2 to be selected.
   EXPECT_EQ_WAIT(conn2, ch.selected_connection(), kMediumTimeout);
   // Destroy the connection to test SignalUnknownAddress.
-  conn1->Destroy();
+  ch.RemoveConnectionForTest(conn1);
   EXPECT_TRUE_WAIT(GetConnectionTo(&ch, "1.1.1.1", 1) == nullptr,
                    kMediumTimeout);
 

p2p/base/port_unittest.cc

@@ -274,6 +274,12 @@ class TestChannel : public sigslot::has_slots<> {
         [this](PortInterface* port) { OnSrcPortDestroyed(port); });
   }
 
+  ~TestChannel() {
+    if (conn_) {
+      conn_->SignalDestroyed.disconnect(this);
+    }
+  }
+
   int complete_count() { return complete_count_; }
   Connection* conn() { return conn_; }
   const SocketAddress& remote_address() { return remote_address_; }
@@ -311,7 +317,9 @@ class TestChannel : public sigslot::has_slots<> {
   void Ping(int64_t now) { conn_->Ping(now); }
   void Stop() {
     if (conn_) {
+      conn_->SignalDestroyed.disconnect(this);
       conn_->Destroy();
+      conn_ = nullptr;
     }
   }
 

p2p/base/tcp_port_unittest.cc

@@ -44,15 +44,23 @@ static const SocketAddress kRemoteIPv6Addr("2401:fa00:4:1000:be30:5bff:fee5:c4",
 
 class ConnectionObserver : public sigslot::has_slots<> {
  public:
-  explicit ConnectionObserver(Connection* conn) {
+  explicit ConnectionObserver(Connection* conn) : conn_(conn) {
     conn->SignalDestroyed.connect(this, &ConnectionObserver::OnDestroyed);
   }
 
+  ~ConnectionObserver() {
+    if (!connection_destroyed_) {
+      RTC_DCHECK(conn_);
+      conn_->SignalDestroyed.disconnect(this);
+    }
+  }
+
   bool connection_destroyed() { return connection_destroyed_; }
 
  private:
   void OnDestroyed(Connection*) { connection_destroyed_ = true; }
 
+  Connection* const conn_;
   bool connection_destroyed_ = false;
 };
 

p2p/base/turn_port_unittest.cc

@@ -151,6 +151,12 @@ class TestConnectionWrapper : public sigslot::has_slots<> {
         this, &TestConnectionWrapper::OnConnectionDestroyed);
   }
 
+  ~TestConnectionWrapper() {
+    if (connection_) {
+      connection_->SignalDestroyed.disconnect(this);
+    }
+  }
+
   Connection* connection() { return connection_; }
 
  private: