Merge 45210 - Fix crasher on SpdySession teardown. When we close the session on error,

we can get into a case where we delete the session itself while cleaning
out the active streams.  This ends up with a reentrant call into
CloseAllStreams.

Fix is to selfhold a reference during teardown.
The test case crashes without the fix, and works fine with the fix.


BUG=42215
TEST=SpdyNetworkTransactionTest.GoAwayWithActiveStream


Review URL: http://codereview.chromium.org/1711008

TBR=mbelshe@chromium.org
Review URL: http://codereview.chromium.org/1723004

git-svn-id: svn://svn.chromium.org/chrome/branches/375/src@45216 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 33 insertions, 0 deletions

diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index cc08a9e..9026907 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -288,6 +288,12 @@ static const unsigned char kPostBodyFrame[] = {
   'h', 'e', 'l', 'l', 'o', '!',                                  // "hello"
 };
 
+static const unsigned char kGoAway[] = {
+  0x80, 0x01, 0x00, 0x07,  // header
+  0x00, 0x00, 0x00, 0x04,  // flags, len
+  0x00, 0x00, 0x00, 0x00,  // last-accepted-stream-id
+};
+
 // Adds headers and values to a map.
 // |extra_headers| is an array of { name, value } pairs, arranged as strings
 // where the even entries are the header names, and the odd entries are the
@@ -2715,4 +2721,27 @@ TEST_F(SpdyNetworkTransactionTest, SettingsPlayback) {
   }
 }
 
+TEST_F(SpdyNetworkTransactionTest, GoAwayWithActiveStream) {
+  MockWrite writes[] = {
+    MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
+              arraysize(kGetSyn)),
+  };
+
+  MockRead reads[] = {
+    MockRead(true, reinterpret_cast<const char*>(kGoAway),
+             arraysize(kGoAway)),
+    MockRead(true, 0, 0)  // EOF
+  };
+
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("http://www.google.com/");
+  request.load_flags = 0;
+  scoped_refptr<DelayedSocketData> data(
+      new DelayedSocketData(1, reads, arraysize(reads),
+                            writes, arraysize(writes)));
+  TransactionHelperResult out = TransactionHelper(request, data.get(), NULL);
+  EXPECT_EQ(ERR_CONNECTION_CLOSED, out.rv);
+}
+
 }  // namespace net
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 8d6d9d2..b893ce9 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -789,6 +789,10 @@ void SpdySession::QueueFrame(spdy::SpdyFrame* frame,
 }
 
 void SpdySession::CloseSessionOnError(net::Error err) {
+  // Closing all streams can have a side-effect of dropping the last reference
+  // to |this|.  Hold a reference through this function.
+  scoped_refptr<SpdySession> self(this);
+
   DCHECK_LT(err, OK);
   LOG(INFO) << "spdy::CloseSessionOnError(" << err << ") for " <<
       host_port_pair().ToString();