A few updates on connection states

webrtc/p2p/base/port.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.
  */
 
@@ skipping 773 matching lines @@
       remote_candidate_(remote_candidate),
       write_state_(STATE_WRITE_INIT),
       receiving_(false),
       connected_(true),
       pruned_(false),
       use_candidate_attr_(false),
       nominated_(false),
       remote_ice_mode_(ICEMODE_FULL),
       requests_(port->thread()),
       rtt_(DEFAULT_RTT),
+      time_created_(rtc::Time()),
       last_ping_sent_(0),
       last_ping_received_(0),
       last_data_received_(0),
       last_ping_response_received_(0),
       recv_rate_tracker_(100u, 10u),
       send_rate_tracker_(100u, 10u),
       sent_packets_discarded_(0),
       sent_packets_total_(0),
       reported_(false),
       state_(STATE_WAITING),
@@ skipping 38 matching lines @@
   return priority;
 }
 
 void Connection::set_write_state(WriteState value) {
   WriteState old_value = write_state_;
   write_state_ = value;
   if (value != old_value) {
     LOG_J(LS_VERBOSE, this) << "set_write_state from: " << old_value << " to "
                             << value;
     SignalStateChange(this);
-    CheckTimeout();
   }
 }
 
 void Connection::set_receiving(bool value) {
   if (value != receiving_) {
     LOG_J(LS_VERBOSE, this) << "set_receiving to " << value;
     receiving_ = value;
     SignalStateChange(this);
-    CheckTimeout();
   }
 }
 
 void Connection::set_state(State state) {
   State old_state = state_;
   state_ = state;
   if (state != old_state) {
     LOG_J(LS_VERBOSE, this) << "set_state";
   }
 }
@@ skipping 34 matching lines @@
     set_receiving(true);
     last_data_received_ = rtc::Time();
     recv_rate_tracker_.AddSamples(size);
     SignalReadPacket(this, data, size, packet_time);
 
     // If timed out sending writability checks, start up again
     if (!pruned_ && (write_state_ == STATE_WRITE_TIMEOUT)) {
       LOG(LS_WARNING) << "Received a data packet on a timed-out Connection. "
                       << "Resetting state to STATE_WRITE_INIT.";
       set_write_state(STATE_WRITE_INIT);
+      Unprune();

[pthatcher1, 2015/09/21 23:49:18]

if (!pruned_) Unprune()?

Why would we unprune if we're not pruned?

[honghaiz3, 2015/09/22 19:35:08]

If it is not pruned, then you don't need to unprune it. 
This makes it similar to the original behavior that when a connection receives a
data packet, it will be pulled out of the pruned state, so it will be available
for pinging again. 

     }
   } else if (!msg) {
     // The packet was STUN, but failed a check and was handled internally.
   } else {
     // The packet is STUN and passed the Port checks.
     // Perform our own checks to ensure this packet is valid.
     // If this is a STUN request, then update the readable bit and respond.
     // If this is a STUN response, then update the writable bit.
     // Log at LS_INFO if we receive a ping on an unwritable connection.
     rtc::LoggingSeverity sev = (!writable() ? rtc::LS_INFO : rtc::LS_VERBOSE);
@@ skipping 70 matching lines @@
   if (write_state_ == STATE_WRITABLE) {
     SignalReadyToSend(this);
   }
 }
 
 void Connection::Prune() {
   if (!pruned_) {
     LOG_J(LS_VERBOSE, this) << "Connection pruned";
     pruned_ = true;
     requests_.Clear();
-    set_write_state(STATE_WRITE_TIMEOUT);
   }
 }
 
+void Connection::Unprune() {
+  if (pruned_) {
+    LOG_J(LS_VERBOSE, this) << "Connection un-pruned";
+    pruned_ = false;
+  }
+}
+
 void Connection::Destroy() {
   LOG_J(LS_VERBOSE, this) << "Connection destroyed";
   port_->thread()->Post(this, MSG_DELETE);
 }
 
 void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) {
   std::ostringstream oss;
   oss << std::boolalpha;
   if (pings_since_last_response_.size() > max) {
     for (size_t i = 0; i < max; i++) {
@@ skipping 18 matching lines @@
     LOG_J(LS_VERBOSE, this) << "UpdateState()"
                             << ", ms since last received response="
                             << now - last_ping_response_received_
                             << ", ms since last received data="
                             << now - last_data_received_
                             << ", rtt=" << rtt
                             << ", pings_since_last_response=" << pings;
   }
 
   // Check the writable state.  (The order of these checks is important.)
+  // If the connection has not sent stun ping for a while (mostly because

[pthatcher1, 2015/09/21 23:49:18]

sent stun ping => sent a stun ping

[honghaiz3, 2015/09/22 19:35:08]

Done.

+  // it is pruned), it is considered stale.
   //
   // Before becoming unwritable, we allow for a fixed number of pings to fail
   // (i.e., receive no response).  We also have to give the response time to
   // get back, so we include a conservative estimate of this.
   //
   // Before timing out writability, we give a fixed amount of time.  This is to
   // allow for changes in network conditions.
 
-  if ((write_state_ == STATE_WRITABLE) &&
+  if (write_state_ == STATE_WRITABLE &&
+      last_ping_sent_ + CONNECTION_WRITE_STALE_TIMEOUT < now) {
+    LOG_J(LS_INFO, this) << "WriteState becomes stale";
+    set_write_state(STATE_WRITE_STALE);
+  }
+  if ((write_state_ == STATE_WRITABLE || write_state_ == STATE_WRITE_STALE) &&
       TooManyFailures(pings_since_last_response_,
                       CONNECTION_WRITE_CONNECT_FAILURES,
                       rtt,
                       now) &&
       TooLongWithoutResponse(pings_since_last_response_,
                              CONNECTION_WRITE_CONNECT_TIMEOUT,
                              now)) {
     uint32 max_pings = CONNECTION_WRITE_CONNECT_FAILURES;
     LOG_J(LS_INFO, this) << "Unwritable after " << max_pings
                          << " ping failures and "
@@ skipping 15 matching lines @@
                          << now - pings_since_last_response_[0].sent_time
                          << " ms without a response"
                          << ", rtt=" << rtt;
     set_write_state(STATE_WRITE_TIMEOUT);
   }
 
   // Check the receiving state.
   uint32 last_recv_time = last_received();
   bool receiving = now <= last_recv_time + receiving_timeout_;
   set_receiving(receiving);
+  if (!receiving) {
+    // If this connection has not received anything for a long time,
+    // destroy the connection. If the connection has never received anything,
+    // use the connection creating time as the starting time for calculating
+    // the receiving timeout.
+    if (now > std::max(last_recv_time, time_created_) +
+        DEAD_CONNECTION_RECEIVE_TIMEOUT) {
+      Destroy();
+    }
+  }
 }
 
 void Connection::Ping(uint32 now) {
   last_ping_sent_ = now;
   ConnectionRequest *req = new ConnectionRequest(this);
   pings_since_last_response_.push_back(SentPing(req->id(), now));
   LOG_J(LS_VERBOSE, this) << "Sending STUN ping "
                           << ", id=" << rtc::hex_encode(req->id());
   requests_.Send(req);
   state_ = STATE_INPROGRESS;
@@ skipping 25 matching lines @@
 
 std::string Connection::ToString() const {
   const char CONNECT_STATE_ABBREV[2] = {
     '-',  // not connected (false)
     'C',  // connected (true)
   };
   const char RECEIVE_STATE_ABBREV[2] = {
     '-',  // not receiving (false)
     'R',  // receiving (true)
   };
-  const char WRITE_STATE_ABBREV[4] = {
+  const char WRITE_STATE_ABBREV[5] = {
     'W',  // STATE_WRITABLE
+    's',  // STATE_WRITE_STALE

[pthatcher1, 2015/09/21 23:49:18]

Maybe a '?' would be good, since it means we don't know since we're not probing
any more.

[honghaiz3, 2015/09/22 19:35:08]

Dropped this (saved for a separate CL). 

     'w',  // STATE_WRITE_UNRELIABLE
     '-',  // STATE_WRITE_INIT
     'x',  // STATE_WRITE_TIMEOUT
   };
   const std::string ICESTATE[4] = {
     "W",  // STATE_WAITING
     "I",  // STATE_INPROGRESS
     "S",  // STATE_SUCCEEDED
     "F"   // STATE_FAILED
   };
@@ skipping 95 matching lines @@
 
 void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
   // Log at LS_INFO if we send a ping on an unwritable connection.
   rtc::LoggingSeverity sev = !writable() ? rtc::LS_INFO : rtc::LS_VERBOSE;
   bool use_candidate = use_candidate_attr();
   LOG_JV(sev, this) << "Sent STUN ping"
                     << ", id=" << rtc::hex_encode(request->id())
                     << ", use_candidate=" << use_candidate;
 }
 
-void Connection::CheckTimeout() {
-  // If write has timed out and it is not receiving, remove the connection.
-  if (!receiving_ && write_state_ == STATE_WRITE_TIMEOUT) {
-    port_->thread()->Post(this, MSG_DELETE);
-  }
-}
-
 void Connection::HandleRoleConflictFromPeer() {
   port_->SignalRoleConflict(port_);
 }
 
 void Connection::MaybeSetRemoteIceCredentials(const std::string& ice_ufrag,
                                               const std::string& ice_pwd) {
   if (remote_candidate_.username() == ice_ufrag &&
       remote_candidate_.password().empty()) {
     remote_candidate_.set_password(ice_pwd);
   }
@@ skipping 131 matching lines @@
     ASSERT(sent < 0);
     error_ = port_->GetError();
     sent_packets_discarded_++;
   } else {
     send_rate_tracker_.AddSamples(sent);
   }
   return sent;
 }
 
 }  // namespace cricket