Stop sending stun binding requests after certain amount of time.

webrtc/p2p/base/stunport.cc

 /*
  *  Copyright 2004 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.
  */
 
 #include "webrtc/p2p/base/stunport.h"
 
 #include "webrtc/p2p/base/common.h"
 #include "webrtc/p2p/base/portallocator.h"
 #include "webrtc/p2p/base/stun.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/common.h"
 #include "webrtc/base/helpers.h"
 #include "webrtc/base/ipaddress.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/nethelpers.h"
 
 namespace cricket {
 
 // TODO: Move these to a common place (used in relayport too)
 const int KEEPALIVE_DELAY = 10 * 1000;  // 10 seconds - sort timeouts
-const int RETRY_DELAY = 50;             // 50ms, from ICE spec
 const int RETRY_TIMEOUT = 50 * 1000;    // ICE says 50 secs
+// Stop sending STUN binding request after this amount of time (in milliseconds)

[pthatcher1, 2015/12/03 21:00:19]

request => requests

[honghaiz3, 2015/12/03 22:21:21]

Done.

+// because the connection binding requests should keep the NAT binding alive.
+const int KEEP_ALIVE_DEADLINE = 120 * 1000;

[pthatcher1, 2015/12/03 21:00:19]

2 * 60 * 1000;  // 2 minutes

Might be more clear

[honghaiz3, 2015/12/03 22:21:21]

Done.

 
 // Handles a binding request sent to the STUN server.
 class StunBindingRequest : public StunRequest {
  public:
-  StunBindingRequest(UDPPort* port, bool keep_alive,
+  StunBindingRequest(UDPPort* port,
+                     int keep_alive_deadline,
                      const rtc::SocketAddress& addr)
-    : port_(port), keep_alive_(keep_alive), server_addr_(addr) {
+      : port_(port),
+        keep_alive_deadline_(keep_alive_deadline),
+        server_addr_(addr) {
     start_time_ = rtc::Time();
   }
 
   virtual ~StunBindingRequest() {
   }
 
   const rtc::SocketAddress& server_addr() const { return server_addr_; }
 
   virtual void Prepare(StunMessage* request) override {
     request->SetType(STUN_BINDING_REQUEST);
   }
 
   virtual void OnResponse(StunMessage* response) override {
     const StunAddressAttribute* addr_attr =
         response->GetAddress(STUN_ATTR_MAPPED_ADDRESS);
     if (!addr_attr) {
       LOG(LS_ERROR) << "Binding response missing mapped address.";
     } else if (addr_attr->family() != STUN_ADDRESS_IPV4 &&
                addr_attr->family() != STUN_ADDRESS_IPV6) {
       LOG(LS_ERROR) << "Binding address has bad family";
     } else {
       rtc::SocketAddress addr(addr_attr->ipaddr(), addr_attr->port());
       port_->OnStunBindingRequestSucceeded(server_addr_, addr);
     }
 
     // We will do a keep-alive regardless of whether this request succeeds.
     // This should have almost no impact on network usage.
-    if (keep_alive_) {
+    if (rtc::TimeSince(port_->start_time()) <= keep_alive_deadline_) {

[pthatcher1, 2015/12/03 21:00:19]

I was thinking that "deadline" would be more an absolute time, not a relative
time.  In otherwords, "December 7th at 7pm PST", not "in 2 weeks".  The latter
is more like a "timeout" than a deadline".  I was thinking more like this:

if (now < keep_alive_deadline_) {
  // ...
}


That way the caller could specify deadline = now + timeout.

[honghaiz3, 2015/12/03 22:21:21]

Done.

       port_->requests_.SendDelayed(
           new StunBindingRequest(port_, true, server_addr_),
           port_->stun_keepalive_delay());
     }
   }
 
   virtual void OnErrorResponse(StunMessage* response) override {
     const StunErrorCodeAttribute* attr = response->GetErrorCode();
     if (!attr) {
       LOG(LS_ERROR) << "Bad allocate response error code";
     } else {
       LOG(LS_ERROR) << "Binding error response:"
                  << " class=" << attr->eclass()
                  << " number=" << attr->number()
                  << " reason='" << attr->reason() << "'";
     }
 
     port_->OnStunBindingOrResolveRequestFailed(server_addr_);
 
-    if (keep_alive_
-        && (rtc::TimeSince(start_time_) <= RETRY_TIMEOUT)) {
+    if (rtc::TimeSince(port_->start_time()) <= keep_alive_deadline_ &&
+        rtc::TimeSince(start_time_) <= RETRY_TIMEOUT) {
       port_->requests_.SendDelayed(
           new StunBindingRequest(port_, true, server_addr_),
           port_->stun_keepalive_delay());
     }
   }
 
   virtual void OnTimeout() override {
     LOG(LS_ERROR) << "Binding request timed out from "
       << port_->GetLocalAddress().ToSensitiveString()
       << " (" << port_->Network()->name() << ")";
 
     port_->OnStunBindingOrResolveRequestFailed(server_addr_);
-
-    if (keep_alive_
-        && (rtc::TimeSince(start_time_) <= RETRY_TIMEOUT)) {
-      port_->requests_.SendDelayed(
-          new StunBindingRequest(port_, true, server_addr_),
-          RETRY_DELAY);
-    }
   }
 
  private:
   UDPPort* port_;
-  bool keep_alive_;
+  int keep_alive_deadline_;
   const rtc::SocketAddress server_addr_;
   uint32_t start_time_;
 };
 
 UDPPort::AddressResolver::AddressResolver(
     rtc::PacketSocketFactory* factory)
     : socket_factory_(factory) {}
 
 UDPPort::AddressResolver::~AddressResolver() {
   for (ResolverMap::iterator it = resolvers_.begin();
@@ skipping 56 matching lines @@
            factory,
            network,
            socket->GetLocalAddress().ipaddr(),
            username,
            password),
       requests_(thread),
       socket_(socket),
       error_(0),
       ready_(false),
       stun_keepalive_delay_(KEEPALIVE_DELAY),
+      start_time_(rtc::Time()),
       emit_local_for_anyaddress_(emit_local_for_anyaddress) {
   requests_.set_origin(origin);
 }
 
 UDPPort::UDPPort(rtc::Thread* thread,
                  rtc::PacketSocketFactory* factory,
                  rtc::Network* network,
                  const rtc::IPAddress& ip,
                  uint16_t min_port,
                  uint16_t max_port,
                  const std::string& username,
                  const std::string& password,
                  const std::string& origin,
                  bool emit_local_for_anyaddress)
     : Port(thread,
            LOCAL_PORT_TYPE,
            factory,
            network,
            ip,
            min_port,
            max_port,
            username,
            password),
       requests_(thread),
       socket_(NULL),
       error_(0),
       ready_(false),
       stun_keepalive_delay_(KEEPALIVE_DELAY),
+      start_time_(rtc::Time()),
       emit_local_for_anyaddress_(emit_local_for_anyaddress) {
   requests_.set_origin(origin);
 }
 
 bool UDPPort::Init() {
   if (!SharedSocket()) {
     ASSERT(socket_ == NULL);
     socket_ = socket_factory()->CreateUdpSocket(
         rtc::SocketAddress(ip(), 0), min_port(), max_port());
     if (!socket_) {
@@ skipping 16 matching lines @@
 
 void UDPPort::PrepareAddress() {
   ASSERT(requests_.empty());
   if (socket_->GetState() == rtc::AsyncPacketSocket::STATE_BOUND) {
     OnLocalAddressReady(socket_, socket_->GetLocalAddress());
   }
 }
 
 void UDPPort::MaybePrepareStunCandidate() {
   // Sending binding request to the STUN server if address is available to
   // prepare STUN candidate.

[pthatcher1, 2015/12/03 21:00:57]

This comment is no longer true.

[honghaiz3, 2015/12/03 22:21:21]

I don't understand. why? 

   if (!server_addresses_.empty()) {
     SendStunBindingRequests();
   } else {
     // Port is done allocating candidates.
     MaybeSetPortCompleteOrError();
   }
 }
 
 Connection* UDPPort::CreateConnection(const Candidate& address,
                                        CandidateOrigin origin) {
@@ skipping 130 matching lines @@
   }
 }
 
 void UDPPort::SendStunBindingRequest(const rtc::SocketAddress& stun_addr) {
   if (stun_addr.IsUnresolvedIP()) {
     ResolveStunAddress(stun_addr);
 
   } else if (socket_->GetState() == rtc::AsyncPacketSocket::STATE_BOUND) {
     // Check if |server_addr_| is compatible with the port's ip.
     if (IsCompatibleAddress(stun_addr)) {
-      requests_.Send(new StunBindingRequest(this, true, stun_addr));
+      requests_.Send(
+          new StunBindingRequest(this, KEEP_ALIVE_DEADLINE, stun_addr));
     } else {
       // Since we can't send stun messages to the server, we should mark this
       // port ready.
       LOG(LS_WARNING) << "STUN server address is incompatible.";
       OnStunBindingOrResolveRequestFailed(stun_addr);
     }
   }
 }
 
 bool UDPPort::MaybeSetDefaultLocalAddress(rtc::SocketAddress* addr) const {
@@ skipping 93 matching lines @@
   const std::vector<Candidate>& existing_candidates = Candidates();
   std::vector<Candidate>::const_iterator it = existing_candidates.begin();
   for (; it != existing_candidates.end(); ++it) {
     if (it->address() == addr)
       return true;
   }
   return false;
 }
 
 }  // namespace cricket