diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index b6173ebf45..535dd90f99 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -645,7 +645,7 @@ void Port::SendBindingErrorResponse(StunMessage* request, } void Port::OnMessage(rtc::Message *pmsg) { - ASSERT(pmsg->message_id == MSG_DEAD); + ASSERT(pmsg->message_id == MSG_CHECK_DEAD); if (dead()) { Destroy(); } @@ -702,12 +702,19 @@ void Port::OnConnectionDestroyed(Connection* conn) { // On the controlled side, ports time out after all connections fail. // Note: If a new connection is added after this message is posted, but it // fails and is removed before kPortTimeoutDelay, then this message will - // still cause the Port to be destroyed. - if (dead()) { - thread_->PostDelayed(RTC_FROM_HERE, timeout_delay_, this, MSG_DEAD); + // not cause the Port to be destroyed. + if (ice_role_ == ICEROLE_CONTROLLED && connections_.empty()) { + last_time_all_connections_removed_ = rtc::TimeMillis(); + thread_->PostDelayed(RTC_FROM_HERE, timeout_delay_, this, MSG_CHECK_DEAD); } } +bool Port::dead() const { + return ice_role_ == ICEROLE_CONTROLLED && connections_.empty() && + rtc::TimeMillis() - last_time_all_connections_removed_ >= + timeout_delay_; +} + void Port::Destroy() { ASSERT(connections_.empty()); LOG_J(LS_INFO, this) << "Port deleted"; diff --git a/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h index 37926c9556..811ffed2cc 100644 --- a/webrtc/p2p/base/port.h +++ b/webrtc/p2p/base/port.h @@ -297,10 +297,7 @@ class Port : public PortInterface, public rtc::MessageHandler, int16_t network_cost() const { return network_cost_; } protected: - enum { - MSG_DEAD = 0, - MSG_FIRST_AVAILABLE - }; + enum { MSG_CHECK_DEAD = 0, MSG_FIRST_AVAILABLE }; virtual void UpdateNetworkCost(); @@ -359,9 +356,7 @@ class Port : public PortInterface, public rtc::MessageHandler, // Whether this port is dead, and hence, should be destroyed on the controlled // side. - bool dead() const { - return ice_role_ == ICEROLE_CONTROLLED && connections_.empty(); - } + bool dead() const; void OnNetworkTypeChanged(const rtc::Network* network); @@ -401,6 +396,7 @@ class Port : public PortInterface, public rtc::MessageHandler, // (WiFi. vs. Cellular). It takes precedence over the priority when // comparing two connections. uint16_t network_cost_; + int64_t last_time_all_connections_removed_ = 0; friend class Connection; }; diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index e156e017fe..e84af9f2be 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -2592,15 +2592,17 @@ TEST_F(PortTest, TestControllingNoTimeout) { } // This test case verifies that the CONTROLLED port does time out, but only -// after connectivity is lost. +// after connectivity is lost and no connection was created during the timeout +// period. TEST_F(PortTest, TestControlledTimeout) { + rtc::ScopedFakeClock clock; UDPPort* port1 = CreateUdpPort(kLocalAddr1); port1->SetIceRole(cricket::ICEROLE_CONTROLLING); port1->SetIceTiebreaker(kTiebreaker1); UDPPort* port2 = CreateUdpPort(kLocalAddr2); ConnectToSignalDestroyed(port2); - port2->set_timeout_delay(10); // milliseconds + port2->set_timeout_delay(100); // milliseconds port2->SetIceRole(cricket::ICEROLE_CONTROLLED); port2->SetIceTiebreaker(kTiebreaker2); @@ -2617,8 +2619,21 @@ TEST_F(PortTest, TestControlledTimeout) { // Simulate a connection that succeeds, and then is destroyed. StartConnectAndStopChannels(&ch1, &ch2); - // The controlled port should be destroyed after 10 milliseconds. - EXPECT_TRUE_WAIT(destroyed(), kTimeout); + SIMULATED_WAIT(false, 80, clock); + ch2.CreateConnection(GetCandidate(ch1.port())); + + // ch2 creates a connection so it will not be destroyed. + SIMULATED_WAIT(destroyed(), 80, clock); + EXPECT_FALSE(destroyed()); + + // Even if ch2 stops now, it won't be destroyed until 100ms after the + // connection is destroyed. + ch2.Stop(); + SIMULATED_WAIT(destroyed(), 80, clock); + EXPECT_FALSE(destroyed()); + + // The controlled port should be destroyed after timeout. + EXPECT_TRUE_SIMULATED_WAIT(destroyed(), 30, clock); } // This test case verifies that if the role of a port changes from controlled