Limit the number of pending requests to the max number of file descri…

…ptors.

Each renderer is only allowed to use a fraction of that, so that one renderer
doesn't DOS the rest.

The limit improves behavior in edge cases and also imitates the functionality
lost when ResourceLoadScheduler was nuked in Blink. That limit was fixed at 10k, which is larger than the number of FDs we support on many platforms.

As part of this, I cleaned up the unit tests. Instead of checking in each test that we count 0 requests in flight (and missing a few tests), we just DCHECK that there are none in flight at destruction time. This exposed a bug with cross-process navigation, which I've also fixed.

BUG=226319

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@199300 0039d316-1c4b-4281-b951-d872f2087c98

base/memory/shared_memory.h

@@ -101,6 +101,9 @@ class BASE_EXPORT SharedMemory {
   // Closes a shared memory handle.
   static void CloseHandle(const SharedMemoryHandle& handle);
 
+  // Returns the maximum number of handles that can be open at once per process.
+  static size_t GetHandleLimit();
+
   // Creates a shared memory object as described by the options struct.
   // Returns true on success and false on failure.
   bool Create(const SharedMemoryCreateOptions& options);

base/memory/shared_memory_posix.cc

@@ -13,9 +13,10 @@
 #include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
-#include "base/threading/platform_thread.h"
+#include "base/process_util.h"
 #include "base/safe_strerror_posix.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/platform_thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/utf_string_conversions.h"
 
@@ -98,6 +99,11 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
     DPLOG(ERROR) << "close";
 }
 
+// static
+size_t SharedMemory::GetHandleLimit() {
+  return base::GetMaxFds();
+}
+
 bool SharedMemory::CreateAndMapAnonymous(size_t size) {
   return CreateAnonymous(size) && Map(size);
 }

base/memory/shared_memory_win.cc

@@ -87,6 +87,13 @@ void SharedMemory::CloseHandle(const SharedMemoryHandle& handle) {
   ::CloseHandle(handle);
 }
 
+// static
+size_t SharedMemory::GetHandleLimit() {
+  // Rounded down from value reported here:
+  // http://blogs.technet.com/b/markrussinovich/archive/2009/09/29/3283844.aspx
+  return static_cast<size_t>(1 << 23);
+}
+
 bool SharedMemory::CreateAndMapAnonymous(size_t size) {
   return CreateAnonymous(size) && Map(size);
 }

base/process_util.h

@@ -223,6 +223,10 @@ BASE_EXPORT extern const char kProcSelfExe[];
 // Returns the ID for the parent of the given process.
 BASE_EXPORT ProcessId GetParentProcessId(ProcessHandle process);
 
+// Returns the maximum number of file descriptors that can be open by a process
+// at once. If the number is unavailable, a conservative best guess is returned.
+size_t GetMaxFds();
+
 // Close all file descriptors, except those which are a destination in the
 // given multimap. Only call this function in a child process where you know
 // that there aren't any other threads.

base/process_util_ios.mm

@@ -7,6 +7,7 @@
 #import <Foundation/Foundation.h>
 #include <mach/task.h>
 #include <stdio.h>
+#include <sys/resource.h>
 
 #include "base/logging.h"
 
@@ -48,6 +49,23 @@ void RaiseProcessToHighPriority() {
   // Impossible on iOS. Do nothing.
 }
 
+size_t GetMaxFds() {
+  static const rlim_t kSystemDefaultMaxFds = 256;
+  rlim_t max_fds;
+  struct rlimit nofile;
+  if (getrlimit(RLIMIT_NOFILE, &nofile)) {
+    // Error case: Take a best guess.
+    max_fds = kSystemDefaultMaxFds;
+  } else {
+    max_fds = nofile.rlim_cur;
+  }
+
+  if (max_fds > INT_MAX)
+    max_fds = INT_MAX;
+
+  return static_cast<size_t>(max_fds);
+}
+
 ProcessMetrics::ProcessMetrics(ProcessHandle process) {}
 
 ProcessMetrics::~ProcessMetrics() {}

base/process_util_posix.cc

@@ -398,13 +398,9 @@ typedef scoped_ptr_malloc<DIR, ScopedDIRClose> ScopedDIR;
   static const char kFDDir[] = "/proc/self/fd";
 #endif
 
-void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
-  // DANGER: no calls to malloc are allowed from now on:
-  // http://crbug.com/36678
-
-  // Get the maximum number of FDs possible.
-  struct rlimit nofile;
+size_t GetMaxFds() {
   rlim_t max_fds;
+  struct rlimit nofile;
   if (getrlimit(RLIMIT_NOFILE, &nofile)) {
     // getrlimit failed. Take a best guess.
     max_fds = kSystemDefaultMaxFds;
@@ -416,11 +412,20 @@ void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
   if (max_fds > INT_MAX)
     max_fds = INT_MAX;
 
-  DirReaderPosix fd_dir(kFDDir);
+  return static_cast<size_t>(max_fds);
+}
+
+void CloseSuperfluousFds(const base::InjectiveMultimap& saved_mapping) {
+  // DANGER: no calls to malloc are allowed from now on:
+  // http://crbug.com/36678
+
+  // Get the maximum number of FDs possible.
+  size_t max_fds = GetMaxFds();
 
+  DirReaderPosix fd_dir(kFDDir);
   if (!fd_dir.IsValid()) {
     // Fallback case: Try every possible fd.
-    for (rlim_t i = 0; i < max_fds; ++i) {
+    for (size_t i = 0; i < max_fds; ++i) {
       const int fd = static_cast<int>(i);
       if (fd == STDIN_FILENO || fd == STDOUT_FILENO || fd == STDERR_FILENO)
         continue;

content/browser/loader/async_resource_handler.cc

@@ -91,12 +91,15 @@ AsyncResourceHandler::AsyncResourceHandler(
       pending_data_count_(0),
       allocation_size_(0),
       did_defer_(false),
+      has_checked_for_sufficient_resources_(false),
       sent_received_response_msg_(false),
       sent_first_data_msg_(false) {
   InitializeResourceBufferConstants();
 }
 
 AsyncResourceHandler::~AsyncResourceHandler() {
+  if (has_checked_for_sufficient_resources_)
+    rdh_->FinishedWithResourcesForRequest(request_);
 }
 
 bool AsyncResourceHandler::OnMessageReceived(const IPC::Message& message,
@@ -338,6 +341,14 @@ bool AsyncResourceHandler::EnsureResourceBufferIsInitialized() {
   if (buffer_ && buffer_->IsInitialized())
     return true;
 
+  if (!has_checked_for_sufficient_resources_) {
+    has_checked_for_sufficient_resources_ = true;
+    if (!rdh_->HasSufficientResourcesForRequest(request_)) {
+      controller()->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
+      return false;
+    }
+  }
+
   buffer_ = new ResourceBuffer();
   return buffer_->Initialize(kBufferSize,
                              kMinAllocationSize,

content/browser/loader/async_resource_handler.h

@@ -87,6 +87,7 @@ class AsyncResourceHandler : public ResourceHandler,
 
   bool did_defer_;
 
+  bool has_checked_for_sufficient_resources_;
   bool sent_received_response_msg_;
   bool sent_first_data_msg_;
 

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -121,6 +121,10 @@ const int kMaxOutstandingRequestsCostPerProcess = 26214400;
 // rather than being intended as fully accurate.
 const int kUserGestureWindowMs = 3500;
 
+// Ratio of |max_num_in_flight_requests_| that any one renderer is allowed to
+// use. Arbitrarily chosen.
+const double kMaxRequestsPerProcessRatio = 0.45;
+
 // All possible error codes from the network module. Note that the error codes
 // are all positive (since histograms expect positive sample values).
 const int kAllNetErrorCodes[] = {
@@ -307,6 +311,11 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl()
     : save_file_manager_(new SaveFileManager()),
       request_id_(-1),
       is_shutdown_(false),
+      num_in_flight_requests_(0),
+      max_num_in_flight_requests_(base::SharedMemory::GetHandleLimit()),
+      max_num_in_flight_requests_per_process_(
+          static_cast<int>(
+              max_num_in_flight_requests_ * kMaxRequestsPerProcessRatio)),
       max_outstanding_requests_cost_per_process_(
           kMaxOutstandingRequestsCostPerProcess),
       filter_(NULL),
@@ -332,6 +341,7 @@ ResourceDispatcherHostImpl::ResourceDispatcherHostImpl()
 }
 
 ResourceDispatcherHostImpl::~ResourceDispatcherHostImpl() {
+  DCHECK(outstanding_requests_stats_map_.empty());
   DCHECK(g_resource_dispatcher_host);
   g_resource_dispatcher_host = NULL;
 }
@@ -378,6 +388,7 @@ void ResourceDispatcherHostImpl::CancelRequestsForContext(
        i != pending_loaders_.end();) {
     if (i->second->GetRequestInfo()->GetContext() == context) {
       loaders_to_cancel.push_back(i->second);
+      IncrementOutstandingRequestsMemory(-1, *i->second->GetRequestInfo());
       pending_loaders_.erase(i++);
     } else {
       ++i;
@@ -403,8 +414,7 @@ void ResourceDispatcherHostImpl::CancelRequestsForContext(
         // We make the assumption that all requests on the list have the same
         // ResourceContext.
         DCHECK_EQ(context, info->GetContext());
-        IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
-                                               info->GetChildID());
+        IncrementOutstandingRequestsMemory(-1, *info);
         loaders_to_cancel.push_back(loader);
       }
       delete loaders;
@@ -913,6 +923,8 @@ void ResourceDispatcherHostImpl::BeginRequest(
     if (it != pending_loaders_.end()) {
       if (it->second->is_transferring()) {
         deferred_loader = it->second;
+        IncrementOutstandingRequestsMemory(-1,
+                                           *deferred_loader->GetRequestInfo());
         pending_loaders_.erase(it);
       } else {
         RecordAction(UserMetricsAction("BadMessageTerminate_RDH"));
@@ -1105,6 +1117,7 @@ void ResourceDispatcherHostImpl::BeginRequest(
 
   if (deferred_loader.get()) {
     pending_loaders_[extra_info->GetGlobalRequestID()] = deferred_loader;
+    IncrementOutstandingRequestsMemory(1, *extra_info);
     deferred_loader->CompleteTransfer(handler.Pass());
   } else {
     BeginRequestInternal(new_request.Pass(), handler.Pass());
@@ -1316,14 +1329,6 @@ void ResourceDispatcherHostImpl::MarkAsTransferredNavigation(
   GetLoader(id)->MarkAsTransferring();
 }
 
-int ResourceDispatcherHostImpl::GetOutstandingRequestsMemoryCost(
-    int child_id) const {
-  OutstandingRequestsMemoryCostMap::const_iterator entry =
-      outstanding_requests_memory_cost_map_.find(child_id);
-  return (entry == outstanding_requests_memory_cost_map_.end()) ?
-      0 : entry->second;
-}
-
 // The object died, so cancel and detach all requests associated with it except
 // for downloads, which belong to the browser process even if initiated via a
 // renderer.
@@ -1439,8 +1444,7 @@ void ResourceDispatcherHostImpl::RemovePendingLoader(
 
   // Remove the memory credit that we added when pushing the request onto
   // the pending list.
-  IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
-                                         info->GetChildID());
+  IncrementOutstandingRequestsMemory(-1, *info);
 
   pending_loaders_.erase(iter);
 
@@ -1471,25 +1475,77 @@ void ResourceDispatcherHostImpl::CancelRequest(int child_id,
   loader->CancelRequest(from_renderer);
 }
 
-int ResourceDispatcherHostImpl::IncrementOutstandingRequestsMemoryCost(
-    int cost,
-    int child_id) {
-  // Retrieve the previous value (defaulting to 0 if not found).
-  OutstandingRequestsMemoryCostMap::iterator prev_entry =
-      outstanding_requests_memory_cost_map_.find(child_id);
-  int new_cost = 0;
-  if (prev_entry != outstanding_requests_memory_cost_map_.end())
-    new_cost = prev_entry->second;
-
-  // Insert/update the total; delete entries when their value reaches 0.
-  new_cost += cost;
-  CHECK(new_cost >= 0);
-  if (new_cost == 0)
-    outstanding_requests_memory_cost_map_.erase(child_id);
+ResourceDispatcherHostImpl::OustandingRequestsStats
+ResourceDispatcherHostImpl::GetOutstandingRequestsStats(
+    const ResourceRequestInfoImpl& info) {
+  OutstandingRequestsStatsMap::iterator entry =
+      outstanding_requests_stats_map_.find(info.GetChildID());
+  OustandingRequestsStats stats = { 0, 0 };
+  if (entry != outstanding_requests_stats_map_.end())
+    stats = entry->second;
+  return stats;
+}
+
+void ResourceDispatcherHostImpl::UpdateOutstandingRequestsStats(
+    const ResourceRequestInfoImpl& info,
+    const OustandingRequestsStats& stats) {
+  if (stats.memory_cost == 0 && stats.num_requests == 0)
+    outstanding_requests_stats_map_.erase(info.GetChildID());
   else
-    outstanding_requests_memory_cost_map_[child_id] = new_cost;
+    outstanding_requests_stats_map_[info.GetChildID()] = stats;
+}
+
+ResourceDispatcherHostImpl::OustandingRequestsStats
+ResourceDispatcherHostImpl::IncrementOutstandingRequestsMemory(
+    int count,
+    const ResourceRequestInfoImpl& info) {
+  DCHECK_EQ(1, abs(count));
+
+  // Retrieve the previous value (defaulting to 0 if not found).
+  OustandingRequestsStats stats = GetOutstandingRequestsStats(info);
+
+  // Insert/update the total; delete entries when their count reaches 0.
+  stats.memory_cost += count * info.memory_cost();
+  DCHECK_GE(stats.memory_cost, 0);
+  UpdateOutstandingRequestsStats(info, stats);
+
+  return stats;
+}
 
-  return new_cost;
+ResourceDispatcherHostImpl::OustandingRequestsStats
+ResourceDispatcherHostImpl::IncrementOutstandingRequestsCount(
+    int count,
+    const ResourceRequestInfoImpl& info) {
+  DCHECK_EQ(1, abs(count));
+  num_in_flight_requests_ += count;
+
+  OustandingRequestsStats stats = GetOutstandingRequestsStats(info);
+  stats.num_requests += count;
+  DCHECK_GE(stats.num_requests, 0);
+  UpdateOutstandingRequestsStats(info, stats);
+
+  return stats;
+}
+
+bool ResourceDispatcherHostImpl::HasSufficientResourcesForRequest(
+    const net::URLRequest* request_) {
+  const ResourceRequestInfoImpl* info =
+      ResourceRequestInfoImpl::ForRequest(request_);
+  OustandingRequestsStats stats = IncrementOutstandingRequestsCount(1, *info);
+
+  if (stats.num_requests > max_num_in_flight_requests_per_process_)
+    return false;
+  if (num_in_flight_requests_ > max_num_in_flight_requests_)
+    return false;
+
+  return true;
+}
+
+void ResourceDispatcherHostImpl::FinishedWithResourcesForRequest(
+    const net::URLRequest* request_) {
+  const ResourceRequestInfoImpl* info =
+      ResourceRequestInfoImpl::ForRequest(request_);
+  IncrementOutstandingRequestsCount(-1, *info);
 }
 
 // static
@@ -1523,12 +1579,11 @@ void ResourceDispatcherHostImpl::BeginRequestInternal(
 
   // Add the memory estimate that starting this request will consume.
   info->set_memory_cost(CalculateApproximateMemoryCost(request.get()));
-  int memory_cost = IncrementOutstandingRequestsMemoryCost(info->memory_cost(),
-                                                           info->GetChildID());
 
   // If enqueing/starting this request will exceed our per-process memory
   // bound, abort it right away.
-  if (memory_cost > max_outstanding_requests_cost_per_process_) {
+  OustandingRequestsStats stats = IncrementOutstandingRequestsMemory(1, *info);
+  if (stats.memory_cost > max_outstanding_requests_cost_per_process_) {
     // We call "CancelWithError()" as a way of setting the net::URLRequest's
     // status -- it has no effect beyond this, since the request hasn't started.
     request->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
@@ -1539,8 +1594,7 @@ void ResourceDispatcherHostImpl::BeginRequestInternal(
       NOTREACHED();
     }
 
-    IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
-                                           info->GetChildID());
+    IncrementOutstandingRequestsMemory(-1, *info);
 
     // A ResourceHandler must not outlive its associated URLRequest.
     handler.reset();
@@ -1735,8 +1789,7 @@ void ResourceDispatcherHostImpl::ProcessBlockedRequestsForRoute(
     linked_ptr<ResourceLoader> loader = *loaders_iter;
     ResourceRequestInfoImpl* info = loader->GetRequestInfo();
     if (cancel_requests) {
-      IncrementOutstandingRequestsMemoryCost(-1 * info->memory_cost(),
-                                             info->GetChildID());
+      IncrementOutstandingRequestsMemory(-1, *info);
     } else {
       StartLoading(info, loader);
     }