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}diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index b00983a..4954744 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -399,9 +399,9 @@
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 @@
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 @@
candidates_.push_back(c);
SignalCandidateReady(this, c);
+ PostAddAddress(is_final);
+}
+
+void Port::PostAddAddress(bool is_final) {
if (is_final) {
SignalPortComplete(this);
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 25a1cf7..e0b3f07 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -84,6 +84,18 @@
// 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 @@
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 @@
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 @@
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 @@
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_;
diff --git a/p2p/base/stunport.cc b/p2p/base/stunport.cc
index 00ecca2..7273123 100644
--- a/p2p/base/stunport.cc
+++ b/p2p/base/stunport.cc
@@ -358,6 +358,10 @@
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::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 =
diff --git a/p2p/base/stunport.h b/p2p/base/stunport.h
index dc9f0e3..ca43cbc 100644
--- a/p2p/base/stunport.h
+++ b/p2p/base/stunport.h
@@ -160,6 +160,9 @@
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,
diff --git a/rtc_base/fake_mdns_responder.h b/rtc_base/fake_mdns_responder.h
index e44d30e..1e60a5d 100644
--- a/rtc_base/fake_mdns_responder.h
+++ b/rtc_base/fake_mdns_responder.h
@@ -23,7 +23,7 @@
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 @@
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 @@
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
diff --git a/rtc_base/fakenetwork.h b/rtc_base/fakenetwork.h
index f3de83b..cb890ec 100644
--- a/rtc_base/fakenetwork.h
+++ b/rtc_base/fakenetwork.h
@@ -84,7 +84,8 @@
void CreateMdnsResponder() {
if (mdns_responder_ == nullptr) {
- mdns_responder_ = absl::make_unique<webrtc::FakeMdnsResponder>();
+ mdns_responder_ =
+ absl::make_unique<webrtc::FakeMdnsResponder>(rtc::Thread::Current());
}
}
