From 1ed7b3dc2b38c5d6930e103bea1fcfdffab7db7a Mon Sep 17 00:00:00 2001
From: "mbelshe@chromium.org"
 <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Thu, 4 Mar 2010 05:41:45 +0000
Subject: Fix a crash when streams are cancelled and we have a BufferedRead
 task outstanding.

BUG=none
TEST=SpdyNetworkTranscation.BufferedCancelled

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@40602 0039d316-1c4b-4281-b951-d872f2087c98
---
 net/spdy/spdy_network_transaction_unittest.cc | 85 ++++++++++++++++++++++++++-
 net/spdy/spdy_stream.cc                       |  5 +-
 2 files changed, 87 insertions(+), 3 deletions(-)

(limited to 'net/spdy')

diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 7dfa76d..2d612f6 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -1420,7 +1420,7 @@ TEST_F(SpdyNetworkTransactionTest, BufferedAll) {
 }
 
 // Verify the case where we buffer data and close the connection.
-TEST_F(SpdyNetworkTransactionTest, BufferedCancelled) {
+TEST_F(SpdyNetworkTransactionTest, BufferedClosed) {
   MockWrite writes[] = {
     MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
               arraysize(kGetSyn)),
@@ -1525,4 +1525,87 @@ TEST_F(SpdyNetworkTransactionTest, BufferedCancelled) {
   EXPECT_TRUE(data->at_write_eof());
 }
 
+// Verify the case where we buffer data and cancel the transaction.
+TEST_F(SpdyNetworkTransactionTest, BufferedCancelled) {
+  MockWrite writes[] = {
+    MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
+              arraysize(kGetSyn)),
+  };
+
+  static const unsigned char kDataFrame[] = {
+    0x00, 0x00, 0x00, 0x01,                                      // header
+    0x00, 0x00, 0x00, 0x07,                                      // length
+    'm', 'e', 's', 's', 'a', 'g', 'e',
+    // NOTE: We didn't FIN the stream.
+  };
+
+  MockRead reads[] = {
+    MockRead(true, reinterpret_cast<const char*>(kGetSynReply),
+             arraysize(kGetSynReply)),
+    MockRead(true, ERR_IO_PENDING),  // Force a wait
+    MockRead(true, reinterpret_cast<const char*>(kDataFrame),
+             arraysize(kDataFrame)),
+    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)));
+
+  // For this test, we can't use the TransactionHelper, because we are
+  // going to tightly control how the IOs fly.
+  TransactionHelperResult out;
+
+  // We disable SSL for this test.
+  SpdySession::SetSSLMode(false);
+
+  SessionDependencies session_deps;
+  scoped_ptr<SpdyNetworkTransaction> trans(
+      new SpdyNetworkTransaction(CreateSession(&session_deps)));
+
+  session_deps.socket_factory.AddSocketDataProvider(data);
+
+  TestCompletionCallback callback;
+
+  int rv = trans->Start(&request, &callback, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+
+  out.rv = callback.WaitForResult();
+  EXPECT_EQ(out.rv, OK);
+
+  const HttpResponseInfo* response = trans->GetResponseInfo();
+  EXPECT_TRUE(response->headers != NULL);
+  EXPECT_TRUE(response->was_fetched_via_spdy);
+  out.status_line = response->headers->GetStatusLine();
+  out.response_info = *response;  // Make a copy so we can verify.
+
+  // Read Data
+  TestCompletionCallback read_callback;
+
+  do {
+    const int kReadSize = 256;
+    scoped_refptr<net::IOBuffer> buf = new net::IOBuffer(kReadSize);
+    rv = trans->Read(buf, kReadSize, &read_callback);
+    if (rv == net::ERR_IO_PENDING) {
+      // Complete the read now, which causes buffering to start.
+      data->CompleteRead();
+      // Destroy the transaction, causing the stream to get cancelled
+      // and orphaning the buffered IO task.
+      trans.reset();
+      break;
+    }
+    // We shouldn't get here in this test.
+    FAIL() << "Unexpected read: " << rv;
+  } while (rv > 0);
+
+  // Flush the MessageLoop; this will cause the buffered IO task
+  // to run for the final time.
+  MessageLoop::current()->RunAllPending();
+}
+
+
 }  // namespace net
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 04865d9..e279770 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -371,8 +371,9 @@ bool SpdyStream::ShouldWaitForMoreBufferedData() const {
 void SpdyStream::DoBufferedReadCallback() {
   buffered_read_callback_pending_ = false;
 
-  // If the response_status_ is not ok, we don't attempt to complete the read.
-  if (response_status_ != OK)
+  // If the transaction is cancelled or errored out, we don't need to complete
+  // the read.
+  if (response_status_ != OK || cancelled_)
     return;
 
   // When more_read_data_pending_ is true, it means that more data has
-- 
cgit v1.1