Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1687)

Unified Diff: net/socket/client_socket_pool_base_unittest.cc

Issue 159597: Fix crash in client_socket_pool_base.cc. (Closed)
Patch Set: add test for late binding scenario Created 11 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/socket/client_socket_pool_base.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/socket/client_socket_pool_base_unittest.cc
diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc
index 730e04114c8b6aae4ceaba4268531ba5cdb5828e..3ccce17ab9f9cefdb5810476445dafae09e91438 100644
--- a/net/socket/client_socket_pool_base_unittest.cc
+++ b/net/socket/client_socket_pool_base_unittest.cc
@@ -974,6 +974,41 @@ TEST_F(ClientSocketPoolBaseTest, ReleaseSockets) {
EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
}
+// Regression test for http://crbug.com/17985.
+TEST_F(ClientSocketPoolBaseTest, GroupWithPendingRequestsIsNotEmpty) {
+ const int kMaxSockets = 3;
+ const int kMaxSocketsPerGroup = 2;
+ CreatePool(kMaxSockets, kMaxSocketsPerGroup);
+
+ const int kHighPriority = kDefaultPriority + 100;
+
+ EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+ EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+
+ // This is going to be a pending request in an otherwise empty group.
+ EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
+
+ // Reach the maximum socket limit.
+ EXPECT_EQ(OK, StartRequest("b", kDefaultPriority));
+
+ // Create a stalled group with high priorities.
+ EXPECT_EQ(ERR_IO_PENDING, StartRequest("c", kHighPriority));
+ EXPECT_EQ(ERR_IO_PENDING, StartRequest("c", kHighPriority));
+ EXPECT_TRUE(pool_->base()->may_have_stalled_group());
+
+ // Release the first two sockets from "a", which will make place
wtc 2009/07/29 20:27:25 Nit: "make place" => "make room"? Or did you mean
+ // for requests from "c". After that "a" will have no active sockets
+ // and one pending request.
+ EXPECT_TRUE(ReleaseOneConnection(KEEP_ALIVE));
+ EXPECT_TRUE(ReleaseOneConnection(KEEP_ALIVE));
+
+ // Closing idle sockets should not get us into trouble, but in the bug
+ // we were hitting a CHECK here.
+ EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a"));
+ pool_->CloseIdleSockets();
+ EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+}
+
class ClientSocketPoolBaseTest_LateBinding : public ClientSocketPoolBaseTest {
protected:
virtual void SetUp() {
@@ -1376,6 +1411,42 @@ TEST_F(ClientSocketPoolBaseTest_LateBinding, DISABLED_LoadState) {
EXPECT_EQ(LOAD_STATE_WAITING_FOR_CACHE, req2.handle()->GetLoadState());
}
+// Regression test for http://crbug.com/17985.
+TEST_F(ClientSocketPoolBaseTest_LateBinding,
+ GroupWithPendingRequestsIsNotEmpty) {
+ const int kMaxSockets = 3;
+ const int kMaxSocketsPerGroup = 2;
+ CreatePool(kMaxSockets, kMaxSocketsPerGroup);
+
+ const int kHighPriority = kDefaultPriority + 100;
+
+ EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+ EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+
+ // This is going to be a pending request in an otherwise empty group.
+ EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
+
+ // Reach the maximum socket limit.
+ EXPECT_EQ(OK, StartRequest("b", kDefaultPriority));
+
+ // Create a stalled group with high priorities.
+ EXPECT_EQ(ERR_IO_PENDING, StartRequest("c", kHighPriority));
+ EXPECT_EQ(ERR_IO_PENDING, StartRequest("c", kHighPriority));
+ EXPECT_TRUE(pool_->base()->may_have_stalled_group());
+
+ // Release the first two sockets from "a", which will make place
wtc 2009/07/29 20:27:25 Nit: "make place" => "make room"?
+ // for requests from "c". After that "a" will have no active sockets
+ // and one pending request.
+ EXPECT_TRUE(ReleaseOneConnection(KEEP_ALIVE));
+ EXPECT_TRUE(ReleaseOneConnection(KEEP_ALIVE));
+
+ // Closing idle sockets should not get us into trouble, but in the bug
+ // we were hitting a CHECK here.
+ EXPECT_EQ(2, pool_->IdleSocketCountInGroup("a"));
+ pool_->CloseIdleSockets();
+ EXPECT_EQ(0, pool_->IdleSocketCountInGroup("a"));
+}
+
} // namespace
} // namespace net
« no previous file with comments | « net/socket/client_socket_pool_base.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698