diff options
author | mbelshe@chromium.org <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-04 05:41:45 +0000 |
---|---|---|
committer | mbelshe@chromium.org <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-04 05:41:45 +0000 |
commit | 1ed7b3dc2b38c5d6930e103bea1fcfdffab7db7a (patch) | |
tree | baa67ac64a2be7bd3e0bdae932ada206ee9d5991 /net/spdy | |
parent | b3187ca4648ae595377786332442e3547eea24f6 (diff) | |
download | chromium_src-1ed7b3dc2b38c5d6930e103bea1fcfdffab7db7a.zip chromium_src-1ed7b3dc2b38c5d6930e103bea1fcfdffab7db7a.tar.gz chromium_src-1ed7b3dc2b38c5d6930e103bea1fcfdffab7db7a.tar.bz2 |
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
Diffstat (limited to 'net/spdy')
-rw-r--r-- | net/spdy/spdy_network_transaction_unittest.cc | 85 | ||||
-rw-r--r-- | net/spdy/spdy_stream.cc | 5 |
2 files changed, 87 insertions, 3 deletions
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 |