Don't delete an ICE connection until it has been pruned or timed out on writing in the case where it

hasn't received anything yet.  Deleting an ICE connection before it is pruned or timed out
when it hasn't received anything yet leads to ICE connections being deleted
before they have a chance to send a ping and receive a response.
BUG=

Review URL: https://codereview.webrtc.org/1544003002

Cr-Commit-Position: refs/heads/master@{#11151}
This commit is contained in:
honghaiz
2016-01-04 21:57:33 -08:00
committed by Commit bot
parent e2976c87f7
commit 37389b42b4
2 changed files with 30 additions and 12 deletions

View File

@ -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 {

View File

@ -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();