[go: up one dir, main page]

[M41 Merge] Add a delay when unlocking WebSocket endpoints.

The synchronous nature of endpoint unlocks for WebSocket throttling permits
denial-of-service attacks. Make it asynchronous, and add a small delay to
make attack timing harder.

BUG=442756
TEST=net_unittests, layout tests
TBR=tyoshino, mmenke

Review URL: https://codereview.chromium.org/835623003

Cr-Commit-Position: refs/heads/master@{#312774}
(cherry picked from commit bafe0f29a763c8325690cbdc42c29fc54d487d4f)

Review URL: https://codereview.chromium.org/935993002

Cr-Commit-Position: refs/branch-heads/2272@{#324}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 658182b..791c9f6 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -24,6 +24,7 @@
 #include "net/http/http_response_headers.h"
 #include "net/socket/client_socket_pool_histograms.h"
 #include "net/socket/socket.h"
+#include "net/socket/websocket_endpoint_lock_manager.h"
 #include "net/ssl/ssl_cert_request_info.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/ssl/ssl_info.h"
@@ -1989,6 +1990,21 @@
   return transport_pool_->ReleaseSocket(group_name, socket.Pass(), id);
 }
 
+ScopedWebSocketEndpointZeroUnlockDelay::
+    ScopedWebSocketEndpointZeroUnlockDelay() {
+  old_delay_ =
+      WebSocketEndpointLockManager::GetInstance()->SetUnlockDelayForTesting(
+          base::TimeDelta());
+}
+
+ScopedWebSocketEndpointZeroUnlockDelay::
+    ~ScopedWebSocketEndpointZeroUnlockDelay() {
+  base::TimeDelta active_delay =
+      WebSocketEndpointLockManager::GetInstance()->SetUnlockDelayForTesting(
+          old_delay_);
+  EXPECT_EQ(active_delay, base::TimeDelta());
+}
+
 const char kSOCKS5GreetRequest[] = { 0x05, 0x01, 0x00 };
 const int kSOCKS5GreetRequestLength = arraysize(kSOCKS5GreetRequest);
 
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 6d3162b..d6c7aba 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -18,6 +18,7 @@
 #include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "base/strings/string16.h"
+#include "base/time/time.h"
 #include "net/base/address_list.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
@@ -1319,6 +1320,18 @@
   DISALLOW_COPY_AND_ASSIGN(MockSOCKSClientSocketPool);
 };
 
+// Convenience class to temporarily set the WebSocketEndpointLockManager unlock
+// delay to zero for testing purposes. Automatically restores the original value
+// when destroyed.
+class ScopedWebSocketEndpointZeroUnlockDelay {
+ public:
+  ScopedWebSocketEndpointZeroUnlockDelay();
+  ~ScopedWebSocketEndpointZeroUnlockDelay();
+
+ private:
+  base::TimeDelta old_delay_;
+};
+
 // Constants for a successful SOCKS v5 handshake.
 extern const char kSOCKS5GreetRequest[];
 extern const int kSOCKS5GreetRequestLength;
diff --git a/net/socket/websocket_endpoint_lock_manager.cc b/net/socket/websocket_endpoint_lock_manager.cc
index e578bb2..1bccb1d 100644
--- a/net/socket/websocket_endpoint_lock_manager.cc
+++ b/net/socket/websocket_endpoint_lock_manager.cc
@@ -6,12 +6,23 @@
 
 #include <utility>
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_log.h"
 
 namespace net {
 
+namespace {
+
+// This delay prevents DoS attacks.
+// TODO(ricea): Replace this with randomised truncated exponential backoff.
+// See crbug.com/377613.
+const int kUnlockDelayInMs = 10;
+
+}  // namespace
+
 WebSocketEndpointLockManager::Waiter::~Waiter() {
   if (next()) {
     DCHECK(previous());
@@ -65,23 +76,31 @@
            << lock_info_it->first.ToString() << " ("
            << socket_lock_info_map_.size() << " socket(s) left)";
   socket_lock_info_map_.erase(socket_it);
-  DCHECK(socket == lock_info_it->second.socket);
+  DCHECK_EQ(socket, lock_info_it->second.socket);
   lock_info_it->second.socket = NULL;
-  UnlockEndpointByIterator(lock_info_it);
+  UnlockEndpointAfterDelay(lock_info_it->first);
 }
 
 void WebSocketEndpointLockManager::UnlockEndpoint(const IPEndPoint& endpoint) {
   LockInfoMap::iterator lock_info_it = lock_info_map_.find(endpoint);
   if (lock_info_it == lock_info_map_.end())
     return;
-
-  UnlockEndpointByIterator(lock_info_it);
+  if (lock_info_it->second.socket)
+    EraseSocket(lock_info_it);
+  UnlockEndpointAfterDelay(endpoint);
 }
 
 bool WebSocketEndpointLockManager::IsEmpty() const {
   return lock_info_map_.empty() && socket_lock_info_map_.empty();
 }
 
+base::TimeDelta WebSocketEndpointLockManager::SetUnlockDelayForTesting(
+    base::TimeDelta new_delay) {
+  base::TimeDelta old_delay = unlock_delay_;
+  unlock_delay_ = new_delay;
+  return old_delay;
+}
+
 WebSocketEndpointLockManager::LockInfo::LockInfo() : socket(NULL) {}
 WebSocketEndpointLockManager::LockInfo::~LockInfo() {
   DCHECK(!socket);
@@ -92,17 +111,37 @@
   DCHECK(!rhs.queue);
 }
 
-WebSocketEndpointLockManager::WebSocketEndpointLockManager() {}
+WebSocketEndpointLockManager::WebSocketEndpointLockManager()
+    : unlock_delay_(base::TimeDelta::FromMilliseconds(kUnlockDelayInMs)),
+      pending_unlock_count_(0),
+      weak_factory_(this) {
+}
 
 WebSocketEndpointLockManager::~WebSocketEndpointLockManager() {
-  DCHECK(lock_info_map_.empty());
+  DCHECK_EQ(lock_info_map_.size(), pending_unlock_count_);
   DCHECK(socket_lock_info_map_.empty());
 }
 
-void WebSocketEndpointLockManager::UnlockEndpointByIterator(
-    LockInfoMap::iterator lock_info_it) {
-  if (lock_info_it->second.socket)
-    EraseSocket(lock_info_it);
+void WebSocketEndpointLockManager::UnlockEndpointAfterDelay(
+    const IPEndPoint& endpoint) {
+  DVLOG(3) << "Delaying " << unlock_delay_.InMilliseconds()
+           << "ms before unlocking endpoint " << endpoint.ToString();
+  ++pending_unlock_count_;
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE,
+      base::Bind(&WebSocketEndpointLockManager::DelayedUnlockEndpoint,
+                 weak_factory_.GetWeakPtr(), endpoint),
+      unlock_delay_);
+}
+
+void WebSocketEndpointLockManager::DelayedUnlockEndpoint(
+    const IPEndPoint& endpoint) {
+  LockInfoMap::iterator lock_info_it = lock_info_map_.find(endpoint);
+  DCHECK_GT(pending_unlock_count_, 0U);
+  --pending_unlock_count_;
+  if (lock_info_it == lock_info_map_.end())
+    return;
+  DCHECK(!lock_info_it->second.socket);
   LockInfo::WaiterQueue* queue = lock_info_it->second.queue.get();
   DCHECK(queue);
   if (queue->empty()) {
@@ -115,7 +154,6 @@
            << " and activating next waiter";
   Waiter* next_job = queue->head()->value();
   next_job->RemoveFromList();
-  // This must be last to minimise the excitement caused by re-entrancy.
   next_job->GotEndpointLock();
 }
 
diff --git a/net/socket/websocket_endpoint_lock_manager.h b/net/socket/websocket_endpoint_lock_manager.h
index d5cad50..bddd5455 100644
--- a/net/socket/websocket_endpoint_lock_manager.h
+++ b/net/socket/websocket_endpoint_lock_manager.h
@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/singleton.h"
+#include "base/time/time.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_export.h"
 #include "net/socket/websocket_transport_client_socket_pool.h"
@@ -19,8 +20,25 @@
 
 class StreamSocket;
 
+// Keep track of ongoing WebSocket connections in order to satisfy the WebSocket
+// connection throttling requirements described in RFC6455 4.1.2:
+//
+//   2.  If the client already has a WebSocket connection to the remote
+//       host (IP address) identified by /host/ and port /port/ pair, even
+//       if the remote host is known by another name, the client MUST wait
+//       until that connection has been established or for that connection
+//       to have failed.  There MUST be no more than one connection in a
+//       CONNECTING state.  If multiple connections to the same IP address
+//       are attempted simultaneously, the client MUST serialize them so
+//       that there is no more than one connection at a time running
+//       through the following steps.
+//
+// This class is neither thread-safe nor thread-compatible.
+// TODO(ricea): Make this class thread-compatible by making it not be a
+// singleton.
 class NET_EXPORT_PRIVATE WebSocketEndpointLockManager {
  public:
+  // Implement this interface to wait for an endpoint to be available.
   class NET_EXPORT_PRIVATE Waiter : public base::LinkNode<Waiter> {
    public:
     // If the node is in a list, removes it.
@@ -45,22 +63,28 @@
   // UnlockSocket().
   void RememberSocket(StreamSocket* socket, const IPEndPoint& endpoint);
 
-  // Releases the lock on the endpoint that was associated with |socket| by
-  // RememberSocket(). If appropriate, triggers the next socket connection.
-  // Should be called exactly once for each |socket| that was passed to
-  // RememberSocket(). Does nothing if UnlockEndpoint() has been called since
+  // Removes the socket association that was recorded by RememberSocket(), then
+  // asynchronously releases the lock on the endpoint after a delay. If
+  // appropriate, calls |waiter->GetEndpointLock()| when the lock is
+  // released. Should be called exactly once for each |socket| that was passed
+  // to RememberSocket(). Does nothing if UnlockEndpoint() has been called since
   // the call to RememberSocket().
   void UnlockSocket(StreamSocket* socket);
 
-  // Releases the lock on |endpoint|. Does nothing if |endpoint| is not locked.
-  // Removes any socket association that was recorded with RememberSocket(). If
-  // appropriate, calls |waiter->GotEndpointLock()|.
+  // Asynchronously releases the lock on |endpoint| after a delay. Does nothing
+  // if |endpoint| is not locked.  Removes any socket association that was
+  // recorded with RememberSocket(). If appropriate, calls
+  // |waiter->GotEndpointLock()| when the lock is released.
   void UnlockEndpoint(const IPEndPoint& endpoint);
 
   // Checks that |lock_info_map_| and |socket_lock_info_map_| are empty. For
   // tests.
   bool IsEmpty() const;
 
+  // Changes the value of the unlock delay. Returns the previous value of the
+  // delay.
+  base::TimeDelta SetUnlockDelayForTesting(base::TimeDelta new_delay);
+
  private:
   struct LockInfo {
     typedef base::LinkedList<Waiter> WaiterQueue;
@@ -97,7 +121,8 @@
   WebSocketEndpointLockManager();
   ~WebSocketEndpointLockManager();
 
-  void UnlockEndpointByIterator(LockInfoMap::iterator lock_info_it);
+  void UnlockEndpointAfterDelay(const IPEndPoint& endpoint);
+  void DelayedUnlockEndpoint(const IPEndPoint& endpoint);
   void EraseSocket(LockInfoMap::iterator lock_info_it);
 
   // If an entry is present in the map for a particular endpoint, then that
@@ -111,6 +136,16 @@
   // is non-NULL if and only if there is an entry in this map for the socket.
   SocketLockInfoMap socket_lock_info_map_;
 
+  // Time to wait between a call to Unlock* and actually unlocking the socket.
+  base::TimeDelta unlock_delay_;
+
+  // Number of sockets currently pending unlock.
+  size_t pending_unlock_count_;
+
+  // The messsage loop holding the unlock delay callback may outlive this
+  // object.
+  base::WeakPtrFactory<WebSocketEndpointLockManager> weak_factory_;
+
   friend struct DefaultSingletonTraits<WebSocketEndpointLockManager>;
 
   DISALLOW_COPY_AND_ASSIGN(WebSocketEndpointLockManager);
diff --git a/net/socket/websocket_endpoint_lock_manager_unittest.cc b/net/socket/websocket_endpoint_lock_manager_unittest.cc
index 1626aa90..2974b5a3 100644
--- a/net/socket/websocket_endpoint_lock_manager_unittest.cc
+++ b/net/socket/websocket_endpoint_lock_manager_unittest.cc
@@ -4,6 +4,9 @@
 
 #include "net/socket/websocket_endpoint_lock_manager.h"
 
+#include "base/message_loop/message_loop.h"
+#include "base/run_loop.h"
+#include "base/time/time.h"
 #include "net/base/net_errors.h"
 #include "net/socket/next_proto.h"
 #include "net/socket/socket_test_util.h"
@@ -94,6 +97,8 @@
   WebSocketEndpointLockManagerTest()
       : instance_(WebSocketEndpointLockManager::GetInstance()) {}
   ~WebSocketEndpointLockManagerTest() override {
+    // Permit any pending asynchronous unlock operations to complete.
+    RunUntilIdle();
     // If this check fails then subsequent tests may fail.
     CHECK(instance_->IsEmpty());
   }
@@ -109,10 +114,14 @@
   void UnlockDummyEndpoint(int times) {
     for (int i = 0; i < times; ++i) {
       instance()->UnlockEndpoint(DummyEndpoint());
+      RunUntilIdle();
     }
   }
 
+  static void RunUntilIdle() { base::RunLoop().RunUntilIdle(); }
+
   WebSocketEndpointLockManager* const instance_;
+  ScopedWebSocketEndpointZeroUnlockDelay zero_unlock_delay_;
 };
 
 TEST_F(WebSocketEndpointLockManagerTest, GetInstanceWorks) {
@@ -131,6 +140,7 @@
 TEST_F(WebSocketEndpointLockManagerTest, GotEndpointLockNotCalledOnOk) {
   FakeWaiter waiter;
   EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &waiter));
+  RunUntilIdle();
   EXPECT_FALSE(waiter.called());
 
   UnlockDummyEndpoint(1);
@@ -141,6 +151,7 @@
   EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &waiters[0]));
   EXPECT_EQ(ERR_IO_PENDING,
             instance()->LockEndpoint(DummyEndpoint(), &waiters[1]));
+  RunUntilIdle();
   EXPECT_FALSE(waiters[1].called());
 
   UnlockDummyEndpoint(2);
@@ -152,6 +163,7 @@
   EXPECT_EQ(ERR_IO_PENDING,
             instance()->LockEndpoint(DummyEndpoint(), &waiters[1]));
   instance()->UnlockEndpoint(DummyEndpoint());
+  RunUntilIdle();
   EXPECT_TRUE(waiters[1].called());
 
   UnlockDummyEndpoint(1);
@@ -169,6 +181,7 @@
   }
 
   instance()->UnlockEndpoint(DummyEndpoint());
+  RunUntilIdle();
 
   FakeWaiter second_lock_holder;
   EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &second_lock_holder));
@@ -185,6 +198,7 @@
 
   instance()->RememberSocket(&dummy_socket, DummyEndpoint());
   instance()->UnlockSocket(&dummy_socket);
+  RunUntilIdle();
   EXPECT_TRUE(waiters[1].called());
 
   UnlockDummyEndpoint(1);
@@ -199,6 +213,7 @@
   EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &waiter));
   instance()->RememberSocket(&dummy_socket, DummyEndpoint());
   instance()->UnlockEndpoint(DummyEndpoint());
+  RunUntilIdle();
   EXPECT_TRUE(instance()->IsEmpty());
 }
 
@@ -213,12 +228,70 @@
 
   instance()->RememberSocket(&dummy_sockets[0], DummyEndpoint());
   instance()->UnlockEndpoint(DummyEndpoint());
+  RunUntilIdle();
   EXPECT_TRUE(waiters[1].called());
   instance()->RememberSocket(&dummy_sockets[1], DummyEndpoint());
 
   UnlockDummyEndpoint(1);
 }
 
+// Calling UnlockSocket() after UnlockEndpoint() does nothing.
+TEST_F(WebSocketEndpointLockManagerTest,
+       UnlockSocketAfterUnlockEndpointDoesNothing) {
+  FakeWaiter waiters[3];
+  FakeStreamSocket dummy_socket;
+
+  EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &waiters[0]));
+  EXPECT_EQ(ERR_IO_PENDING,
+            instance()->LockEndpoint(DummyEndpoint(), &waiters[1]));
+  EXPECT_EQ(ERR_IO_PENDING,
+            instance()->LockEndpoint(DummyEndpoint(), &waiters[2]));
+  instance()->RememberSocket(&dummy_socket, DummyEndpoint());
+  instance()->UnlockEndpoint(DummyEndpoint());
+  instance()->UnlockSocket(&dummy_socket);
+  RunUntilIdle();
+  EXPECT_TRUE(waiters[1].called());
+  EXPECT_FALSE(waiters[2].called());
+
+  UnlockDummyEndpoint(2);
+}
+
+// UnlockEndpoint() should always be asynchronous.
+TEST_F(WebSocketEndpointLockManagerTest, UnlockEndpointIsAsynchronous) {
+  FakeWaiter waiters[2];
+  EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &waiters[0]));
+  EXPECT_EQ(ERR_IO_PENDING,
+            instance()->LockEndpoint(DummyEndpoint(), &waiters[1]));
+
+  instance()->UnlockEndpoint(DummyEndpoint());
+  EXPECT_FALSE(waiters[1].called());
+  RunUntilIdle();
+  EXPECT_TRUE(waiters[1].called());
+
+  UnlockDummyEndpoint(1);
+}
+
+// UnlockEndpoint() should normally have a delay.
+TEST_F(WebSocketEndpointLockManagerTest, UnlockEndpointIsDelayed) {
+  const base::TimeDelta one_millisecond = base::TimeDelta::FromMilliseconds(1);
+  instance()->SetUnlockDelayForTesting(one_millisecond);
+  FakeWaiter waiters[2];
+  EXPECT_EQ(OK, instance()->LockEndpoint(DummyEndpoint(), &waiters[0]));
+  EXPECT_EQ(ERR_IO_PENDING,
+            instance()->LockEndpoint(DummyEndpoint(), &waiters[1]));
+
+  instance()->UnlockEndpoint(DummyEndpoint());
+  RunUntilIdle();
+  EXPECT_FALSE(waiters[1].called());
+  base::RunLoop run_loop;
+  base::MessageLoop::current()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), one_millisecond);
+  run_loop.Run();
+  EXPECT_TRUE(waiters[1].called());
+  instance()->SetUnlockDelayForTesting(base::TimeDelta());
+  UnlockDummyEndpoint(1);
+}
+
 }  // namespace
 
 }  // namespace net
diff --git a/net/socket/websocket_transport_client_socket_pool_unittest.cc b/net/socket/websocket_transport_client_socket_pool_unittest.cc
index 2189181..03bdf43 100644
--- a/net/socket/websocket_transport_client_socket_pool_unittest.cc
+++ b/net/socket/websocket_transport_client_socket_pool_unittest.cc
@@ -48,7 +48,7 @@
   run_loop.Run();
 }
 
-class WebSocketTransportClientSocketPoolTest : public testing::Test {
+class WebSocketTransportClientSocketPoolTest : public ::testing::Test {
  protected:
   WebSocketTransportClientSocketPoolTest()
       : params_(new TransportSocketParams(
@@ -68,10 +68,15 @@
               NULL) {}
 
   ~WebSocketTransportClientSocketPoolTest() override {
+    RunUntilIdle();
+    // ReleaseAllConnections() calls RunUntilIdle() after releasing each
+    // connection.
     ReleaseAllConnections(ClientSocketPoolTest::NO_KEEP_ALIVE);
     EXPECT_TRUE(WebSocketEndpointLockManager::GetInstance()->IsEmpty());
   }
 
+  static void RunUntilIdle() { base::RunLoop().RunUntilIdle(); }
+
   int StartRequest(const std::string& group_name, RequestPriority priority) {
     scoped_refptr<TransportSocketParams> params(
         new TransportSocketParams(
@@ -108,6 +113,7 @@
   MockTransportClientSocketFactory client_socket_factory_;
   WebSocketTransportClientSocketPool pool_;
   ClientSocketPoolTest test_base_;
+  ScopedWebSocketEndpointZeroUnlockDelay zero_unlock_delay_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(WebSocketTransportClientSocketPoolTest);
@@ -502,7 +508,7 @@
   EXPECT_EQ(OK, request(0)->WaitForResult());
   EXPECT_FALSE(request(1)->handle()->is_initialized());
   request(0)->handle()->Reset();
-  base::RunLoop().RunUntilIdle();
+  RunUntilIdle();
   EXPECT_TRUE(request(1)->handle()->is_initialized());
 }
 
@@ -518,7 +524,7 @@
   EXPECT_EQ(OK, callback.WaitForResult());
   EXPECT_FALSE(request(0)->handle()->is_initialized());
   handle.reset();
-  base::RunLoop().RunUntilIdle();
+  RunUntilIdle();
   EXPECT_TRUE(request(0)->handle()->is_initialized());
 }
 
@@ -531,7 +537,7 @@
   EXPECT_EQ(OK, request(0)->WaitForResult());
   EXPECT_FALSE(request(1)->handle()->is_initialized());
   WebSocketTransportClientSocketPool::UnlockEndpoint(request(0)->handle());
-  base::RunLoop().RunUntilIdle();
+  RunUntilIdle();
   EXPECT_TRUE(request(1)->handle()->is_initialized());
 }
 
@@ -548,7 +554,7 @@
 
   EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
   EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
-  base::RunLoop().RunUntilIdle();
+  RunUntilIdle();
   pool_.CancelRequest("a", request(0)->handle());
   EXPECT_EQ(OK, request(1)->WaitForResult());
 }
@@ -902,8 +908,9 @@
 TEST_F(WebSocketTransportClientSocketPoolTest, MaxSocketsEnforced) {
   host_resolver_->set_synchronous_mode(true);
   for (int i = 0; i < kMaxSockets; ++i) {
-    EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+    ASSERT_EQ(OK, StartRequest("a", kDefaultPriority));
     WebSocketTransportClientSocketPool::UnlockEndpoint(request(i)->handle());
+    RunUntilIdle();
   }
   EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
 }
@@ -914,13 +921,13 @@
   }
   // Now there are 32 sockets waiting to connect, and one stalled.
   for (int i = 0; i < kMaxSockets; ++i) {
-    base::RunLoop().RunUntilIdle();
+    RunUntilIdle();
     EXPECT_TRUE(request(i)->handle()->is_initialized());
     EXPECT_TRUE(request(i)->handle()->socket());
     WebSocketTransportClientSocketPool::UnlockEndpoint(request(i)->handle());
   }
   // Now there are 32 sockets connected, and one stalled.
-  base::RunLoop().RunUntilIdle();
+  RunUntilIdle();
   EXPECT_FALSE(request(kMaxSockets)->handle()->is_initialized());
   EXPECT_FALSE(request(kMaxSockets)->handle()->socket());
 }
@@ -928,8 +935,9 @@
 TEST_F(WebSocketTransportClientSocketPoolTest, StalledSocketReleased) {
   host_resolver_->set_synchronous_mode(true);
   for (int i = 0; i < kMaxSockets; ++i) {
-    EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+    ASSERT_EQ(OK, StartRequest("a", kDefaultPriority));
     WebSocketTransportClientSocketPool::UnlockEndpoint(request(i)->handle());
+    RunUntilIdle();
   }
 
   EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
@@ -953,7 +961,7 @@
   }
   EXPECT_EQ(OK, request(0)->WaitForResult());
   request(1)->handle()->Reset();
-  base::RunLoop().RunUntilIdle();
+  RunUntilIdle();
   EXPECT_FALSE(pool_.IsStalled());
 }
 
@@ -972,6 +980,7 @@
     EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
   }
   request(kMaxSockets)->handle()->Reset();
+  RunUntilIdle();
   EXPECT_FALSE(pool_.IsStalled());
 }
 
@@ -1114,6 +1123,7 @@
 
   request(0)->handle()->Reset();  // calls CancelRequest()
 
+  RunUntilIdle();
   // We should now be able to create a new connection without blocking on the
   // endpoint lock.
   EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
@@ -1123,11 +1133,12 @@
 // Endpoint, not two.
 TEST_F(WebSocketTransportClientSocketPoolTest, EndpointLockIsOnlyReleasedOnce) {
   host_resolver_->set_synchronous_mode(true);
-  EXPECT_EQ(OK, StartRequest("a", kDefaultPriority));
+  ASSERT_EQ(OK, StartRequest("a", kDefaultPriority));
   EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
   EXPECT_EQ(ERR_IO_PENDING, StartRequest("a", kDefaultPriority));
   // First socket completes handshake.
   WebSocketTransportClientSocketPool::UnlockEndpoint(request(0)->handle());
+  RunUntilIdle();
   // First socket is closed.
   request(0)->handle()->Reset();
   // Second socket should have been released.
diff --git a/net/websockets/websocket_stream_test.cc b/net/websockets/websocket_stream_test.cc
index 801e84e..3f06d277 100644
--- a/net/websockets/websocket_stream_test.cc
+++ b/net/websockets/websocket_stream_test.cc
@@ -107,6 +107,12 @@
 class WebSocketStreamCreateTest : public ::testing::Test {
  public:
   WebSocketStreamCreateTest() : has_failed_(false), ssl_fatal_(false) {}
+  ~WebSocketStreamCreateTest() override {
+    // Permit any endpoint locks to be released.
+    stream_request_.reset();
+    stream_.reset();
+    RunUntilIdle();
+  }
 
   void CreateAndConnectCustomResponse(
       const std::string& socket_url,
@@ -249,6 +255,7 @@
   SSLInfo ssl_info_;
   bool ssl_fatal_;
   ScopedVector<SSLSocketDataProvider> ssl_data_;
+  ScopedWebSocketEndpointZeroUnlockDelay zero_unlock_delay_;
 };
 
 // There are enough tests of the Sec-WebSocket-Extensions header that they