Single-threaded stress test for URLRequestThrottlerManager

to try to reproduce bug seen only in the wild. On my system,
it does not reproduce the bug. Want to check in in case it
reproduces on any of the bots.

Also, add code that will make minidumps of this crash more information-rich, to help track down problem. Add guard buffers around a couple of key things, to help identify memory overwrite. Add simple unit test to make sure the new style of CHECK for one or more null values in the map works correctly.

BUG=71721
TEST=none

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

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

6 files changed, 168 insertions, 16 deletions

diff --git a/net/url_request/url_request_throttler_entry.cc b/net/url_request/url_request_throttler_entry.cc
index bf97feb..6db76c3 100644
--- a/net/url_request/url_request_throttler_entry.cc
+++ b/net/url_request/url_request_throttler_entry.cc
@@ -187,6 +187,10 @@ void URLRequestThrottlerEntry::ReceivedContentWasMalformed() {
   exponential_backoff_release_time_ = CalculateExponentialBackoffReleaseTime();
 }
 
+void URLRequestThrottlerEntry::SetEntryLifetimeMsForTest(int lifetime_ms) {
+  entry_lifetime_ms_ = lifetime_ms;
+}
+
 URLRequestThrottlerEntry::~URLRequestThrottlerEntry() {
 }
 
diff --git a/net/url_request/url_request_throttler_entry.h b/net/url_request/url_request_throttler_entry.h
index 4026237..cefe8d2 100644
--- a/net/url_request/url_request_throttler_entry.h
+++ b/net/url_request/url_request_throttler_entry.h
@@ -80,6 +80,7 @@ class URLRequestThrottlerEntry : public URLRequestThrottlerEntryInterface {
   virtual void UpdateWithResponse(
       const URLRequestThrottlerHeaderInterface* response);
   virtual void ReceivedContentWasMalformed();
+  virtual void SetEntryLifetimeMsForTest(int lifetime_ms);
 
  protected:
   virtual ~URLRequestThrottlerEntry();
@@ -147,7 +148,7 @@ class URLRequestThrottlerEntry : public URLRequestThrottlerEntryInterface {
   const double jitter_factor_;
   const int maximum_backoff_ms_;
   // Set to -1 if the entry never expires.
-  const int entry_lifetime_ms_;
+  int entry_lifetime_ms_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestThrottlerEntry);
 };
diff --git a/net/url_request/url_request_throttler_entry_interface.h b/net/url_request/url_request_throttler_entry_interface.h
index ed3c17f..e3fc5ba 100644
--- a/net/url_request/url_request_throttler_entry_interface.h
+++ b/net/url_request/url_request_throttler_entry_interface.h
@@ -50,6 +50,9 @@ class URLRequestThrottlerEntryInterface
   // the request, i.e. it will count as a failure.
   virtual void ReceivedContentWasMalformed() = 0;
 
+  // For unit testing only.
+  virtual void SetEntryLifetimeMsForTest(int lifetime_ms) = 0;
+
  protected:
   friend class base::RefCountedThreadSafe<URLRequestThrottlerEntryInterface>;
   virtual ~URLRequestThrottlerEntryInterface() {}
diff --git a/net/url_request/url_request_throttler_manager.cc b/net/url_request/url_request_throttler_manager.cc
index 39e1f4e..07e3bb6 100644
--- a/net/url_request/url_request_throttler_manager.cc
+++ b/net/url_request/url_request_throttler_manager.cc
@@ -6,9 +6,31 @@
 
 #include <list>
 
+// TODO(joi): Remove once crbug.com/71721 is fixed.
+#include "base/command_line.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 
+namespace {
+
+// TODO(joi): Remove after crbug.com/71721 is fixed.
+struct IteratorHistory {
+  // Copy of 'this' pointer at time of access; this helps both because
+  // the this pointer is often obfuscated (at least for this particular
+  // stack trace) in fully optimized builds, and possibly to detect
+  // changes in the this pointer during iteration over the map (e.g.
+  // from another thread overwriting memory).
+  net::URLRequestThrottlerManager* self;
+
+  // Copy of URL key.
+  char url[256];
+
+  // Not a refptr, we don't want to change behavior by keeping it alive.
+  net::URLRequestThrottlerEntryInterface* entry;
+};
+
+}  // namespace
+
 namespace net {
 
 ThreadCheckerForRelease::ThreadCheckerForRelease()
@@ -67,9 +89,6 @@ scoped_refptr<URLRequestThrottlerEntryInterface>
 void URLRequestThrottlerManager::OverrideEntryForTests(
     const GURL& url,
     URLRequestThrottlerEntry* entry) {
-  if (entry == NULL)
-    return;
-
   // Normalize the url.
   std::string url_id = GetIdFromUrl(url);
 
@@ -103,6 +122,10 @@ URLRequestThrottlerManager::URLRequestThrottlerManager()
   url_id_replacements_.ClearUsername();
   url_id_replacements_.ClearQuery();
   url_id_replacements_.ClearRef();
+
+  // TODO(joi): Remove after crbug.com/71721 is fixed.
+  base::strlcpy(magic_buffer_1_, "MAGICZZ", arraysize(magic_buffer_1_));
+  base::strlcpy(magic_buffer_2_, "GOOGYZZ", arraysize(magic_buffer_2_));
 }
 
 URLRequestThrottlerManager::~URLRequestThrottlerManager() {
@@ -118,7 +141,8 @@ std::string URLRequestThrottlerManager::GetIdFromUrl(const GURL& url) const {
     return url.possibly_invalid_spec();
 
   GURL id = url.ReplaceComponents(url_id_replacements_);
-  return StringToLowerASCII(id.spec());
+  // TODO(joi): Remove "GOOGY/MONSTA" stuff once crbug.com/71721 is done
+  return StringPrintf("GOOGY%sMONSTA", StringToLowerASCII(id.spec()).c_str());
 }
 
 void URLRequestThrottlerManager::GarbageCollectEntriesIfNecessary() {
@@ -131,17 +155,59 @@ void URLRequestThrottlerManager::GarbageCollectEntriesIfNecessary() {
 }
 
 void URLRequestThrottlerManager::GarbageCollectEntries() {
+  // TODO(joi): Remove these crash report aids once crbug.com/71721
+  // is figured out.
+
+  // Copy the current process command line, in case some labs feature
+  // is in common between the crash dumps.  Note that this is not equivalent
+  // to the command line stored in the PEB of the minidump since it may
+  // have been modified based on the about:labs preferences.
+  std::string command_line_string;

+#if defined(OS_WIN)

+  std::wstring wstr = CommandLine::ForCurrentProcess()->command_line_string();

+  command_line_string = WideToASCII(wstr);

+#else

+  command_line_string =

+      CommandLine::ForCurrentProcess()->command_line_string();

+#endif
+  char command_line_buffer[400] = { 0 };
+  base::strlcpy(command_line_buffer, command_line_string.c_str(),
+                arraysize(command_line_buffer));
+
+  IteratorHistory history[32] = { { 0 } };
+  size_t history_ix = 0;
+  history[history_ix++].self = this;
+
+  int nulls_found = 0;
   UrlEntryMap::iterator i = url_entries_.begin();
-
   while (i != url_entries_.end()) {
-    CHECK(i->second.get());
-    if ((i->second)->IsEntryOutdated()) {
+    if (i->second == NULL) {
+      ++nulls_found;
+    }
+
+    // Keep a log of the first 31 items accessed after the first
+    // NULL encountered (hypothesis is there are multiple NULLs,
+    // and we may learn more about pattern of memory overwrite).
+    // We also log when we access the first entry, to get an original
+    // value for our this pointer.
+    if (nulls_found > 0 && history_ix < arraysize(history)) {
+      history[history_ix].self = this;
+      base::strlcpy(history[history_ix].url, i->first.c_str(),
+                    arraysize(history[history_ix].url));
+      history[history_ix].entry = i->second.get();
+      ++history_ix;
+    }
+
+    // TODO(joi): Remove first i->second check once no more bug.
+    if ((i->second) && (i->second)->IsEntryOutdated()) {
       url_entries_.erase(i++);
     } else {
       ++i;
     }
   }
 
+  CHECK(nulls_found == 0);
+
   // In case something broke we want to make sure not to grow indefinitely.
   while (url_entries_.size() > kMaximumNumberOfEntries) {
     url_entries_.erase(url_entries_.begin());
diff --git a/net/url_request/url_request_throttler_manager.h b/net/url_request/url_request_throttler_manager.h
index 1862cd0..f037fe7 100644
--- a/net/url_request/url_request_throttler_manager.h
+++ b/net/url_request/url_request_throttler_manager.h
@@ -118,10 +118,20 @@ class URLRequestThrottlerManager {
   // Number of requests that will be made between garbage collection.
   static const unsigned int kRequestsBetweenCollecting;
 
+  // Constructor copies the string "MAGICZZ\0" into this buffer; using it
+  // to try to detect memory overwrites affecting url_entries_ in the wild.
+  // TODO(joi): Remove once crbug.com/71721 is figured out.
+  char magic_buffer_1_[8];
+
   // Map that contains a list of URL ID and their matching
   // URLRequestThrottlerEntry.
   UrlEntryMap url_entries_;
 
+  // Constructor copies the string "GOOGYZZ\0" into this buffer; using it
+  // to try to detect memory overwrites affecting url_entries_ in the wild.
+  // TODO(joi): Remove once crbug.com/71721 is figured out.
+  char magic_buffer_2_[8];
+
   // This keeps track of how many requests have been made. Used with
   // GarbageCollectEntries.
   unsigned int requests_since_last_gc_;
diff --git a/net/url_request/url_request_throttler_unittest.cc b/net/url_request/url_request_throttler_unittest.cc
index 0683f91..a281564 100644
--- a/net/url_request/url_request_throttler_unittest.cc
+++ b/net/url_request/url_request_throttler_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "base/pickle.h"
 #include "base/scoped_ptr.h"
+#include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
 #include "base/time.h"
 #include "net/base/test_completion_callback.h"
@@ -293,21 +294,27 @@ TEST(URLRequestThrottlerManager, IsUrlStandardised) {
   MockURLRequestThrottlerManager manager;
   GurlAndString test_values[] = {
       GurlAndString(GURL("http://www.example.com"),
-                    std::string("http://www.example.com/"), __LINE__),
+                    std::string("GOOGYhttp://www.example.com/MONSTA"),
+                    __LINE__),
       GurlAndString(GURL("http://www.Example.com"),
-                    std::string("http://www.example.com/"), __LINE__),
+                    std::string("GOOGYhttp://www.example.com/MONSTA"),
+                    __LINE__),
       GurlAndString(GURL("http://www.ex4mple.com/Pr4c71c41"),
-                    std::string("http://www.ex4mple.com/pr4c71c41"), __LINE__),
-      GurlAndString(GURL("http://www.example.com/0/token/false"),
-                    std::string("http://www.example.com/0/token/false"),
+                    std::string("GOOGYhttp://www.ex4mple.com/pr4c71c41MONSTA"),
                     __LINE__),
+      GurlAndString(
+          GURL("http://www.example.com/0/token/false"),
+          std::string("GOOGYhttp://www.example.com/0/token/falseMONSTA"),
+          __LINE__),
       GurlAndString(GURL("http://www.example.com/index.php?code=javascript"),
-                    std::string("http://www.example.com/index.php"), __LINE__),
+                    std::string("GOOGYhttp://www.example.com/index.phpMONSTA"),
+                    __LINE__),
       GurlAndString(GURL("http://www.example.com/index.php?code=1#superEntry"),
-                    std::string("http://www.example.com/index.php"),
+                    std::string("GOOGYhttp://www.example.com/index.phpMONSTA"),
                     __LINE__),
       GurlAndString(GURL("http://www.example.com:1234/"),
-                    std::string("http://www.example.com:1234/"), __LINE__)};
+                    std::string("GOOGYhttp://www.example.com:1234/MONSTA"),
+                    __LINE__)};
 
   for (unsigned int i = 0; i < arraysize(test_values); ++i) {
     std::string temp = manager.DoGetUrlIdFromUrl(test_values[i].url);
@@ -344,3 +351,64 @@ TEST(URLRequestThrottlerManager, IsHostBeingRegistered) {
 
   EXPECT_EQ(3, manager.GetNumberOfEntries());
 }
+
+class DummyResponseHeaders : public net::URLRequestThrottlerHeaderInterface {
+ public:
+  DummyResponseHeaders(int response_code) : response_code_(response_code) {}
+
+  std::string GetNormalizedValue(const std::string& key) const {
+    return "";
+  }
+
+  // Returns the HTTP response code associated with the request.
+  int GetResponseCode() const {
+    return response_code_;
+  }
+
+ private:
+  int response_code_;
+};
+
+TEST(URLRequestThrottlerManager, StressTest) {
+  MockURLRequestThrottlerManager manager;
+
+  for (int i = 0; i < 10000; ++i) {
+    // Make some entries duplicates or triplicates.
+    int entry_number = i + 2;
+    if (i % 17 == 0) {
+      entry_number = i - 1;
+    } else if ((i - 1) % 17 == 0) {
+      entry_number = i - 2;
+    } else if (i % 13 == 0) {
+      entry_number = i - 1;
+    }
+
+    scoped_refptr<net::URLRequestThrottlerEntryInterface> entry =
+        manager.RegisterRequestUrl(GURL(StringPrintf(
+            "http://bingourl.com/%d", entry_number)));
+    entry->IsDuringExponentialBackoff();
+    entry->GetExponentialBackoffReleaseTime();
+
+    DummyResponseHeaders headers(i % 7 == 0 ? 500 : 200);
+    entry->UpdateWithResponse(&headers);
+    entry->SetEntryLifetimeMsForTest(1);
+
+    entry->IsDuringExponentialBackoff();
+    entry->GetExponentialBackoffReleaseTime();
+    entry->ReserveSendingTimeForNextRequest(base::TimeTicks::Now());
+
+    if (i % 11 == 0) {
+      manager.DoGarbageCollectEntries();
+    }
+  }
+}
+
+#if defined(GTEST_HAS_DEATH_TEST)
+TEST(URLRequestThrottlerManager, NullHandlingTest) {
+  MockURLRequestThrottlerManager manager;
+  manager.OverrideEntryForTests(GURL("http://www.foo.com/"), NULL);
+  ASSERT_DEATH({
+      manager.DoGarbageCollectEntries();
+  }, "");
+}
+#endif  // defined(GTEST_HAS_DEATH_TEST)