Do not delete the turn port entry right away when the respective connection is deleted.

BUG=webrtc:5120

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

Cr-Commit-Position: refs/heads/master@{#10641}

webrtc/p2p/base/turnport.cc

@@ -140,6 +140,11 @@ class TurnEntry : public sigslot::has_slots<> {
   const rtc::SocketAddress& address() const { return ext_addr_; }
   BindState state() const { return state_; }
 
+  uint32_t destruction_timestamp() { return destruction_timestamp_; }
+  void set_destruction_timestamp(uint32_t destruction_timestamp) {
+    destruction_timestamp_ = destruction_timestamp;
+  }
+
   // Helper methods to send permission and channel bind requests.
   void SendCreatePermissionRequest(int delay);
   void SendChannelBindRequest(int delay);
@@ -160,6 +165,11 @@ class TurnEntry : public sigslot::has_slots<> {
   int channel_id_;
   rtc::SocketAddress ext_addr_;
   BindState state_;
+  // A non-zero value indicates that this entry is scheduled to be destroyed.
+  // It is also used as an ID of the event scheduling. When the destruction
+  // event actually fires, the TurnEntry will be destroyed only if the
+  // timestamp here matches the one in the firing event.
+  uint32_t destruction_timestamp_ = 0;
 };
 
 TurnPort::TurnPort(rtc::Thread* thread,
@@ -239,7 +249,7 @@ TurnPort::~TurnPort() {
   }
 
   while (!entries_.empty()) {
-    DestroyEntry(entries_.front()->address());
+    DestroyEntry(entries_.front());
   }
   if (resolver_) {
     resolver_->Destroy(false);
@@ -438,7 +448,7 @@ Connection* TurnPort::CreateConnection(const Candidate& address,
   }
 
   // Create an entry, if needed, so we can get our permissions set up correctly.
-  CreateEntry(address.address());
+  CreateOrRefreshEntry(address.address());
 
   // A TURN port will have two candiates, STUN and TURN. STUN may not
   // present in all cases. If present stun candidate will be added first
@@ -713,7 +723,6 @@ void TurnPort::OnMessage(rtc::Message* message) {
     }
     return;
   }
-
   Port::OnMessage(message);
 }
 
@@ -898,24 +907,55 @@ TurnEntry* TurnPort::FindEntry(int channel_id) const {
   return (it != entries_.end()) ? *it : NULL;
 }
 
-TurnEntry* TurnPort::CreateEntry(const rtc::SocketAddress& addr) {
+void TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr) {
-  ASSERT(FindEntry(addr) == NULL);
+  TurnEntry* entry = FindEntry(addr);
-  TurnEntry* entry = new TurnEntry(this, next_channel_number_++, addr);
+  if (entry == nullptr) {
-  entries_.push_back(entry);
+    entry = new TurnEntry(this, next_channel_number_++, addr);
-  return entry;
+    entries_.push_back(entry);
+  } else {
+    // The channel binding request for the entry will be refreshed automatically
+    // until the entry is destroyed.
+    CancelEntryDestruction(entry);
+  }
 }
 
-void TurnPort::DestroyEntry(const rtc::SocketAddress& addr) {
+void TurnPort::DestroyEntry(TurnEntry* entry) {
-  TurnEntry* entry = FindEntry(addr);
   ASSERT(entry != NULL);
   entry->SignalDestroyed(entry);
   entries_.remove(entry);
   delete entry;
 }
 
+void TurnPort::DestroyEntryIfNotCancelled(TurnEntry* entry,
+                                          uint32_t timestamp) {
+  bool cancelled = timestamp != entry->destruction_timestamp();
+  if (!cancelled) {
+    DestroyEntry(entry);
+  }
+}
+
 void TurnPort::OnConnectionDestroyed(Connection* conn) {
-  // Destroying TurnEntry for the connection, which is already destroyed.
+  // Schedule an event to destroy TurnEntry for the connection, which is
-  DestroyEntry(conn->remote_candidate().address());
+  // already destroyed.
+  const rtc::SocketAddress& remote_address = conn->remote_candidate().address();
+  TurnEntry* entry = FindEntry(remote_address);
+  ASSERT(entry != NULL);
+  ScheduleEntryDestruction(entry);
+}
+
+void TurnPort::ScheduleEntryDestruction(TurnEntry* entry) {
+  ASSERT(entry->destruction_timestamp() == 0);
+  uint32_t timestamp = rtc::Time();
+  entry->set_destruction_timestamp(timestamp);
+  invoker_.AsyncInvokeDelayed<void>(
+      thread(),
+      rtc::Bind(&TurnPort::DestroyEntryIfNotCancelled, this, entry, timestamp),
+      TURN_PERMISSION_TIMEOUT);
+}
+
+void TurnPort::CancelEntryDestruction(TurnEntry* entry) {
+  ASSERT(entry->destruction_timestamp() != 0);
+  entry->set_destruction_timestamp(0);
 }
 
 TurnAllocateRequest::TurnAllocateRequest(TurnPort* port)

webrtc/p2p/base/turnport.h

@@ -16,9 +16,10 @@
 #include <set>
 #include <string>
 
+#include "webrtc/base/asyncinvoker.h"
+#include "webrtc/base/asyncpacketsocket.h"
 #include "webrtc/p2p/base/port.h"
 #include "webrtc/p2p/client/basicportallocator.h"
-#include "webrtc/base/asyncpacketsocket.h"
 
 namespace rtc {
 class AsyncResolver;
@@ -122,6 +123,9 @@ class TurnPort : public Port {
     return socket_;
   }
 
+  // For testing only.
+  rtc::AsyncInvoker* invoker() { return &invoker_; }
+
   // Signal with resolved server address.
   // Parameters are port, server address and resolved server address.
   // This signal will be sent only if server address is resolved successfully.
@@ -215,8 +219,13 @@ class TurnPort : public Port {
   bool HasPermission(const rtc::IPAddress& ipaddr) const;
   TurnEntry* FindEntry(const rtc::SocketAddress& address) const;
   TurnEntry* FindEntry(int channel_id) const;
-  TurnEntry* CreateEntry(const rtc::SocketAddress& address);
+  void CreateOrRefreshEntry(const rtc::SocketAddress& address);
-  void DestroyEntry(const rtc::SocketAddress& address);
+  void DestroyEntry(TurnEntry* entry);
+  // Destroys the entry only if |timestamp| matches the destruction timestamp
+  // in |entry|.
+  void DestroyEntryIfNotCancelled(TurnEntry* entry, uint32_t timestamp);
+  void ScheduleEntryDestruction(TurnEntry* entry);
+  void CancelEntryDestruction(TurnEntry* entry);
   void OnConnectionDestroyed(Connection* conn);
 
   ProtocolAddress server_address_;
@@ -244,6 +253,8 @@ class TurnPort : public Port {
   // The number of retries made due to allocate mismatch error.
   size_t allocate_mismatch_retries_;
 
+  rtc::AsyncInvoker invoker_;
+
   friend class TurnEntry;
   friend class TurnAllocateRequest;
   friend class TurnRefreshRequest;

webrtc/p2p/base/turnport_unittest.cc

@@ -385,6 +385,48 @@ class TurnPortTest : public testing::Test,
     EXPECT_TRUE(conn2->receiving());
   }
 
+  void TestDestroyTurnConnection() {
+    turn_port_->PrepareAddress();
+    ASSERT_TRUE_WAIT(turn_ready_, kTimeout);
+    // Create a remote UDP port
+    CreateUdpPort();
+    udp_port_->PrepareAddress();
+    ASSERT_TRUE_WAIT(udp_ready_, kTimeout);
+
+    // Create connections on both ends.
+    Connection* conn1 = udp_port_->CreateConnection(turn_port_->Candidates()[0],
+                                                    Port::ORIGIN_MESSAGE);
+    Connection* conn2 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
+                                                     Port::ORIGIN_MESSAGE);
+    ASSERT_TRUE(conn2 != NULL);
+    ASSERT_TRUE_WAIT(turn_create_permission_success_, kTimeout);
+    // Make sure turn connection can receive.
+    conn1->Ping(0);
+    EXPECT_EQ_WAIT(Connection::STATE_WRITABLE, conn1->write_state(), kTimeout);
+    EXPECT_FALSE(turn_unknown_address_);
+
+    // Destroy the connection on the turn port. The TurnEntry is still
+    // there. So the turn port gets ping from unknown address if it is pinged.
+    conn2->Destroy();
+    conn1->Ping(0);
+    EXPECT_TRUE_WAIT(turn_unknown_address_, kTimeout);
+
+    // Flush all requests in the invoker to destroy the TurnEntry.
+    // Now the turn port cannot receive the ping.
+    turn_unknown_address_ = false;
+    turn_port_->invoker()->Flush(rtc::Thread::Current());
+    conn1->Ping(0);
+    rtc::Thread::Current()->ProcessMessages(500);
+    EXPECT_FALSE(turn_unknown_address_);
+
+    // If the connection is created again, it will start to receive pings.
+    conn2 = turn_port_->CreateConnection(udp_port_->Candidates()[0],
+                                         Port::ORIGIN_MESSAGE);
+    conn1->Ping(0);
+    EXPECT_TRUE_WAIT(conn2->receiving(), kTimeout);
+    EXPECT_FALSE(turn_unknown_address_);
+  }
+
   void TestTurnSendData() {
     turn_port_->PrepareAddress();
     EXPECT_TRUE_WAIT(turn_ready_, kTimeout);
@@ -694,6 +736,20 @@ TEST_F(TurnPortTest, TestTurnTcpConnection) {
   TestTurnConnection();
 }
 
+// Test that if a connection on a TURN port is destroyed, the TURN port can
+// still receive ping on that connection as if it is from an unknown address.
+// If the connection is created again, it will be used to receive ping.
+TEST_F(TurnPortTest, TestDestroyTurnConnection) {
+  CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
+  TestDestroyTurnConnection();
+}
+
+// Similar to above, except that this test will use the shared socket.
+TEST_F(TurnPortTest, TestDestroyTurnConnectionUsingSharedSocket) {
+  CreateSharedTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);
+  TestDestroyTurnConnection();
+}
+
 // Test that we fail to create a connection when we want to use TLS over TCP.
 // This test should be removed once we have TLS support.
 TEST_F(TurnPortTest, TestTurnTlsTcpConnectionFails) {