From 2cd7afe7e2dc011ab00bdbc131039b16aa8fbdeb Mon Sep 17 00:00:00 2001
From: Honghai Zhang <honghaiz@webrtc.org>
Date: Thu, 12 Nov 2015 11:14:33 -0800
Subject: [PATCH] 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-Commit-Position: refs/heads/master@{#10626}
---
 webrtc/p2p/base/port.cc          | 16 +++---------
 webrtc/p2p/base/port.h           |  6 ++++-
 webrtc/p2p/base/port_unittest.cc | 45 ++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 13 deletions(-)

diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index f04e619426..90275aa141 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -1127,17 +1127,9 @@ bool Connection::dead(uint32_t now) const {
     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 @@ void Connection::OnMessage(rtc::Message *pmsg) {
   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/webrtc/p2p/base/port.h b/webrtc/p2p/base/port.h
index 2f69f49459..62ca3a50b7 100644
--- a/webrtc/p2p/base/port.h
+++ b/webrtc/p2p/base/port.h
@@ -54,6 +54,10 @@ extern const char TCPTYPE_SIMOPEN_STR[];
 // 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 @@ class Connection : public rtc::MessageHandler,
 
   // 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/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc
index 62ba572237..261da0aeae 100644
--- a/webrtc/p2p/base/port_unittest.cc
+++ b/webrtc/p2p/base/port_unittest.cc
@@ -1235,6 +1235,51 @@ TEST_F(PortTest, TestSslTcpToSslTcpRelay) {
 }
 */
 
+// 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.