Commit 10821128 authored by ukai@chromium.org's avatar ukai@chromium.org

Don't process the same address for websocket throttling.

Host resolver might return address list that contains the same address twice or more.  In this case, we should not process the address for websocket throttling multiple times.
If we don't have duplicate check of address, it would put the job throttle by the job itself (deadlock).

I think this is cause of failure in Linux perf (webkit.org) bot and websocket fail to connect localhost on ubuntu (crbug.com/36652)

BUG=36652,41319,40995
TEST=none

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@45156 0039d316-1c4b-4281-b951-d872f2087c98
parent 13fd6d12
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <string> #include <string>
#include "base/hash_tables.h"
#include "base/message_loop.h" #include "base/message_loop.h"
#include "base/ref_counted.h" #include "base/ref_counted.h"
#include "base/singleton.h" #include "base/singleton.h"
...@@ -53,10 +54,17 @@ WebSocketThrottle::~WebSocketThrottle() { ...@@ -53,10 +54,17 @@ WebSocketThrottle::~WebSocketThrottle() {
void WebSocketThrottle::PutInQueue(WebSocketJob* job) { void WebSocketThrottle::PutInQueue(WebSocketJob* job) {
queue_.push_back(job); queue_.push_back(job);
const AddressList& address_list = job->address_list(); const AddressList& address_list = job->address_list();
base::hash_set<std::string> address_set;
for (const struct addrinfo* addrinfo = address_list.head(); for (const struct addrinfo* addrinfo = address_list.head();
addrinfo != NULL; addrinfo != NULL;
addrinfo = addrinfo->ai_next) { addrinfo = addrinfo->ai_next) {
std::string addrkey = AddrinfoToHashkey(addrinfo); std::string addrkey = AddrinfoToHashkey(addrinfo);
// If |addrkey| is already processed, don't do it again.
if (address_set.find(addrkey) != address_set.end())
continue;
address_set.insert(addrkey);
ConnectingAddressMap::iterator iter = addr_map_.find(addrkey); ConnectingAddressMap::iterator iter = addr_map_.find(addrkey);
if (iter == addr_map_.end()) { if (iter == addr_map_.end()) {
ConnectingQueue* queue = new ConnectingQueue(); ConnectingQueue* queue = new ConnectingQueue();
...@@ -65,6 +73,7 @@ void WebSocketThrottle::PutInQueue(WebSocketJob* job) { ...@@ -65,6 +73,7 @@ void WebSocketThrottle::PutInQueue(WebSocketJob* job) {
} else { } else {
iter->second->push_back(job); iter->second->push_back(job);
job->SetWaiting(); job->SetWaiting();
DLOG(INFO) << "Waiting on " << addrkey;
} }
} }
} }
...@@ -83,12 +92,19 @@ void WebSocketThrottle::RemoveFromQueue(WebSocketJob* job) { ...@@ -83,12 +92,19 @@ void WebSocketThrottle::RemoveFromQueue(WebSocketJob* job) {
if (!in_queue) if (!in_queue)
return; return;
const AddressList& address_list = job->address_list(); const AddressList& address_list = job->address_list();
base::hash_set<std::string> address_set;
for (const struct addrinfo* addrinfo = address_list.head(); for (const struct addrinfo* addrinfo = address_list.head();
addrinfo != NULL; addrinfo != NULL;
addrinfo = addrinfo->ai_next) { addrinfo = addrinfo->ai_next) {
std::string addrkey = AddrinfoToHashkey(addrinfo); std::string addrkey = AddrinfoToHashkey(addrinfo);
// If |addrkey| is already processed, don't do it again.
if (address_set.find(addrkey) != address_set.end())
continue;
address_set.insert(addrkey);
ConnectingAddressMap::iterator iter = addr_map_.find(addrkey); ConnectingAddressMap::iterator iter = addr_map_.find(addrkey);
DCHECK(iter != addr_map_.end()); DCHECK(iter != addr_map_.end());
ConnectingQueue* queue = iter->second; ConnectingQueue* queue = iter->second;
// Job may not be front of queue when job is closed early while waiting. // Job may not be front of queue when job is closed early while waiting.
for (ConnectingQueue::iterator iter = queue->begin(); for (ConnectingQueue::iterator iter = queue->begin();
......
...@@ -279,4 +279,29 @@ TEST_F(WebSocketThrottleTest, Throttle) { ...@@ -279,4 +279,29 @@ TEST_F(WebSocketThrottleTest, Throttle) {
MessageLoopForIO::current()->RunAllPending(); MessageLoopForIO::current()->RunAllPending();
} }
TEST_F(WebSocketThrottleTest, NoThrottleForDuplicateAddress) {
DummySocketStreamDelegate delegate;
// For localhost: 127.0.0.1, 127.0.0.1
struct addrinfo* addr = AddAddr(127, 0, 0, 1, NULL);
addr = AddAddr(127, 0, 0, 1, addr);
scoped_refptr<WebSocketJob> w1 = new WebSocketJob(&delegate);
scoped_refptr<SocketStream> s1 =
new SocketStream(GURL("ws://localhost/"), w1.get());
w1->InitSocketStream(s1.get());
WebSocketThrottleTest::MockSocketStreamConnect(s1, addr);
DeleteAddrInfo(addr);
DLOG(INFO) << "socket1";
TestCompletionCallback callback_s1;
// Trying to open connection to localhost will start without wait.
EXPECT_EQ(OK, w1->OnStartOpenConnection(s1, &callback_s1));
DLOG(INFO) << "socket1 close";
w1->OnClose(s1.get());
s1->DetachDelegate();
DLOG(INFO) << "Done";
MessageLoopForIO::current()->RunAllPending();
}
} }
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