[TurnPort] Bring back stricter assumptions after recent fixes.

This also swithces lifetime management of entries to using
std::unique_ptr.

Bug: chromium:1374310
Change-Id: I5a2c89e9b3bcf7bceec2a4e5347750540ee21e1f
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279521
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38432}

p2p/base/turn_port.cc

@@ -146,6 +146,7 @@ class TurnEntry : public sigslot::has_slots<> {
  public:
   enum BindState { STATE_UNBOUND, STATE_BINDING, STATE_BOUND };
   TurnEntry(TurnPort* port, Connection* conn, int channel_id);
+  ~TurnEntry();
 
   TurnPort* port() { return port_; }
 
@@ -271,7 +272,7 @@ TurnPort::TurnPort(TaskQueueBase* thread,
       tls_elliptic_curves_(tls_elliptic_curves),
       tls_cert_verifier_(tls_cert_verifier),
       credentials_(credentials),
-      socket_(NULL),
+      socket_(nullptr),
       error_(0),
       stun_dscp_value_(rtc::DSCP_NO_CHANGE),
       request_manager_(
@@ -294,9 +295,7 @@ TurnPort::~TurnPort() {
     Release();
   }
 
-  while (!entries_.empty()) {
-    DestroyEntry(entries_.front());
-  }
+  entries_.clear();
 
   if (socket_)
     socket_->UnsubscribeClose(this);
@@ -547,7 +546,7 @@ void TurnPort::OnAllocateMismatch() {
   } else {
     delete socket_;
   }
-  socket_ = NULL;
+  socket_ = nullptr;
 
   ResetNonce();
   PrepareAddress();
@@ -641,16 +640,7 @@ int TurnPort::SendTo(const void* data,
                      bool payload) {
   // Try to find an entry for this specific address; we should have one.
   TurnEntry* entry = FindEntry(addr);
-  if (!entry) {
-    RTC_LOG(LS_ERROR) << "Did not find the TurnEntry for address "
-                      << addr.ToSensitiveString();
-    // Although not finding an entry isn't a socket error, at this level we need
-    // to return SOCKET_ERROR (-1) and for ease of troubleshooting/debugging
-    // assign a value to `error_` that makes some semantic sense. In this case
-    // we pick EADDRNOTAVAIL (10049 on Windows, 9903 in errno.h).
-    error_ = EADDRNOTAVAIL;
-    return SOCKET_ERROR;
-  }
+  RTC_DCHECK(entry);
 
   if (!ready()) {
     error_ = ENOTCONN;
@@ -1176,35 +1166,29 @@ void TurnPort::ResetNonce() {
 }
 
 bool TurnPort::HasPermission(const rtc::IPAddress& ipaddr) const {
-  return absl::c_any_of(entries_, [&ipaddr](const TurnEntry* e) {
+  return absl::c_any_of(entries_, [&ipaddr](const auto& e) {
     return e->address().ipaddr() == ipaddr;
   });
 }
 
 TurnEntry* TurnPort::FindEntry(const rtc::SocketAddress& addr) const {
   auto it = absl::c_find_if(
-      entries_, [&addr](const TurnEntry* e) { return e->address() == addr; });
-  return (it != entries_.end()) ? *it : NULL;
+      entries_, [&addr](const auto& e) { return e->address() == addr; });
+  return (it != entries_.end()) ? it->get() : nullptr;
 }
 
 TurnEntry* TurnPort::FindEntry(int channel_id) const {
-  auto it = absl::c_find_if(entries_, [&channel_id](const TurnEntry* e) {
+  auto it = absl::c_find_if(entries_, [&channel_id](const auto& e) {
     return e->channel_id() == channel_id;
   });
-  return (it != entries_.end()) ? *it : NULL;
-}
-
-bool TurnPort::EntryExists(TurnEntry* e) {
-  return absl::c_linear_search(entries_, e);
+  return (it != entries_.end()) ? it->get() : nullptr;
 }
 
 bool TurnPort::CreateOrRefreshEntry(Connection* conn, int channel_number) {
   const Candidate& remote_candidate = conn->remote_candidate();
   TurnEntry* entry = FindEntry(remote_candidate.address());
-
   if (entry == nullptr) {
-    entry = new TurnEntry(this, conn, channel_number);
-    entries_.push_back(entry);
+    entries_.push_back(std::make_unique<TurnEntry>(this, conn, channel_number));
     return true;
   }
 
@@ -1215,26 +1199,12 @@ bool TurnPort::CreateOrRefreshEntry(Connection* conn, int channel_number) {
   return false;
 }
 
-void TurnPort::DestroyEntry(TurnEntry* entry) {
-  entry->destroyed_callback_list_.Send(entry);
-  entries_.remove(entry);
-  delete entry;
-}
-
 void TurnPort::HandleConnectionDestroyed(Connection* conn) {
   // Schedule an event to destroy TurnEntry for the connection, which is
   // being destroyed.
   const rtc::SocketAddress& remote_address = conn->remote_candidate().address();
+  // We should always have an entry for this connection.
   TurnEntry* entry = FindEntry(remote_address);
-  if (!entry) {
-    // TODO(chromium:1374310): This happens because more than one connection
-    // may be associated with an entry. Previously a connection with the same
-    // address has been destroyed and subsequently the entry removed
-    // (prematurely.)
-    RTC_DLOG_F(LS_WARNING) << "Entry has been removed.";
-    return;
-  }
-
   rtc::scoped_refptr<webrtc::PendingTaskSafetyFlag> flag =
       entry->UntrackConnection(conn);
   if (flag) {
@@ -1243,9 +1213,14 @@ void TurnPort::HandleConnectionDestroyed(Connection* conn) {
     // If an entry gets reused (associated with a new connection) while this
     // task is pending, the entry will reset the safety flag, thus cancel this
     // task.
-    thread()->PostDelayedTask(
-        SafeTask(flag, [this, entry] { DestroyEntry(entry); }),
-        kTurnPermissionTimeout);
+    thread()->PostDelayedTask(SafeTask(flag,
+                                       [this, entry] {
+                                         entries_.erase(absl::c_find_if(
+                                             entries_, [entry](const auto& e) {
+                                               return e.get() == entry;
+                                             }));
+                                       }),
+                              kTurnPermissionTimeout);
   }
 }
 
@@ -1752,6 +1727,10 @@ TurnEntry::TurnEntry(TurnPort* port, Connection* conn, int channel_id)
   SendCreatePermissionRequest(0);
 }
 
+TurnEntry::~TurnEntry() {
+  destroyed_callback_list_.Send(this);
+}
+
 void TurnEntry::TrackConnection(Connection* conn) {
   RTC_DCHECK(absl::c_find(connections_, conn) == connections_.end());
   if (connections_.empty()) {

p2p/base/turn_port.h

@@ -13,7 +13,6 @@
 
 #include <stdio.h>
 
-#include <list>
 #include <map>
 #include <memory>
 #include <set>
@@ -239,7 +238,6 @@ class TurnPort : public Port {
   void Close();
 
  private:
-  typedef std::list<TurnEntry*> EntryList;
   typedef std::map<rtc::Socket::Option, int> SocketOptionsMap;
   typedef std::set<rtc::SocketAddress> AttemptedServerSet;
 
@@ -296,8 +294,6 @@ class TurnPort : public Port {
   bool HasPermission(const rtc::IPAddress& ipaddr) const;
   TurnEntry* FindEntry(const rtc::SocketAddress& address) const;
   TurnEntry* FindEntry(int channel_id) const;
-  bool EntryExists(TurnEntry* e);
-  void DestroyEntry(TurnEntry* entry);
 
   // Marks the connection with remote address `address` failed and
   // pruned (a.k.a. write-timed-out). Returns true if a connection is found.
@@ -333,7 +329,7 @@ class TurnPort : public Port {
   std::string hash_;   // Digest of username:realm:password
 
   int next_channel_number_;
-  EntryList entries_;
+  std::vector<std::unique_ptr<TurnEntry>> entries_;
 
   PortState state_;
   // By default the value will be set to 0. This value will be used in