Correctly handle SPDY GOAWAY frames.

net/spdy/spdy_session.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_session.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/basictypes.h"
@@ skipping 1114 matching lines @@
     // We sent the frame successfully.
     OnWriteComplete(rv);
 
     // TODO(mbelshe):  Test this error case.  Maybe we should mark the socket
     //                 as in an error state.
     if (rv < 0)
       break;
   }
 }
 
-void SpdySession::CloseAllStreams(net::Error status) {
-  base::StatsCounter abandoned_streams("spdy.abandoned_streams");
-  base::StatsCounter abandoned_push_streams(
-      "spdy.abandoned_push_streams");
-
-  if (!active_streams_.empty())
-    abandoned_streams.Add(active_streams_.size());
-  if (!unclaimed_pushed_streams_.empty()) {
-    streams_abandoned_count_ += unclaimed_pushed_streams_.size();
-    abandoned_push_streams.Add(unclaimed_pushed_streams_.size());
-    unclaimed_pushed_streams_.clear();
-  }
-
+void SpdySession::CloseAllStreamsAfter(SpdyStreamId last_good_stream_id,
+                                       net::Error status) {
   for (int i = 0; i < NUM_PRIORITIES; ++i) {
     PendingStreamRequestQueue queue;
     queue.swap(pending_create_stream_queues_[i]);
     for (PendingStreamRequestQueue::const_iterator it = queue.begin();
          it != queue.end(); ++it) {
       (*it)->OnRequestComplete(NULL, ERR_ABORTED);
     }
   }
 
-  while (!active_streams_.empty()) {
-    ActiveStreamMap::iterator it = active_streams_.begin();
+  ActiveStreamMap::iterator it =
+      active_streams_.lower_bound(last_good_stream_id + 1);
+  while (it != active_streams_.end()) {
     const scoped_refptr<SpdyStream>& stream = it->second;
+    ++it;
+    if (stream->stream_id() <= last_good_stream_id) {

[akalin, 2013/04/18 01:29:59]

you don't need this check, right? Since ActiveStreamMap is ordered.

[Ryan Hamilton, 2013/04/18 17:18:29]

Hah!  Yes :>  Done.

+      continue;
+    }
     LogAbandonedStream(stream, status);
     DeleteStream(stream->stream_id(), status);
+    streams_abandoned_count_++;

[akalin, 2013/04/18 01:29:59]

hunh. we weren't incrementing this correctly? i see.

[Ryan Hamilton, 2013/04/18 17:18:29]

Before I got started?  I think we were doing something like:

abandoned_streams.Add(active_streams_.size());

At least that was the state when I started hacking.  Looks like the name of that
var changed in a recent CL.

   }
 
   while (!created_streams_.empty()) {
     CreatedStreamSet::iterator it = created_streams_.begin();
     const scoped_refptr<SpdyStream> stream = *it;
     created_streams_.erase(it);
     LogAbandonedStream(stream, status);
     stream->OnClose(status);
   }
 
-  write_queue_.Clear();
+  write_queue_.RemovePendingWritesForStreamsAfter(last_good_stream_id);
+}
+
+void SpdySession::CloseAllStreams(net::Error status) {

[akalin, 2013/04/18 01:29:59]

please put this above to match order in header (or change the header order)

[Ryan Hamilton, 2013/04/18 17:18:29]

Done.

+  base::StatsCounter abandoned_streams("spdy.abandoned_streams");
+  base::StatsCounter abandoned_push_streams(
+      "spdy.abandoned_push_streams");
+
+  if (!unclaimed_pushed_streams_.empty()) {
+    streams_abandoned_count_ += unclaimed_pushed_streams_.size();
+    abandoned_push_streams.Add(unclaimed_pushed_streams_.size());
+    unclaimed_pushed_streams_.clear();
+  }
+
+  CloseAllStreamsAfter(0, status);
 }
 
 void SpdySession::LogAbandonedStream(const scoped_refptr<SpdyStream>& stream,
                                      net::Error status) {
   DCHECK(stream);
   std::string description = base::StringPrintf(
       "ABANDONED (stream_id=%d): ", stream->stream_id()) + stream->path();
   stream->LogStreamError(status, description);
 }
 
@@ skipping 553 matching lines @@
 
 void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
                            SpdyGoAwayStatus status) {
   net_log_.AddEvent(NetLog::TYPE_SPDY_SESSION_GOAWAY,
       base::Bind(&NetLogSpdyGoAwayCallback,
                  last_accepted_stream_id,
                  active_streams_.size(),
                  unclaimed_pushed_streams_.size(),
                  status));
   RemoveFromPool();
-  CloseAllStreams(net::ERR_ABORTED);
-
-  // TODO(willchan): Cancel any streams that are past the GoAway frame's
-  // |last_accepted_stream_id|.
-
-  // Don't bother killing any streams that are still reading.  They'll either
-  // complete successfully or get an ERR_CONNECTION_CLOSED when the socket is
-  // closed.
+  CloseAllStreamsAfter(last_accepted_stream_id, net::ERR_ABORTED);
 }
 
 void SpdySession::OnPing(uint32 unique_id) {
   net_log_.AddEvent(
       NetLog::TYPE_SPDY_SESSION_PING,
       base::Bind(&NetLogSpdyPingCallback, unique_id, "received"));
 
   // Send response to a PING from server.
   if (unique_id % 2 == 0) {
     WritePingFrame(unique_id);
@@ skipping 569 matching lines @@
   }
 
   session_recv_window_size_ -= delta_window_size;
   net_log_.AddEvent(
       NetLog::TYPE_SPDY_SESSION_UPDATE_RECV_WINDOW,
       base::Bind(&NetLogSpdySessionWindowUpdateCallback,
                  -delta_window_size, session_recv_window_size_));
 }
 
 }  // namespace net