Skip to content

GitLab

T
tangled
Commit 0fc7c604 authored by rch@chromium.org's avatar rch@chromium.org
Browse files
Options

Fix bug in SpdySession where map.erase() was called on an iterator when the... 

Fix bug in SpdySession where map.erase() was called on an iterator when the underlying map had changed, making the iterator invalid.

BUG=139518
TEST=SpdySessionSpdy*Test.CloseSessionWithTwoCreatedStreams


Review URL: https://chromiumcodereview.appspot.com/10821086

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@149071 0039d316-1c4b-4281-b951-d872f2087c98
parent cf90af4e
Hide whitespace changes
Inline Side-by-side
Showing with 277 additions and 15 deletions
net/spdy/spdy_session.cc
View file @ 0fc7c604
...@@ -1016,10 +1016,10 @@ void SpdySession::CloseAllStreams(net::Error status) { ...@@ -1016,10 +1016,10 @@ void SpdySession::CloseAllStreams(net::Error status) {
while (!created_streams_.empty()) { while (!created_streams_.empty()) {
CreatedStreamSet::iterator it = created_streams_.begin(); CreatedStreamSet::iterator it = created_streams_.begin();
const scoped_refptr<SpdyStream>& stream = *it; const scoped_refptr<SpdyStream> stream = *it;
created_streams_.erase(it);
LogAbandonedStream(stream, status); LogAbandonedStream(stream, status);
stream->OnClose(status); stream->OnClose(status);
created_streams_.erase(it);
} }
// We also need to drain the queue. // We also need to drain the queue.
......
net/spdy/spdy_session_spdy2_unittest.cc
View file @ 0fc7c604
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
using namespace net::test_spdy2; using namespace net::test_spdy2;
namespace net {
namespace { namespace {
base::TimeTicks the_near_future() { base::TimeTicks the_near_future() {
...@@ -22,9 +24,35 @@ base::TimeTicks the_near_future() { ...@@ -22,9 +24,35 @@ base::TimeTicks the_near_future() {
base::TimeDelta::FromSeconds(301); base::TimeDelta::FromSeconds(301);
} }
} // namespace class ClosingDelegate : public SpdyStream::Delegate {
public:
ClosingDelegate(SpdyStream* stream) : stream_(stream) {}
namespace net { // SpdyStream::Delegate implementation:
virtual bool OnSendHeadersComplete(int status) OVERRIDE {
return true;
}
virtual int OnSendBody() OVERRIDE {
return OK;
}
virtual int OnSendBodyComplete(int status, bool* eof) OVERRIDE {
return OK;
}
virtual int OnResponseReceived(const SpdyHeaderBlock& response,
base::Time response_time,
int status) OVERRIDE {
return OK;
}
virtual void OnDataReceived(const char* data, int length) OVERRIDE {}
virtual void OnDataSent(int length) OVERRIDE {}
virtual void OnClose(int status) OVERRIDE {
stream_->Close();
}
private:
SpdyStream* stream_;
};
} // namespace
// TODO(cbentzel): Expose compression setter/getter in public SpdySession // TODO(cbentzel): Expose compression setter/getter in public SpdySession
// interface rather than going through all these contortions. // interface rather than going through all these contortions.
...@@ -1386,4 +1414,107 @@ TEST_F(SpdySessionSpdy2Test, CancelStream) { ...@@ -1386,4 +1414,107 @@ TEST_F(SpdySessionSpdy2Test, CancelStream) {
spdy_stream2 = NULL; spdy_stream2 = NULL;
} }
TEST_F(SpdySessionSpdy2Test, CloseSessionWithTwoCreatedStreams) {
// Test that if a sesion is closed with two created streams pending,
// it does not crash. http://crbug.com/139518
MockConnect connect_data(SYNCHRONOUS, OK);
SpdySessionDependencies session_deps;
session_deps.host_resolver->set_synchronous_mode(true);
// No actual data will be sent.
MockWrite writes[] = {
MockWrite(ASYNC, 0, 1) // EOF
};
MockRead reads[] = {
MockRead(ASYNC, 0, 0) // EOF
};
DeterministicSocketData data(reads, arraysize(reads),
writes, arraysize(writes));
data.set_connect_data(connect_data);
session_deps.deterministic_socket_factory->AddSocketDataProvider(&data);
SSLSocketDataProvider ssl(SYNCHRONOUS, OK);
session_deps.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl);
scoped_refptr<HttpNetworkSession> http_session(
SpdySessionDependencies::SpdyCreateSessionDeterministic(&session_deps));
const std::string kTestHost("www.foo.com");
const int kTestPort = 80;
HostPortPair test_host_port_pair(kTestHost, kTestPort);
HostPortProxyPair pair(test_host_port_pair, ProxyServer::Direct());
SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool());
// Create a session.
EXPECT_FALSE(spdy_session_pool->HasSession(pair));
scoped_refptr<SpdySession> session =
spdy_session_pool->Get(pair, BoundNetLog());
ASSERT_TRUE(spdy_session_pool->HasSession(pair));
scoped_refptr<TransportSocketParams> transport_params(
new TransportSocketParams(test_host_port_pair,
MEDIUM,
false,
false,
OnHostResolutionCallback()));
scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
EXPECT_EQ(OK, connection->Init(test_host_port_pair.ToString(),
transport_params, MEDIUM, CompletionCallback(),
http_session->GetTransportSocketPool(
HttpNetworkSession::NORMAL_SOCKET_POOL),
BoundNetLog()));
EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
scoped_refptr<SpdyStream> spdy_stream1;
TestCompletionCallback callback1;
GURL url1("http://www.google.com");
EXPECT_EQ(OK, session->CreateStream(url1, HIGHEST, &spdy_stream1,
BoundNetLog(), callback1.callback()));
EXPECT_EQ(0u, spdy_stream1->stream_id());
scoped_refptr<SpdyStream> spdy_stream2;
TestCompletionCallback callback2;
GURL url2("http://www.google.com");
EXPECT_EQ(OK, session->CreateStream(url2, LOWEST, &spdy_stream2,
BoundNetLog(), callback2.callback()));
EXPECT_EQ(0u, spdy_stream2->stream_id());
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
(*headers)["scheme"] = url1.scheme();
(*headers)["host"] = url1.host();
(*headers)["url"] = url1.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());
ClosingDelegate delegate1(spdy_stream1.get());
spdy_stream1->SetDelegate(&delegate1);
spdy_stream2->set_spdy_headers(headers2.Pass());
EXPECT_TRUE(spdy_stream2->HasUrl());
ClosingDelegate delegate2(spdy_stream2.get());
spdy_stream2->SetDelegate(&delegate2);
spdy_stream1->SendRequest(false);
spdy_stream2->SendRequest(false);
// Ensure that the streams have not yet been activated and assigned an id.
EXPECT_EQ(0u, spdy_stream1->stream_id());
EXPECT_EQ(0u, spdy_stream2->stream_id());
// Ensure we don't crash while closing the session.
session->CloseSessionOnError(ERR_ABORTED, true, "");
EXPECT_TRUE(spdy_stream1->closed());
EXPECT_TRUE(spdy_stream2->closed());
spdy_stream1 = NULL;
spdy_stream2 = NULL;
}
} // namespace net } // namespace net
net/spdy/spdy_session_spdy3_unittest.cc
View file @ 0fc7c604
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
using namespace net::test_spdy3; using namespace net::test_spdy3;
namespace net {
namespace { namespace {
base::TimeTicks the_near_future() { base::TimeTicks the_near_future() {
...@@ -22,22 +24,35 @@ base::TimeTicks the_near_future() { ...@@ -22,22 +24,35 @@ base::TimeTicks the_near_future() {
base::TimeDelta::FromSeconds(301); base::TimeDelta::FromSeconds(301);
} }
} // namespace class ClosingDelegate : public SpdyStream::Delegate {
public:
namespace net { ClosingDelegate(SpdyStream* stream) : stream_(stream) {}
// TODO(cbentzel): Expose compression setter/getter in public SpdySession // SpdyStream::Delegate implementation:
// interface rather than going through all these contortions. virtual bool OnSendHeadersComplete(int status) OVERRIDE {
class SpdySessionSpdy3Test : public PlatformTest { return true;
protected: }
virtual void SetUp() { virtual int OnSendBody() OVERRIDE {
SpdySession::set_default_protocol(kProtoSPDY3); return OK;
}
virtual int OnSendBodyComplete(int status, bool* eof) OVERRIDE {
return OK;
}
virtual int OnResponseReceived(const SpdyHeaderBlock& response,
base::Time response_time,
int status) OVERRIDE {
return OK;
}
virtual void OnDataReceived(const char* data, int length) OVERRIDE {}
virtual void OnDataSent(int length) OVERRIDE {}
virtual void OnClose(int status) OVERRIDE {
stream_->Close();
} }
private: private:
SpdyTestStateHelper spdy_state_; SpdyStream* stream_;
}; };
class TestSpdyStreamDelegate : public net::SpdyStream::Delegate { class TestSpdyStreamDelegate : public net::SpdyStream::Delegate {
public: public:
explicit TestSpdyStreamDelegate(const CompletionCallback& callback) explicit TestSpdyStreamDelegate(const CompletionCallback& callback)
...@@ -76,6 +91,19 @@ class TestSpdyStreamDelegate : public net::SpdyStream::Delegate { ...@@ -76,6 +91,19 @@ class TestSpdyStreamDelegate : public net::SpdyStream::Delegate {
CompletionCallback callback_; CompletionCallback callback_;
}; };
} // namespace
// TODO(cbentzel): Expose compression setter/getter in public SpdySession
// interface rather than going through all these contortions.
class SpdySessionSpdy3Test : public PlatformTest {
protected:
virtual void SetUp() {
SpdySession::set_default_protocol(kProtoSPDY3);
}
private:
SpdyTestStateHelper spdy_state_;
};
// Test the SpdyIOBuffer class. // Test the SpdyIOBuffer class.
TEST_F(SpdySessionSpdy3Test, SpdyIOBuffer) { TEST_F(SpdySessionSpdy3Test, SpdyIOBuffer) {
std::priority_queue<SpdyIOBuffer> queue_; std::priority_queue<SpdyIOBuffer> queue_;
...@@ -1556,4 +1584,107 @@ TEST_F(SpdySessionSpdy3Test, CancelStream) { ...@@ -1556,4 +1584,107 @@ TEST_F(SpdySessionSpdy3Test, CancelStream) {
spdy_stream2 = NULL; spdy_stream2 = NULL;
} }
TEST_F(SpdySessionSpdy3Test, CloseSessionWithTwoCreatedStreams) {
// Test that if a sesion is closed with two created streams pending,
// it does not crash. http://crbug.com/139518
MockConnect connect_data(SYNCHRONOUS, OK);
SpdySessionDependencies session_deps;
session_deps.host_resolver->set_synchronous_mode(true);
// No actual data will be sent.
MockWrite writes[] = {
MockWrite(ASYNC, 0, 1) // EOF
};
MockRead reads[] = {
MockRead(ASYNC, 0, 0) // EOF
};
DeterministicSocketData data(reads, arraysize(reads),
writes, arraysize(writes));
data.set_connect_data(connect_data);
session_deps.deterministic_socket_factory->AddSocketDataProvider(&data);
SSLSocketDataProvider ssl(SYNCHRONOUS, OK);
session_deps.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl);
scoped_refptr<HttpNetworkSession> http_session(
SpdySessionDependencies::SpdyCreateSessionDeterministic(&session_deps));
const std::string kTestHost("www.foo.com");
const int kTestPort = 80;
HostPortPair test_host_port_pair(kTestHost, kTestPort);
HostPortProxyPair pair(test_host_port_pair, ProxyServer::Direct());
SpdySessionPool* spdy_session_pool(http_session->spdy_session_pool());
// Create a session.
EXPECT_FALSE(spdy_session_pool->HasSession(pair));
scoped_refptr<SpdySession> session =
spdy_session_pool->Get(pair, BoundNetLog());
ASSERT_TRUE(spdy_session_pool->HasSession(pair));
scoped_refptr<TransportSocketParams> transport_params(
new TransportSocketParams(test_host_port_pair,
MEDIUM,
false,
false,
OnHostResolutionCallback()));
scoped_ptr<ClientSocketHandle> connection(new ClientSocketHandle);
EXPECT_EQ(OK, connection->Init(test_host_port_pair.ToString(),
transport_params, MEDIUM, CompletionCallback(),
http_session->GetTransportSocketPool(
HttpNetworkSession::NORMAL_SOCKET_POOL),
BoundNetLog()));
EXPECT_EQ(OK, session->InitializeWithSocket(connection.release(), false, OK));
scoped_refptr<SpdyStream> spdy_stream1;
TestCompletionCallback callback1;
GURL url1("http://www.google.com");
EXPECT_EQ(OK, session->CreateStream(url1, HIGHEST, &spdy_stream1,
BoundNetLog(), callback1.callback()));
EXPECT_EQ(0u, spdy_stream1->stream_id());
scoped_refptr<SpdyStream> spdy_stream2;
TestCompletionCallback callback2;
GURL url2("http://www.google.com");
EXPECT_EQ(OK, session->CreateStream(url2, LOWEST, &spdy_stream2,
BoundNetLog(), callback2.callback()));
EXPECT_EQ(0u, spdy_stream2->stream_id());
scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
(*headers)["method"] = "GET";
(*headers)["scheme"] = url1.scheme();
(*headers)["host"] = url1.host();
(*headers)["url"] = url1.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());
ClosingDelegate delegate1(spdy_stream1.get());
spdy_stream1->SetDelegate(&delegate1);
spdy_stream2->set_spdy_headers(headers2.Pass());
EXPECT_TRUE(spdy_stream2->HasUrl());
ClosingDelegate delegate2(spdy_stream2.get());
spdy_stream2->SetDelegate(&delegate2);
spdy_stream1->SendRequest(false);
spdy_stream2->SendRequest(false);
// Ensure that the streams have not yet been activated and assigned an id.
EXPECT_EQ(0u, spdy_stream1->stream_id());
EXPECT_EQ(0u, spdy_stream2->stream_id());
// Ensure we don't crash while closing the session.
session->CloseSessionOnError(ERR_ABORTED, true, "");
EXPECT_TRUE(spdy_stream1->closed());
EXPECT_TRUE(spdy_stream2->closed());
spdy_stream1 = NULL;
spdy_stream2 = NULL;
}
} // namespace net } // namespace net
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment