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

webrtc/p2p/base/turnport.cc

 /*
  *  Copyright 2012 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 122 matching lines @@
   enum BindState { STATE_UNBOUND, STATE_BINDING, STATE_BOUND };
   TurnEntry(TurnPort* port, int channel_id,
             const rtc::SocketAddress& ext_addr);
 
   TurnPort* port() { return port_; }
 
   int channel_id() const { return channel_id_; }
   const rtc::SocketAddress& address() const { return ext_addr_; }
   BindState state() const { return state_; }
 
+  uint32_t msg_id() { return msg_id_; }
+  void set_msg_id(uint32_t msg_id) { msg_id_ = msg_id; }
+
   // Helper methods to send permission and channel bind requests.
   void SendCreatePermissionRequest();
   void SendChannelBindRequest(int delay);
   // Sends a packet to the given destination address.
   // This will wrap the packet in STUN if necessary.
   int Send(const void* data, size_t size, bool payload,
            const rtc::PacketOptions& options);
 
   void OnCreatePermissionSuccess();
   void OnCreatePermissionError(StunMessage* response, int code);
   void OnChannelBindSuccess();
   void OnChannelBindError(StunMessage* response, int code);
   // Signal sent when TurnEntry is destroyed.
   sigslot::signal1<TurnEntry*> SignalDestroyed;
 
  private:
   TurnPort* port_;
   int channel_id_;
   rtc::SocketAddress ext_addr_;
   BindState state_;
+  // If this is not zero, there is a delayed message that will destroy the
+  // TurnEntry. |msg_id_| is the message id, used to cancel the message.
+  uint32_t msg_id_ = 0;

[pthatcher1, 2015/11/11 01:08:41]

I think this would be more clear as delayed_destroy_msg_id_. 

And the comment would be more clear as "If set, this entry is schedule to be
destroyed with the following message ID.  The message ID can be used to cancel
the scheduled destruction.".

But even then, I'm not quite sure putting in on the TurnEntry is the right
option.  I think it might make more sense to have a table in the port of id =>
turn_entry.  Basically std::map<int, TurnEntry*>.  Then, check the map to see if
it's scheduled for destruction or not.  Then we don't need to complicated the
TurnEntry class.

[honghaiz3, 2015/11/11 23:02:55]

If we use the map, the map would have to be map<TurnEntry*, int> to map the
Entry to the message id (because later, we need to find the message id from the
Found TurnEntry). Using a field in the TurnEntry is like a more direct way of
mapping the turn entry to the message id without the complication of maps. Using
a map for this may have other complications. For example, we will need to
maintain the consistence of the map and the existing TurnEntries (if an entry is
destroyed, also need to delete the one in the map, etc).  

 };
 
 TurnPort::TurnPort(rtc::Thread* thread,
                    rtc::PacketSocketFactory* factory,
                    rtc::Network* network,
                    rtc::AsyncPacketSocket* socket,
                    const std::string& username,
                    const std::string& password,
                    const ProtocolAddress& server_address,
                    const RelayCredentials& credentials,
@@ skipping 258 matching lines @@
 
   if (!IsCompatibleAddress(address.address())) {
     return NULL;
   }
 
   if (state_ == STATE_DISCONNECTED) {
     return NULL;
   }
 
   // Create an entry, if needed, so we can get our permissions set up correctly.
-  CreateEntry(address.address());
+  CreateEntryIfNeeded(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
   // and TURN candidate later.
   for (size_t index = 0; index < Candidates().size(); ++index) {
     if (Candidates()[index].type() == RELAY_PORT_TYPE) {
       ProxyConnection* conn = new ProxyConnection(this, index, address);
       conn->SignalDestroyed.connect(this, &TurnPort::OnConnectionDestroyed);
       AddConnection(conn);
       return conn;
@@ skipping 254 matching lines @@
       // with the alternate server. PrepareAddress will send stun binding once
       // the new socket is connected.
       ASSERT(server_address().proto == PROTO_TCP);
       ASSERT(!SharedSocket());
       delete socket_;
       socket_ = NULL;
       PrepareAddress();
     }
     return;
   }
-
   Port::OnMessage(message);
 }
 
 void TurnPort::OnAllocateRequestTimeout() {
   OnAllocateError();
 }
 
 void TurnPort::HandleDataIndication(const char* data, size_t size,
                                     const rtc::PacketTime& packet_time) {
   // Read in the message, and process according to RFC5766, Section 10.4.
@@ skipping 164 matching lines @@
 
 static bool MatchesChannelId(TurnEntry* e, int id) {
   return e->channel_id() == id;
 }
 TurnEntry* TurnPort::FindEntry(int channel_id) const {
   EntryList::const_iterator it = std::find_if(entries_.begin(), entries_.end(),
       std::bind2nd(std::ptr_fun(MatchesChannelId), channel_id));
   return (it != entries_.end()) ? *it : NULL;
 }
 
-TurnEntry* TurnPort::CreateEntry(const rtc::SocketAddress& addr) {
-  ASSERT(FindEntry(addr) == NULL);
-  TurnEntry* entry = new TurnEntry(this, next_channel_number_++, addr);
-  entries_.push_back(entry);
+TurnEntry* TurnPort::CreateEntryIfNeeded(const rtc::SocketAddress& addr) {

[pthatcher1, 2015/11/11 01:08:41]

I think it would make sense to call this GetOrCreateEntry.  We have similarly
named things throughout the code base.

[honghaiz3, 2015/11/11 23:02:55]

Done.

+  TurnEntry* entry = FindEntry(addr);
+  if (entry == nullptr) {
+    entry = new TurnEntry(this, next_channel_number_++, addr);
+    entries_.push_back(entry);
+  } else {
+    ASSERT(entry->msg_id() != 0);
+    thread()->Clear(&invoker_, entry->msg_id());

[pthatcher1, 2015/11/11 01:08:41]

I feel like this logic should be in its own method, perhaps something like
CancelEntryDestruction(entry).  We could have a companion method
"ScheduleEntryDestruction(entry)".

[honghaiz3, 2015/11/11 23:02:55]

Done.

+    entry->set_msg_id(0);
+  }
   return entry;
 }
 
 void TurnPort::DestroyEntry(const rtc::SocketAddress& addr) {
   TurnEntry* entry = FindEntry(addr);
   ASSERT(entry != NULL);
   entry->SignalDestroyed(entry);
   entries_.remove(entry);
   delete entry;
 }
 
 void TurnPort::OnConnectionDestroyed(Connection* conn) {
   // Destroying TurnEntry for the connection, which is already destroyed.
-  DestroyEntry(conn->remote_candidate().address());
+  rtc::SocketAddress remote_address = conn->remote_candidate().address();
+  TurnEntry* entry = FindEntry(remote_address);
+  ASSERT(entry != NULL && entry->msg_id() == 0);
+  ++last_msg_id_;
+  entry->set_msg_id(last_msg_id_);
+  invoker_.AsyncInvokeDelayed<void>(
+      thread(), rtc::Bind(&TurnPort::DestroyEntry, this, remote_address),
+      TURN_PERMISSION_TIMEOUT, last_msg_id_);

[pthatcher1, 2015/11/11 01:08:41]

I think this is where a method called "ScheduleEntryDestruction(entry,
TURN_PERMISSION_TIMEOUT)" would makes sense.

[honghaiz3, 2015/11/11 23:02:55]

Done.

 }
 
 TurnAllocateRequest::TurnAllocateRequest(TurnPort* port)
     : StunRequest(new TurnMessage()),
       port_(port) {
 }
 
 void TurnAllocateRequest::Prepare(StunMessage* request) {
   // Create the request as indicated in RFC 5766, Section 6.1.
   request->SetType(TURN_ALLOCATE_REQUEST);
@@ skipping 433 matching lines @@
   // bind request as per http://tools.ietf.org/html/rfc5766#section-11.3
   if (code == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
       // Send channel bind request with fresh nonce.
       SendChannelBindRequest(0);
     }
   }
 }
 
 }  // namespace cricket