Skip to content

Commit

Permalink
Correctly handle SPDY GOAWAY frames.
Browse files Browse the repository at this point in the history 
Review URL: https://chromiumcodereview.appspot.com/14232014

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@195032 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rch@chromium.org committed Apr 19, 2013
1 parent a71d324 commit 38dfd13
Show file tree
Hide file tree
Showing 11 changed files with 227 additions and 182 deletions.
46 changes: 23 additions & 23 deletions net/spdy/spdy_session.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1139,19 +1139,8 @@ void SpdySession::WriteSocket() {
}
}

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]);
Expand All @@ -1161,11 +1150,14 @@ void SpdySession::CloseAllStreams(net::Error status) {
}
}

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;
LogAbandonedStream(stream, status);
DeleteStream(stream->stream_id(), status);
streams_abandoned_count_++;
}

while (!created_streams_.empty()) {
Expand All @@ -1176,6 +1168,21 @@ void SpdySession::CloseAllStreams(net::Error status) {
stream->OnClose(status);
}

write_queue_.RemovePendingWritesForStreamsAfter(last_good_stream_id);
}

void SpdySession::CloseAllStreams(net::Error status) {
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);
write_queue_.Clear();
}

Expand Down Expand Up @@ -1753,14 +1760,7 @@ void SpdySession::OnGoAway(SpdyStreamId last_accepted_stream_id,
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) {
Expand Down
9 changes: 8 additions & 1 deletion net/spdy/spdy_session.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -604,7 +604,14 @@ class NET_EXPORT SpdySession : public base::RefCounted<SpdySession>,
void RecordHistograms();
void RecordProtocolErrorHistogram(SpdyProtocolErrorDetails details);

// Closes all streams. Used as part of shutdown.
// Closes all active streams with stream id's greater than
// |last_good_stream_id|, as well as any created or pending streams.
// Does not close unclaimed push streams.
void CloseAllStreamsAfter(SpdyStreamId last_good_stream_id,
net::Error status);

// Closes all streams, including unclaimed push streams. Used as part of
// shutdown.
void CloseAllStreams(net::Error status);

void LogAbandonedStream(const scoped_refptr<SpdyStream>& stream,
Expand Down
64 changes: 54 additions & 10 deletions net/spdy/spdy_session_spdy2_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,32 +110,76 @@ TEST_F(SpdySessionSpdy2Test, GoAway) {
session_deps_.host_resolver->set_synchronous_mode(true);

MockConnect connect_data(SYNCHRONOUS, OK);
scoped_ptr<SpdyFrame> goaway(ConstructSpdyGoAway());
scoped_ptr<SpdyFrame> goaway(ConstructSpdyGoAway(1));
MockRead reads[] = {
CreateMockRead(*goaway),
MockRead(SYNCHRONOUS, 0, 0) // EOF
CreateMockRead(*goaway, 2),
MockRead(ASYNC, 0, 3) // EOF
};
StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0);
scoped_ptr<SpdyFrame> req1(ConstructSpdyGet(NULL, 0, false, 1, MEDIUM));
scoped_ptr<SpdyFrame> req2(ConstructSpdyGet(NULL, 0, false, 3, MEDIUM));
MockWrite writes[] = {
CreateMockWrite(*req1, 0),
CreateMockWrite(*req2, 1),
};
DeterministicSocketData data(reads, arraysize(reads),
writes, arraysize(writes));
data.set_connect_data(connect_data);
session_deps_.socket_factory->AddSocketDataProvider(&data);
session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data);

SSLSocketDataProvider ssl(SYNCHRONOUS, OK);
ssl.SetNextProto(kProtoSPDY2);
session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl);
session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl);

CreateNetworkSession();
CreateDeterministicNetworkSession();

scoped_refptr<SpdySession> session = CreateInitializedSession();

EXPECT_EQ(2, session->GetProtocolVersion());

// Flush the SpdySession::OnReadComplete() task.
MessageLoop::current()->RunUntilIdle();
GURL url("http://www.google.com");
scoped_refptr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());

scoped_refptr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());

scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
(*headers)["scheme"] = url.scheme();
(*headers)["host"] = url.host();
(*headers)["url"] = url.path();
(*headers)["version"] = "HTTP/1.1";
scoped_ptr<SpdyHeaderBlock> headers2(new SpdyHeaderBlock);
*headers2 = *headers;

spdy_stream1->set_spdy_headers(headers.Pass());
EXPECT_TRUE(spdy_stream1->HasUrl());

spdy_stream2->set_spdy_headers(headers2.Pass());
EXPECT_TRUE(spdy_stream2->HasUrl());

spdy_stream1->SendRequest(false);
spdy_stream2->SendRequest(false);
data.RunFor(2);

EXPECT_EQ(1u, spdy_stream1->stream_id());
EXPECT_EQ(3u, spdy_stream2->stream_id());

EXPECT_TRUE(spdy_session_pool_->HasSession(pair_));

// Read and process the GOAWAY frame.
data.RunFor(1);

EXPECT_FALSE(spdy_session_pool_->HasSession(pair_));

EXPECT_TRUE(session->IsStreamActive(1));
EXPECT_FALSE(session->IsStreamActive(3));

scoped_refptr<SpdySession> session2 = GetSession(pair_);

spdy_stream1->Close();
spdy_stream1 = NULL;
spdy_stream2 = NULL;

// Delete the first session.
session = NULL;

Expand Down
65 changes: 55 additions & 10 deletions net/spdy/spdy_session_spdy3_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,32 +127,76 @@ TEST_F(SpdySessionSpdy3Test, GoAway) {
session_deps_.host_resolver->set_synchronous_mode(true);

MockConnect connect_data(SYNCHRONOUS, OK);
scoped_ptr<SpdyFrame> goaway(ConstructSpdyGoAway());
scoped_ptr<SpdyFrame> goaway(ConstructSpdyGoAway(1));
MockRead reads[] = {
CreateMockRead(*goaway),
MockRead(SYNCHRONOUS, 0, 0) // EOF
CreateMockRead(*goaway, 2),
MockRead(ASYNC, 0, 3) // EOF
};
StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0);
scoped_ptr<SpdyFrame> req1(ConstructSpdyGet(NULL, 0, false, 1, MEDIUM));
scoped_ptr<SpdyFrame> req2(ConstructSpdyGet(NULL, 0, false, 3, MEDIUM));
MockWrite writes[] = {
CreateMockWrite(*req1, 0),
CreateMockWrite(*req2, 1),
};
DeterministicSocketData data(reads, arraysize(reads),
writes, arraysize(writes));
data.set_connect_data(connect_data);
session_deps_.socket_factory->AddSocketDataProvider(&data);
session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data);

SSLSocketDataProvider ssl(SYNCHRONOUS, OK);
ssl.SetNextProto(kProtoSPDY3);
session_deps_.socket_factory->AddSSLSocketDataProvider(&ssl);
session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl);

CreateNetworkSession();
CreateDeterministicNetworkSession();

scoped_refptr<SpdySession> session = CreateInitializedSession();

EXPECT_EQ(3, session->GetProtocolVersion());

// Flush the SpdySession::OnReadComplete() task.
MessageLoop::current()->RunUntilIdle();
GURL url("http://www.google.com");
scoped_refptr<SpdyStream> spdy_stream1 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());

scoped_refptr<SpdyStream> spdy_stream2 =
CreateStreamSynchronously(session, url, MEDIUM, BoundNetLog());

scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)[":method"] = "GET";
(*headers)[":scheme"] = url.scheme();
(*headers)[":host"] = url.host();
(*headers)[":path"] = url.path();
(*headers)[":version"] = "HTTP/1.1";
scoped_ptr<SpdyHeaderBlock> headers2(new SpdyHeaderBlock);
*headers2 = *headers;

spdy_stream1->set_spdy_headers(headers.Pass());
EXPECT_TRUE(spdy_stream1->HasUrl());

spdy_stream2->set_spdy_headers(headers2.Pass());
EXPECT_TRUE(spdy_stream2->HasUrl());

spdy_stream1->SendRequest(false);
spdy_stream2->SendRequest(false);
data.RunFor(2);

EXPECT_EQ(1u, spdy_stream1->stream_id());
EXPECT_EQ(3u, spdy_stream2->stream_id());

EXPECT_TRUE(spdy_session_pool_->HasSession(pair_));

// Read and process the GOAWAY frame.
data.RunFor(1);

EXPECT_FALSE(spdy_session_pool_->HasSession(pair_));

EXPECT_TRUE(session->IsStreamActive(1));
EXPECT_FALSE(session->IsStreamActive(3));

scoped_refptr<SpdySession> session2 = GetSession(pair_);

spdy_stream1->Close();
spdy_stream1 = NULL;
spdy_stream2 = NULL;

// Delete the first session.
session = NULL;

Expand Down Expand Up @@ -1150,6 +1194,7 @@ TEST_F(SpdySessionSpdy3Test, CloseSessionWithTwoCreatedStreams) {

spdy_stream1 = NULL;
spdy_stream2 = NULL;
session = NULL;
}

TEST_F(SpdySessionSpdy3Test, VerifyDomainAuthentication) {
Expand Down
Loading

0 comments on commit 38dfd13

Please sign in to comment.