summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
authormbelshe@chromium.org <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-12-21 21:01:12 +0000
committermbelshe@chromium.org <mbelshe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-12-21 21:01:12 +0000
commit92683b5a068ab7c7f58251dbee9b047314977bae (patch)
tree5f177d57e8b6f39473f1325610fd90ed9119b1a1 /net
parentf57838fd46869064f0125bb33b1c4f7cae07019a (diff)
downloadchromium_src-92683b5a068ab7c7f58251dbee9b047314977bae.zip
chromium_src-92683b5a068ab7c7f58251dbee9b047314977bae.tar.gz
chromium_src-92683b5a068ab7c7f58251dbee9b047314977bae.tar.bz2
Fix crash with CancelStream. The stream didn't inform
the FlipSession, and the FlipSession continued to try to notify the stream of new data. Also re-enable the CancelTransaction test, which did catch the crash. BUG=none TEST=FlipNetworkTransactionTest.CancelTransaction Review URL: http://codereview.chromium.org/506062 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35106 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r--net/flip/flip_network_transaction_unittest.cc15
-rw-r--r--net/flip/flip_session.cc14
-rwxr-xr-xnet/flip/flip_stream.cc13
3 files changed, 26 insertions, 16 deletions
diff --git a/net/flip/flip_network_transaction_unittest.cc b/net/flip/flip_network_transaction_unittest.cc
index 29d266c..77ed06c 100644
--- a/net/flip/flip_network_transaction_unittest.cc
+++ b/net/flip/flip_network_transaction_unittest.cc
@@ -39,7 +39,15 @@ class SessionDependencies {
: host_resolver(new MockHostResolver),
proxy_service(CreateNullProxyService()),
ssl_config_service(new SSLConfigServiceDefaults),
- flip_session_pool(new FlipSessionPool) {}
+ flip_session_pool(new FlipSessionPool) {
+ // Note: The CancelledTransaction test does cleanup by running all tasks
+ // in the message loop (RunAllPending). Unfortunately, that doesn't clean
+ // up tasks on the host resolver thread; and TCPConnectJob is currently
+ // not cancellable. Using synchronous lookups allows the test to shutdown
+ // cleanly. Until we have cancellable TCPConnectJobs, use synchronous
+ // lookups.
+ host_resolver->set_synchronous_mode(true);
+ }
// Custom proxy service dependency.
explicit SessionDependencies(ProxyService* proxy_service)
@@ -441,10 +449,7 @@ TEST_F(FlipNetworkTransactionTest, ResponseWithoutSynReply) {
EXPECT_EQ(ERR_SYN_REPLY_NOT_RECEIVED, out.rv);
}
-// TODO(willchan): Look into why TCPConnectJobs are still alive when this test
-// goes away. They're calling into the ClientSocketFactory which doesn't exist
-// anymore, so it crashes.
-TEST_F(FlipNetworkTransactionTest, DISABLED_CancelledTransaction) {
+TEST_F(FlipNetworkTransactionTest, CancelledTransaction) {
MockWrite writes[] = {
MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
arraysize(kGetSyn)),
diff --git a/net/flip/flip_session.cc b/net/flip/flip_session.cc
index a38e4f6..b6abbe9 100644
--- a/net/flip/flip_session.cc
+++ b/net/flip/flip_session.cc
@@ -395,6 +395,9 @@ bool FlipSession::CancelStream(flip::FlipStreamId stream_id) {
if (!IsStreamActive(stream_id))
return false;
+ // TODO(mbelshe): We should send a FIN_STREAM control frame here
+ // so that the server can cancel a large send.
+
// TODO(mbelshe): Write a method for tearing down a stream
// that cleans it out of the active list, the pending list,
// etc.
@@ -531,7 +534,11 @@ void FlipSession::OnWriteComplete(int result) {
DCHECK_GT(result, static_cast<int>(flip::FlipFrame::size()));
result -= static_cast<int>(flip::FlipFrame::size());
}
- stream->OnWriteComplete(result);
+
+ // It is possible that the stream was cancelled while we were writing
+ // to the socket.
+ if (!stream->cancelled())
+ stream->OnWriteComplete(result);
// Cleanup the write which just completed.
in_flight_write_.release();
@@ -770,6 +777,7 @@ void FlipSession::OnStreamFrameData(flip::FlipStreamId stream_id,
LOG(INFO) << "Flip data for stream " << stream_id << ", " << len << " bytes";
bool valid_stream = IsStreamActive(stream_id);
if (!valid_stream) {
+ // NOTE: it may just be that the stream was cancelled.
LOG(WARNING) << "Received data frame for invalid stream " << stream_id;
return;
}
@@ -866,6 +874,7 @@ void FlipSession::OnSynReply(const flip::FlipSynReplyControlFrame* frame,
flip::FlipStreamId stream_id = frame->stream_id();
bool valid_stream = IsStreamActive(stream_id);
if (!valid_stream) {
+ // NOTE: it may just be that the stream was cancelled.
LOG(WARNING) << "Received SYN_REPLY for invalid stream " << stream_id;
return;
}
@@ -903,6 +912,7 @@ void FlipSession::OnSynReply(const flip::FlipSynReplyControlFrame* frame,
scoped_refptr<FlipStream> stream = active_streams_[stream_id];
CHECK(stream->stream_id() == stream_id);
+ CHECK(!stream->cancelled());
HttpResponseInfo response;
if (FlipHeadersToHttpResponse(*headers, &response)) {
GetSSLInfo(&response.ssl_info);
@@ -949,11 +959,13 @@ void FlipSession::OnFin(const flip::FlipFinStreamControlFrame* frame) {
flip::FlipStreamId stream_id = frame->stream_id();
bool valid_stream = IsStreamActive(stream_id);
if (!valid_stream) {
+ // NOTE: it may just be that the stream was cancelled.
LOG(WARNING) << "Received FIN for invalid stream" << stream_id;
return;
}
scoped_refptr<FlipStream> stream = active_streams_[stream_id];
CHECK(stream->stream_id() == stream_id);
+ CHECK(!stream->cancelled());
if (frame->status() == 0) {
stream->OnDataReceived(NULL, 0);
} else {
diff --git a/net/flip/flip_stream.cc b/net/flip/flip_stream.cc
index 9f76b95..f283241 100755
--- a/net/flip/flip_stream.cc
+++ b/net/flip/flip_stream.cc
@@ -146,16 +146,7 @@ void FlipStream::Cancel() {
cancelled_ = true;
user_callback_ = NULL;
- // TODO(willchan): Should we also call FlipSession::CancelStream()? What
- // makes more sense? If we cancel the stream, perhaps we should send the
- // server a control packet to tell it to stop sending to the dead stream. But
- // then does FlipSession deactivate the stream? It would log warnings for
- // data packets for an invalid stream then, unless we maintained some data
- // structure to keep track of cancelled stream ids. Ugh. Currently
- // FlipStream does not call CancelStream(). We should free up all the memory
- // associated with the stream though (ditch all the IOBuffers) and at all
- // FlipSession's entrypoints into FlipStream check |cancelled_| and not copy
- // the data.
+ session_->CancelStream(stream_id_);
}
void FlipStream::OnResponseReceived(const HttpResponseInfo& response) {
@@ -231,6 +222,8 @@ bool FlipStream::OnDataReceived(const char* data, int length) {
}
void FlipStream::OnWriteComplete(int status) {
+ // TODO(mbelshe): Check for cancellation here. If we're cancelled, we
+ // should discontinue the DoLoop.
DoLoop(status);
}