Skip to content

Commit

Permalink
Reverting again ... More crashes, and the instrumentation did not app…
Browse files Browse the repository at this point in the history 
…ear to help

Revert 130129 - Revert 129034 - Revert 127893 - Revert 127730 - Revert 127717 - Revert 118788 - Revert 113405 - Revert 113305 - Revert 113300 - Revert 112134 - Revert 112130 - Close idle connections / SPDY sessions when needed

Due to the idle connection state being held by different socket pools, it's possible for one socket pool to hold an idle socket in a lower layer socket pool. From the lower level socket pool's perspective, the socket is being "actively" used. From the higher socket pool's (including SpdySession, which is more of a connection manager) perspective, the connection is idle and can be closed if we have hit a limit.

Normally this isn't a big deal, except when we have a lot of idle SPDY connections and are connecting via a proxy, so we have low connection limits through the proxy server. We address this problem by allowing lower-level socket pools to tell higher level socket pools to close a socket.

Fixed ASAN test failures by removing .Times(1) and .Times(2) from CloseMultipleIdleSocketsHeldByLayeredPoolWhenNeeded unittest (this removes the tests relying on the order of std::set in CloseOneIdleConnectionInLayeredPool). ASAN is prob
ably causing the memory allocator to allocate the pools differently. The std::set is ordered by LayeredPool* which is the address of the LayeredPool (willchan).

Added NET_EXPORT for layered_pool class defintion to fix windows shared compile.

BUG=62364, 92244, 109876, 110368, 119847
TEST=


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

TBR=rch@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10006036

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@131145 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rch@chromium.org committed Apr 6, 2012
1 parent 7429426 commit 2c75400
Show file tree
Hide file tree
Showing 22 changed files with 38 additions and 1,425 deletions.
181 changes: 0 additions & 181 deletions net/http/http_network_transaction_spdy21_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9552,185 +9552,4 @@ TEST_F(HttpNetworkTransactionSpdy21Test, SendPipelineEvictionFallback) {
EXPECT_EQ("hello world", out.response_data);
}

TEST_F(HttpNetworkTransactionSpdy21Test, CloseIdleSpdySessionToOpenNewOne) {
HttpStreamFactory::SetNextProtos(SpdyNextProtos());
int old_max_sockets_per_group =
ClientSocketPoolManager::max_sockets_per_group(
HttpNetworkSession::NORMAL_SOCKET_POOL);
int old_max_sockets_per_proxy_server =
ClientSocketPoolManager::max_sockets_per_proxy_server(
HttpNetworkSession::NORMAL_SOCKET_POOL);
int old_max_sockets_per_pool =
ClientSocketPoolManager::max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL);
ClientSocketPoolManager::set_max_sockets_per_group(
HttpNetworkSession::NORMAL_SOCKET_POOL, 1);
ClientSocketPoolManager::set_max_sockets_per_proxy_server(
HttpNetworkSession::NORMAL_SOCKET_POOL, 1);
ClientSocketPoolManager::set_max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL, 1);

// Use two different hosts with different IPs so they don't get pooled.
SessionDependencies session_deps;
session_deps.host_resolver->rules()->AddRule("a.com", "10.0.0.1");
session_deps.host_resolver->rules()->AddRule("b.com", "10.0.0.2");
scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));

SSLSocketDataProvider ssl1(ASYNC, OK);
ssl1.SetNextProto(kProtoSPDY21);
SSLSocketDataProvider ssl2(ASYNC, OK);
ssl2.SetNextProto(kProtoSPDY21);
session_deps.socket_factory.AddSSLSocketDataProvider(&ssl1);
session_deps.socket_factory.AddSSLSocketDataProvider(&ssl2);

scoped_ptr<SpdyFrame> host1_req(ConstructSpdyGet(
"https://www.a.com", false, 1, LOWEST));
MockWrite spdy1_writes[] = {
CreateMockWrite(*host1_req, 1),
};
scoped_ptr<SpdyFrame> host1_resp(ConstructSpdyGetSynReply(NULL, 0, 1));
scoped_ptr<SpdyFrame> host1_resp_body(ConstructSpdyBodyFrame(1, true));
MockRead spdy1_reads[] = {
CreateMockRead(*host1_resp, 2),
CreateMockRead(*host1_resp_body, 3),
MockRead(ASYNC, ERR_IO_PENDING, 4),
};

scoped_ptr<OrderedSocketData> spdy1_data(
new OrderedSocketData(
spdy1_reads, arraysize(spdy1_reads),
spdy1_writes, arraysize(spdy1_writes)));
session_deps.socket_factory.AddSocketDataProvider(spdy1_data.get());

scoped_ptr<SpdyFrame> host2_req(ConstructSpdyGet(
"https://www.b.com", false, 1, LOWEST));
MockWrite spdy2_writes[] = {
CreateMockWrite(*host2_req, 1),
};
scoped_ptr<SpdyFrame> host2_resp(ConstructSpdyGetSynReply(NULL, 0, 1));
scoped_ptr<SpdyFrame> host2_resp_body(ConstructSpdyBodyFrame(1, true));
MockRead spdy2_reads[] = {
CreateMockRead(*host2_resp, 2),
CreateMockRead(*host2_resp_body, 3),
MockRead(ASYNC, ERR_IO_PENDING, 4),
};

scoped_ptr<OrderedSocketData> spdy2_data(
new OrderedSocketData(
spdy2_reads, arraysize(spdy2_reads),
spdy2_writes, arraysize(spdy2_writes)));
session_deps.socket_factory.AddSocketDataProvider(spdy2_data.get());

MockWrite http_write[] = {
MockWrite("GET / HTTP/1.1\r\n"
"Host: www.a.com\r\n"
"Connection: keep-alive\r\n\r\n"),
};

MockRead http_read[] = {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
MockRead("Content-Length: 6\r\n\r\n"),
MockRead("hello!"),
};
StaticSocketDataProvider http_data(http_read, arraysize(http_read),
http_write, arraysize(http_write));
session_deps.socket_factory.AddSocketDataProvider(&http_data);

HostPortPair host_port_pair_a("www.a.com", 443);
HostPortProxyPair host_port_proxy_pair_a(
host_port_pair_a, ProxyServer::Direct());
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));

TestCompletionCallback callback;
HttpRequestInfo request1;
request1.method = "GET";
request1.url = GURL("https://www.a.com/");
request1.load_flags = 0;
scoped_ptr<HttpNetworkTransaction> trans(new HttpNetworkTransaction(session));

int rv = trans->Start(&request1, callback.callback(), BoundNetLog());
EXPECT_EQ(ERR_IO_PENDING, rv);
EXPECT_EQ(OK, callback.WaitForResult());

const HttpResponseInfo* response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
ASSERT_TRUE(response->headers != NULL);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
EXPECT_TRUE(response->was_fetched_via_spdy);
EXPECT_TRUE(response->was_npn_negotiated);

std::string response_data;
ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
EXPECT_EQ("hello!", response_data);
trans.reset();
EXPECT_TRUE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));

HostPortPair host_port_pair_b("www.b.com", 443);
HostPortProxyPair host_port_proxy_pair_b(
host_port_pair_b, ProxyServer::Direct());
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_b));
HttpRequestInfo request2;
request2.method = "GET";
request2.url = GURL("https://www.b.com/");
request2.load_flags = 0;
trans.reset(new HttpNetworkTransaction(session));

rv = trans->Start(&request2, callback.callback(), BoundNetLog());
EXPECT_EQ(ERR_IO_PENDING, rv);
EXPECT_EQ(OK, callback.WaitForResult());

response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
ASSERT_TRUE(response->headers != NULL);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
EXPECT_TRUE(response->was_fetched_via_spdy);
EXPECT_TRUE(response->was_npn_negotiated);
ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
EXPECT_EQ("hello!", response_data);
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));
EXPECT_TRUE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_b));

HostPortPair host_port_pair_a1("www.a.com", 80);
HostPortProxyPair host_port_proxy_pair_a1(
host_port_pair_a1, ProxyServer::Direct());
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a1));
HttpRequestInfo request3;
request3.method = "GET";
request3.url = GURL("http://www.a.com/");
request3.load_flags = 0;
trans.reset(new HttpNetworkTransaction(session));

rv = trans->Start(&request3, callback.callback(), BoundNetLog());
EXPECT_EQ(ERR_IO_PENDING, rv);
EXPECT_EQ(OK, callback.WaitForResult());

response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
ASSERT_TRUE(response->headers != NULL);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
EXPECT_FALSE(response->was_fetched_via_spdy);
EXPECT_FALSE(response->was_npn_negotiated);
ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
EXPECT_EQ("hello!", response_data);
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_b));

HttpStreamFactory::SetNextProtos(std::vector<std::string>());
ClientSocketPoolManager::set_max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets_per_pool);
ClientSocketPoolManager::set_max_sockets_per_proxy_server(
HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets_per_proxy_server);
ClientSocketPoolManager::set_max_sockets_per_group(
HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets_per_group);
}

} // namespace net
181 changes: 0 additions & 181 deletions net/http/http_network_transaction_spdy2_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9551,185 +9551,4 @@ TEST_F(HttpNetworkTransactionSpdy2Test, SendPipelineEvictionFallback) {
EXPECT_EQ("hello world", out.response_data);
}

TEST_F(HttpNetworkTransactionSpdy2Test, CloseIdleSpdySessionToOpenNewOne) {
HttpStreamFactory::SetNextProtos(SpdyNextProtos());
int old_max_sockets_per_group =
ClientSocketPoolManager::max_sockets_per_group(
HttpNetworkSession::NORMAL_SOCKET_POOL);
int old_max_sockets_per_proxy_server =
ClientSocketPoolManager::max_sockets_per_proxy_server(
HttpNetworkSession::NORMAL_SOCKET_POOL);
int old_max_sockets_per_pool =
ClientSocketPoolManager::max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL);
ClientSocketPoolManager::set_max_sockets_per_group(
HttpNetworkSession::NORMAL_SOCKET_POOL, 1);
ClientSocketPoolManager::set_max_sockets_per_proxy_server(
HttpNetworkSession::NORMAL_SOCKET_POOL, 1);
ClientSocketPoolManager::set_max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL, 1);

// Use two different hosts with different IPs so they don't get pooled.
SessionDependencies session_deps;
session_deps.host_resolver->rules()->AddRule("a.com", "10.0.0.1");
session_deps.host_resolver->rules()->AddRule("b.com", "10.0.0.2");
scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));

SSLSocketDataProvider ssl1(ASYNC, OK);
ssl1.SetNextProto(kProtoSPDY2);
SSLSocketDataProvider ssl2(ASYNC, OK);
ssl2.SetNextProto(kProtoSPDY2);
session_deps.socket_factory.AddSSLSocketDataProvider(&ssl1);
session_deps.socket_factory.AddSSLSocketDataProvider(&ssl2);

scoped_ptr<SpdyFrame> host1_req(ConstructSpdyGet(
"https://www.a.com", false, 1, LOWEST));
MockWrite spdy1_writes[] = {
CreateMockWrite(*host1_req, 1),
};
scoped_ptr<SpdyFrame> host1_resp(ConstructSpdyGetSynReply(NULL, 0, 1));
scoped_ptr<SpdyFrame> host1_resp_body(ConstructSpdyBodyFrame(1, true));
MockRead spdy1_reads[] = {
CreateMockRead(*host1_resp, 2),
CreateMockRead(*host1_resp_body, 3),
MockRead(ASYNC, ERR_IO_PENDING, 4),
};

scoped_ptr<OrderedSocketData> spdy1_data(
new OrderedSocketData(
spdy1_reads, arraysize(spdy1_reads),
spdy1_writes, arraysize(spdy1_writes)));
session_deps.socket_factory.AddSocketDataProvider(spdy1_data.get());

scoped_ptr<SpdyFrame> host2_req(ConstructSpdyGet(
"https://www.b.com", false, 1, LOWEST));
MockWrite spdy2_writes[] = {
CreateMockWrite(*host2_req, 1),
};
scoped_ptr<SpdyFrame> host2_resp(ConstructSpdyGetSynReply(NULL, 0, 1));
scoped_ptr<SpdyFrame> host2_resp_body(ConstructSpdyBodyFrame(1, true));
MockRead spdy2_reads[] = {
CreateMockRead(*host2_resp, 2),
CreateMockRead(*host2_resp_body, 3),
MockRead(ASYNC, ERR_IO_PENDING, 4),
};

scoped_ptr<OrderedSocketData> spdy2_data(
new OrderedSocketData(
spdy2_reads, arraysize(spdy2_reads),
spdy2_writes, arraysize(spdy2_writes)));
session_deps.socket_factory.AddSocketDataProvider(spdy2_data.get());

MockWrite http_write[] = {
MockWrite("GET / HTTP/1.1\r\n"
"Host: www.a.com\r\n"
"Connection: keep-alive\r\n\r\n"),
};

MockRead http_read[] = {
MockRead("HTTP/1.1 200 OK\r\n"),
MockRead("Content-Type: text/html; charset=iso-8859-1\r\n"),
MockRead("Content-Length: 6\r\n\r\n"),
MockRead("hello!"),
};
StaticSocketDataProvider http_data(http_read, arraysize(http_read),
http_write, arraysize(http_write));
session_deps.socket_factory.AddSocketDataProvider(&http_data);

HostPortPair host_port_pair_a("www.a.com", 443);
HostPortProxyPair host_port_proxy_pair_a(
host_port_pair_a, ProxyServer::Direct());
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));

TestCompletionCallback callback;
HttpRequestInfo request1;
request1.method = "GET";
request1.url = GURL("https://www.a.com/");
request1.load_flags = 0;
scoped_ptr<HttpNetworkTransaction> trans(new HttpNetworkTransaction(session));

int rv = trans->Start(&request1, callback.callback(), BoundNetLog());
EXPECT_EQ(ERR_IO_PENDING, rv);
EXPECT_EQ(OK, callback.WaitForResult());

const HttpResponseInfo* response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
ASSERT_TRUE(response->headers != NULL);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
EXPECT_TRUE(response->was_fetched_via_spdy);
EXPECT_TRUE(response->was_npn_negotiated);

std::string response_data;
ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
EXPECT_EQ("hello!", response_data);
trans.reset();
EXPECT_TRUE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));

HostPortPair host_port_pair_b("www.b.com", 443);
HostPortProxyPair host_port_proxy_pair_b(
host_port_pair_b, ProxyServer::Direct());
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_b));
HttpRequestInfo request2;
request2.method = "GET";
request2.url = GURL("https://www.b.com/");
request2.load_flags = 0;
trans.reset(new HttpNetworkTransaction(session));

rv = trans->Start(&request2, callback.callback(), BoundNetLog());
EXPECT_EQ(ERR_IO_PENDING, rv);
EXPECT_EQ(OK, callback.WaitForResult());

response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
ASSERT_TRUE(response->headers != NULL);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
EXPECT_TRUE(response->was_fetched_via_spdy);
EXPECT_TRUE(response->was_npn_negotiated);
ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
EXPECT_EQ("hello!", response_data);
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));
EXPECT_TRUE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_b));

HostPortPair host_port_pair_a1("www.a.com", 80);
HostPortProxyPair host_port_proxy_pair_a1(
host_port_pair_a1, ProxyServer::Direct());
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a1));
HttpRequestInfo request3;
request3.method = "GET";
request3.url = GURL("http://www.a.com/");
request3.load_flags = 0;
trans.reset(new HttpNetworkTransaction(session));

rv = trans->Start(&request3, callback.callback(), BoundNetLog());
EXPECT_EQ(ERR_IO_PENDING, rv);
EXPECT_EQ(OK, callback.WaitForResult());

response = trans->GetResponseInfo();
ASSERT_TRUE(response != NULL);
ASSERT_TRUE(response->headers != NULL);
EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
EXPECT_FALSE(response->was_fetched_via_spdy);
EXPECT_FALSE(response->was_npn_negotiated);
ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
EXPECT_EQ("hello!", response_data);
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_a));
EXPECT_FALSE(
session->spdy_session_pool()->HasSession(host_port_proxy_pair_b));

HttpStreamFactory::SetNextProtos(std::vector<std::string>());
ClientSocketPoolManager::set_max_sockets_per_pool(
HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets_per_pool);
ClientSocketPoolManager::set_max_sockets_per_proxy_server(
HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets_per_proxy_server);
ClientSocketPoolManager::set_max_sockets_per_group(
HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets_per_group);
}

} // namespace net
Loading

0 comments on commit 2c75400

Please sign in to comment.