When doing DisableEquivalentPhases, exclude those AllocationSequences
whose network has ever been removed. It is unlikely the sockets/ports/candidates created from
those AllocationSequences will still be valid.
BUG=
Review URL: https://codereview.webrtc.org/1361183004
Cr-Commit-Position: refs/heads/master@{#10093}
diff --git a/webrtc/base/fakenetwork.h b/webrtc/base/fakenetwork.h
index 065d08d..fb99d59 100644
--- a/webrtc/base/fakenetwork.h
+++ b/webrtc/base/fakenetwork.h
@@ -34,8 +34,12 @@
typedef std::vector<SocketAddress> IfaceList;
void AddInterface(const SocketAddress& iface) {
- // ensure a unique name for the interface
- SocketAddress address("test" + rtc::ToString(next_index_++), 0);
+ // Ensure a unique name for the interface if its name is not given.
+ AddInterface(iface, "test" + rtc::ToString(next_index_++));
+ }
+
+ void AddInterface(const SocketAddress& iface, const std::string& if_name) {
+ SocketAddress address(if_name, 0);
address.SetResolvedIP(iface.ipaddr());
ifaces_.push_back(address);
DoUpdateNetworks();
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index 9820252..11d0cef 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -298,28 +298,35 @@
allocation_started_ = true;
}
-// For each network, see if we have a sequence that covers it already. If not,
-// create a new sequence to create the appropriate ports.
-void BasicPortAllocatorSession::DoAllocate() {
- bool done_signal_needed = false;
- std::vector<rtc::Network*> networks;
-
+void BasicPortAllocatorSession::GetNetworks(
+ std::vector<rtc::Network*>* networks) {
+ networks->clear();
+ rtc::NetworkManager* network_manager = allocator_->network_manager();
+ ASSERT(network_manager != nullptr);
// If the network permission state is BLOCKED, we just act as if the flag has
// been passed in.
- if (allocator_->network_manager()->enumeration_permission() ==
+ if (network_manager->enumeration_permission() ==
rtc::NetworkManager::ENUMERATION_BLOCKED) {
set_flags(flags() | PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION);
}
-
// If the adapter enumeration is disabled, we'll just bind to any address
// instead of specific NIC. This is to ensure the same routing for http
// traffic by OS is also used here to avoid any local or public IP leakage
// during stun process.
if (flags() & PORTALLOCATOR_DISABLE_ADAPTER_ENUMERATION) {
- allocator_->network_manager()->GetAnyAddressNetworks(&networks);
+ network_manager->GetAnyAddressNetworks(networks);
} else {
- allocator_->network_manager()->GetNetworks(&networks);
+ network_manager->GetNetworks(networks);
}
+}
+
+// For each network, see if we have a sequence that covers it already. If not,
+// create a new sequence to create the appropriate ports.
+void BasicPortAllocatorSession::DoAllocate() {
+ bool done_signal_needed = false;
+ std::vector<rtc::Network*> networks;
+ GetNetworks(&networks);
+
if (networks.empty()) {
LOG(LS_WARNING) << "Machine has no networks; no ports will be allocated";
done_signal_needed = true;
@@ -377,6 +384,18 @@
}
void BasicPortAllocatorSession::OnNetworksChanged() {
+ std::vector<rtc::Network*> networks;
+ GetNetworks(&networks);
+ for (AllocationSequence* sequence : sequences_) {
+ // Remove the network from the allocation sequence if it is not in
+ // |networks|.
+ if (!sequence->network_removed() &&
+ std::find(networks.begin(), networks.end(), sequence->network()) ==
+ networks.end()) {
+ sequence->OnNetworkRemoved();
+ }
+ }
+
network_manager_started_ = true;
if (allocation_started_)
DoAllocate();
@@ -711,12 +730,24 @@
turn_ports_.clear();
}
+void AllocationSequence::OnNetworkRemoved() {
+ // Stop the allocation sequence if its network is gone.
+ Stop();
+ network_removed_ = true;
+}
+
AllocationSequence::~AllocationSequence() {
session_->network_thread()->Clear(this);
}
void AllocationSequence::DisableEquivalentPhases(rtc::Network* network,
PortConfiguration* config, uint32* flags) {
+ if (network_removed_) {
+ // If the network of this allocation sequence has ever gone away,
+ // it won't be equivalent to the new network.
+ return;
+ }
+
if (!((network == network_) && (ip_ == network->GetBestIP()))) {
// Different network setup; nothing is equivalent.
return;
diff --git a/webrtc/p2p/client/basicportallocator.h b/webrtc/p2p/client/basicportallocator.h
index a27ff4c..37ce1a0 100644
--- a/webrtc/p2p/client/basicportallocator.h
+++ b/webrtc/p2p/client/basicportallocator.h
@@ -182,6 +182,7 @@
void MaybeSignalCandidatesAllocationDone();
void OnPortAllocationComplete(AllocationSequence* seq);
PortData* FindPort(Port* port);
+ void GetNetworks(std::vector<rtc::Network*>* networks);
bool CheckCandidateFilter(const Candidate& c);
@@ -260,8 +261,11 @@
~AllocationSequence();
bool Init();
void Clear();
+ void OnNetworkRemoved();
State state() const { return state_; }
+ const rtc::Network* network() const { return network_; }
+ bool network_removed() const { return network_removed_; }
// Disables the phases for a new sequence that this one already covers for an
// equivalent network setup.
@@ -311,6 +315,7 @@
void OnPortDestroyed(PortInterface* port);
BasicPortAllocatorSession* session_;
+ bool network_removed_ = false;
rtc::Network* network_;
rtc::IPAddress ip_;
PortConfiguration* config_;
diff --git a/webrtc/p2p/client/portallocator_unittest.cc b/webrtc/p2p/client/portallocator_unittest.cc
index 9ea22dd..2893807 100644
--- a/webrtc/p2p/client/portallocator_unittest.cc
+++ b/webrtc/p2p/client/portallocator_unittest.cc
@@ -111,12 +111,18 @@
void AddInterface(const SocketAddress& addr) {
network_manager_.AddInterface(addr);
}
+ void AddInterface(const SocketAddress& addr, const std::string& if_name) {
+ network_manager_.AddInterface(addr, if_name);
+ }
void AddInterfaceAsDefaultRoute(const SocketAddress& addr) {
AddInterface(addr);
// When a binding comes from the any address, the |addr| will be used as the
// srflx address.
vss_->SetDefaultRoute(addr.ipaddr());
}
+ void RemoveInterface(const SocketAddress& addr) {
+ network_manager_.RemoveInterface(addr);
+ }
bool SetPortRange(int min_port, int max_port) {
return allocator_->SetPortRange(min_port, max_port);
}
@@ -432,6 +438,61 @@
EXPECT_TRUE(candidate_allocation_done_);
}
+// Test that when the same network interface is brought down and up, the
+// port allocator session will restart a new allocation sequence if
+// it is not stopped.
+TEST_F(PortAllocatorTest, TestSameNetworkDownAndUpWhenSessionNotStopped) {
+ std::string if_name("test_net0");
+ AddInterface(kClientAddr, if_name);
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_EQ(4U, ports_.size());
+ EXPECT_TRUE(candidate_allocation_done_);
+ candidate_allocation_done_ = false;
+ candidates_.clear();
+ ports_.clear();
+
+ RemoveInterface(kClientAddr);
+ ASSERT_EQ_WAIT(0U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_EQ(0U, ports_.size());
+ EXPECT_FALSE(candidate_allocation_done_);
+
+ // When the same interfaces are added again, new candidates/ports should be
+ // generated.
+ AddInterface(kClientAddr, if_name);
+ ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_EQ(4U, ports_.size());
+ EXPECT_TRUE(candidate_allocation_done_);
+}
+
+// Test that when the same network interface is brought down and up, the
+// port allocator session will not restart a new allocation sequence if
+// it is stopped.
+TEST_F(PortAllocatorTest, TestSameNetworkDownAndUpWhenSessionStopped) {
+ std::string if_name("test_net0");
+ AddInterface(kClientAddr, if_name);
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ ASSERT_EQ_WAIT(7U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_EQ(4U, ports_.size());
+ EXPECT_TRUE(candidate_allocation_done_);
+ session_->StopGettingPorts();
+ candidates_.clear();
+ ports_.clear();
+
+ RemoveInterface(kClientAddr);
+ ASSERT_EQ_WAIT(0U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_EQ(0U, ports_.size());
+
+ // When the same interfaces are added again, new candidates/ports should not
+ // be generated because the session has stopped.
+ AddInterface(kClientAddr, if_name);
+ ASSERT_EQ_WAIT(0U, candidates_.size(), kDefaultAllocationTimeout);
+ EXPECT_EQ(0U, ports_.size());
+ EXPECT_TRUE(candidate_allocation_done_);
+}
+
// Verify candidates with default step delay of 1sec.
TEST_F(PortAllocatorTest, TestGetAllPortsWithOneSecondStepDelay) {
AddInterface(kClientAddr);
