Resolve the race condition between mDNS name registration and

cricket::Port::SignalPortComplete.

The mDNS name registration is asynchronously executed by the mDNS
responder, and a host candidate with an mDNS name is only gathered after
this completes. SignalPortComplete however is currently done
synchronously by UDPPort, and any candidate gathered by a UDPPort after
this signal is fired would be discarded.

Bug: webrtc:9964, webrtc:9605
Change-Id: If8aaf193ef26c06bd118e6418b62ba0de5e87e3c
Reviewed-on: https://webrtc-review.googlesource.com/c/109541
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Reviewed-by: Zach Stein <zstein@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Commit-Queue: Qingsi Wang <qingsi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25534}

p2p/base/port.cc

@@ -399,9 +399,9 @@ void Port::AddAddress(const rtc::SocketAddress& address,
                       const std::string& type,
                       uint32_t type_preference,
                       uint32_t relay_preference,
-                      bool final) {
+                      bool is_final) {
   AddAddress(address, base_address, related_address, protocol, relay_protocol,
-             tcptype, type, type_preference, relay_preference, "", final);
+             tcptype, type, type_preference, relay_preference, "", is_final);
 }
 
 void Port::AddAddress(const rtc::SocketAddress& address,
@@ -448,9 +448,13 @@ void Port::AddAddress(const rtc::SocketAddress& address,
         c.set_address(hostname_address);
         RTC_DCHECK(c.related_address() == rtc::SocketAddress());
         if (weak_ptr != nullptr) {
+          weak_ptr->set_mdns_name_registration_status(
+              MdnsNameRegistrationStatus::kCompleted);
           weak_ptr->FinishAddingAddress(c, is_final);
         }
       };
+      set_mdns_name_registration_status(
+          MdnsNameRegistrationStatus::kInProgress);
       network_->GetMdnsResponder()->CreateNameForAddress(c.address().ipaddr(),
                                                          callback);
       return;
@@ -468,6 +472,10 @@ void Port::FinishAddingAddress(const Candidate& c, bool is_final) {
   candidates_.push_back(c);
   SignalCandidateReady(this, c);
 
+  PostAddAddress(is_final);
+}
+
+void Port::PostAddAddress(bool is_final) {
   if (is_final) {
     SignalPortComplete(this);
   }

p2p/base/port.h

@@ -84,6 +84,18 @@ enum class IceCandidatePairState {
   // frozen because we have not implemented ICE freezing logic.
 };
 
+enum class MdnsNameRegistrationStatus {
+  // IP concealment with mDNS is not enabled or the name registration process is
+  // not started yet.
+  kNotStarted,
+  // A request to create and register an mDNS name for a local IP address of a
+  // host candidate is sent to the mDNS responder.
+  kInProgress,
+  // The name registration is complete and the created name is returned by the
+  // mDNS responder.
+  kCompleted,
+};
+
 // Stats that we can return about the port of a STUN candidate.
 class StunStats {
  public:
@@ -393,7 +405,7 @@ class Port : public PortInterface,
                   const std::string& type,
                   uint32_t type_preference,
                   uint32_t relay_preference,
-                  bool final);
+                  bool is_final);
 
   void AddAddress(const rtc::SocketAddress& address,
                   const rtc::SocketAddress& base_address,
@@ -409,6 +421,8 @@ class Port : public PortInterface,
 
   void FinishAddingAddress(const Candidate& c, bool is_final);
 
+  virtual void PostAddAddress(bool is_final);
+
   // Adds the given connection to the map keyed by the remote candidate address.
   // If an existing connection has the same address, the existing one will be
   // replaced and destroyed.
@@ -444,6 +458,13 @@ class Port : public PortInterface,
 
   void CopyPortInformationToPacketInfo(rtc::PacketInfo* info) const;
 
+  MdnsNameRegistrationStatus mdns_name_registration_status() const {
+    return mdns_name_registration_status_;
+  }
+  void set_mdns_name_registration_status(MdnsNameRegistrationStatus status) {
+    mdns_name_registration_status_ = status;
+  }
+
  private:
   void Construct();
   // Called when one of our connections deletes itself.
@@ -488,6 +509,8 @@ class Port : public PortInterface,
   int16_t network_cost_;
   State state_ = State::INIT;
   int64_t last_time_all_connections_removed_ = 0;
+  MdnsNameRegistrationStatus mdns_name_registration_status_ =
+      MdnsNameRegistrationStatus::kNotStarted;
 
   rtc::WeakPtrFactory<Port> weak_factory_;
 

p2p/base/stunport.cc

@@ -358,6 +358,10 @@ void UDPPort::OnLocalAddressReady(rtc::AsyncPacketSocket* socket,
   MaybePrepareStunCandidate();
 }
 
+void UDPPort::PostAddAddress(bool is_final) {
+  MaybeSetPortCompleteOrError();
+}
+
 void UDPPort::OnReadPacket(rtc::AsyncPacketSocket* socket,
                            const char* data,
                            size_t size,
@@ -517,8 +521,14 @@ void UDPPort::OnStunBindingOrResolveRequestFailed(
 }
 
 void UDPPort::MaybeSetPortCompleteOrError() {
-  if (ready_)
+  if (mdns_name_registration_status() ==
+      MdnsNameRegistrationStatus::kInProgress) {
     return;
+  }
+
+  if (ready_) {
+    return;
+  }
 
   // Do not set port ready if we are still waiting for bind responses.
   const size_t servers_done_bind_request =

p2p/base/stunport.h

@@ -160,6 +160,9 @@ class UDPPort : public Port {
 
   void OnLocalAddressReady(rtc::AsyncPacketSocket* socket,
                            const rtc::SocketAddress& address);
+
+  void PostAddAddress(bool is_final) override;
+
   void OnReadPacket(rtc::AsyncPacketSocket* socket,
                     const char* data,
                     size_t size,

rtc_base/fake_mdns_responder.h

@@ -23,7 +23,7 @@ namespace webrtc {
 
 class FakeMdnsResponder : public MdnsResponderInterface {
  public:
-  FakeMdnsResponder() = default;
+  explicit FakeMdnsResponder(rtc::Thread* thread) : thread_(thread) {}
   ~FakeMdnsResponder() = default;
 
   void CreateNameForAddress(const rtc::IPAddress& addr,
@@ -35,7 +35,9 @@ class FakeMdnsResponder : public MdnsResponderInterface {
       name = std::to_string(next_available_id_++) + ".local";
       addr_name_map_[addr] = name;
     }
-    callback(addr, name);
+    invoker_.AsyncInvoke<void>(
+        RTC_FROM_HERE, thread_,
+        [callback, addr, name]() { callback(addr, name); });
   }
   void RemoveNameForAddress(const rtc::IPAddress& addr,
                             NameRemovedCallback callback) override {
@@ -43,12 +45,16 @@ class FakeMdnsResponder : public MdnsResponderInterface {
     if (it != addr_name_map_.end()) {
       addr_name_map_.erase(it);
     }
-    callback(it != addr_name_map_.end());
+    bool result = it != addr_name_map_.end();
+    invoker_.AsyncInvoke<void>(RTC_FROM_HERE, thread_,
+                               [callback, result]() { callback(result); });
   }
 
  private:
   uint32_t next_available_id_ = 0;
   std::map<rtc::IPAddress, std::string> addr_name_map_;
+  rtc::Thread* thread_;
+  rtc::AsyncInvoker invoker_;
 };
 
 }  // namespace webrtc

rtc_base/fakenetwork.h

@@ -84,7 +84,8 @@ class FakeNetworkManager : public NetworkManagerBase, public MessageHandler {
 
   void CreateMdnsResponder() {
     if (mdns_responder_ == nullptr) {
-      mdns_responder_ = absl::make_unique<webrtc::FakeMdnsResponder>();
+      mdns_responder_ =
+          absl::make_unique<webrtc::FakeMdnsResponder>(rtc::Thread::Current());
     }
   }