Do not delete a connection until it has not received anything for 30 seconds.
BUG=
R=pthatcher@webrtc.org
Review URL: https://codereview.webrtc.org/1422623015 .
Cr-Original-Commit-Position: refs/heads/master@{#10626}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 2cd7afe7e2dc011ab00bdbc131039b16aa8fbdeb
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index f04e619..90275aa 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -1127,17 +1127,9 @@
return false;
}
- if (receiving_) {
- // A connection that is receiving is alive.
- return false;
- }
-
- // A connection is alive until it is inactive.
- return !active();
-
- // TODO(honghaiz): Move from using the write state to using the receiving
- // state with something like the following:
- // return (now > (last_received() + DEAD_CONNECTION_RECEIVE_TIMEOUT));
+ // It is dead if it has not received anything for
+ // DEAD_CONNECTION_RECEIVE_TIMEOUT milliseconds.
+ return (now > (last_received() + DEAD_CONNECTION_RECEIVE_TIMEOUT));
}
std::string Connection::ToDebugId() const {
@@ -1304,7 +1296,7 @@
delete this;
}
-uint32_t Connection::last_received() {
+uint32_t Connection::last_received() const {
return std::max(last_data_received_,
std::max(last_ping_received_, last_ping_response_received_));
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 2f69f49..62ca3a5 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -54,6 +54,10 @@
// it.
const uint32_t MIN_CONNECTION_LIFETIME = 10 * 1000; // 10 seconds.
+// A connection will be declared dead if it has not received anything for this
+// long.
+const uint32_t DEAD_CONNECTION_RECEIVE_TIMEOUT = 30 * 1000; // 30 seconds.
+
// The timeout duration when a connection does not receive anything.
const uint32_t WEAK_CONNECTION_RECEIVE_TIMEOUT = 2500; // 2.5 seconds
@@ -559,7 +563,7 @@
// Returns the last received time of any data, stun request, or stun
// response in milliseconds
- uint32_t last_received();
+ uint32_t last_received() const;
protected:
enum { MSG_DELETE = 0, MSG_FIRST_AVAILABLE };
diff --git a/p2p/base/port_unittest.cc b/p2p/base/port_unittest.cc
index 62ba572..261da0a 100644
--- a/p2p/base/port_unittest.cc
+++ b/p2p/base/port_unittest.cc
@@ -1235,6 +1235,51 @@
}
*/
+// Test that a connection will be dead and deleted if
+// i) it has never received anything for MIN_CONNECTION_LIFETIME milliseconds
+// since it was created, or
+// ii) it has not received anything for DEAD_CONNECTION_RECEIVE_TIMEOUT
+// milliseconds since last receiving.
+TEST_F(PortTest, TestConnectionDead) {
+ UDPPort* port1 = CreateUdpPort(kLocalAddr1);
+ UDPPort* port2 = CreateUdpPort(kLocalAddr2);
+ TestChannel ch1(port1);
+ TestChannel ch2(port2);
+ // Acquire address.
+ ch1.Start();
+ ch2.Start();
+ ASSERT_EQ_WAIT(1, ch1.complete_count(), kTimeout);
+ ASSERT_EQ_WAIT(1, ch2.complete_count(), kTimeout);
+
+ 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);
+ EXPECT_TRUE(ch1.conn() != nullptr);
+ conn->UpdateState(after_created + MIN_CONNECTION_LIFETIME + 1);
+ EXPECT_TRUE_WAIT(ch1.conn() == nullptr, kTimeout);
+
+ // Create a connection again and receive a ping.
+ ch1.CreateConnection(GetCandidate(port2));
+ conn = ch1.conn();
+ ASSERT(conn != nullptr);
+ uint32_t before_last_receiving = rtc::Time();
+ conn->ReceivedPing();
+ uint32_t after_last_receiving = rtc::Time();
+ // The connection will be dead after DEAD_CONNECTION_RECEIVE_TIMEOUT
+ conn->UpdateState(
+ before_last_receiving + DEAD_CONNECTION_RECEIVE_TIMEOUT - 1);
+ rtc::Thread::Current()->ProcessMessages(100);
+ EXPECT_TRUE(ch1.conn() != nullptr);
+ conn->UpdateState(after_last_receiving + DEAD_CONNECTION_RECEIVE_TIMEOUT + 1);
+ EXPECT_TRUE_WAIT(ch1.conn() == nullptr, kTimeout);
+}
+
// This test case verifies standard ICE features in STUN messages. Currently it
// verifies Message Integrity attribute in STUN messages and username in STUN
// binding request will have colon (":") between remote and local username.
