diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc index ddf38dcb18..f6a30097f8 100644 --- a/webrtc/p2p/base/port.cc +++ b/webrtc/p2p/base/port.cc @@ -1124,17 +1124,28 @@ void Connection::ReceivedPingResponse() { } bool Connection::dead(uint32_t now) const { - if (now < (time_created_ms_ + MIN_CONNECTION_LIFETIME)) { - // A connection that hasn't passed its minimum lifetime is still alive. - // We do this to prevent connections from being pruned too quickly - // during a network change event when two networks would be up - // simultaneously but only for a brief period. + if (last_received() > 0) { + // If it has ever received anything, we keep it alive until it hasn't + // received anything for DEAD_CONNECTION_RECEIVE_TIMEOUT. This covers the + // normal case of a successfully used connection that stops working. This + // also allows a remote peer to continue pinging over a locally inactive + // (pruned) connection. + return (now > (last_received() + DEAD_CONNECTION_RECEIVE_TIMEOUT)); + } + + if (active()) { + // If it has never received anything, keep it alive as long as it is + // actively pinging and not pruned. Otherwise, the connection might be + // deleted before it has a chance to ping. This is the normal case for a + // new connection that is pinging but hasn't received anything yet. return false; } - // It is dead if it has not received anything for - // DEAD_CONNECTION_RECEIVE_TIMEOUT milliseconds. - return (now > (last_received() + DEAD_CONNECTION_RECEIVE_TIMEOUT)); + // If it has never received anything and is not actively pinging (pruned), we + // keep it around for at least MIN_CONNECTION_LIFETIME to prevent connections + // from being pruned too quickly during a network change event when two + // networks would be up simultaneously but only for a brief period. + return now > (time_created_ms_ + MIN_CONNECTION_LIFETIME); } std::string Connection::ToDebugId() const { diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc index 4bff58adc7..34806d9a79 100644 --- a/webrtc/p2p/base/port_unittest.cc +++ b/webrtc/p2p/base/port_unittest.cc @@ -1255,19 +1255,26 @@ TEST_F(PortTest, TestConnectionDead) { ASSERT_EQ_WAIT(1, ch1.complete_count(), kTimeout); ASSERT_EQ_WAIT(1, ch2.complete_count(), kTimeout); + // Test case that the connection has never received anything. uint32_t before_created = rtc::Time(); ch1.CreateConnection(GetCandidate(port2)); uint32_t after_created = rtc::Time(); Connection* conn = ch1.conn(); ASSERT(conn != nullptr); - // If the connection has never received anything, it will be dead after - // MIN_CONNECTION_LIFETIME - conn->UpdateState(before_created + MIN_CONNECTION_LIFETIME - 1); - rtc::Thread::Current()->ProcessMessages(100); + // It is not dead if it is after MIN_CONNECTION_LIFETIME but not pruned. + conn->UpdateState(after_created + MIN_CONNECTION_LIFETIME + 1); + rtc::Thread::Current()->ProcessMessages(0); EXPECT_TRUE(ch1.conn() != nullptr); + // It is not dead if it is before MIN_CONNECTION_LIFETIME and pruned. + conn->UpdateState(before_created + MIN_CONNECTION_LIFETIME - 1); + conn->Prune(); + rtc::Thread::Current()->ProcessMessages(0); + EXPECT_TRUE(ch1.conn() != nullptr); + // It will be dead after MIN_CONNECTION_LIFETIME and pruned. conn->UpdateState(after_created + MIN_CONNECTION_LIFETIME + 1); EXPECT_TRUE_WAIT(ch1.conn() == nullptr, kTimeout); + // Test case that the connection has received something. // Create a connection again and receive a ping. ch1.CreateConnection(GetCandidate(port2)); conn = ch1.conn();