protect multi-threaded access to data

possible fix for brave/brave-browser#2970
brave · Feb 1, 2019 · 6150be9 · 6150be9
1 parent 07fd3df
commit 6150be9
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 15 deletions.
diff --git a/components/brave_shields/browser/https_everywhere_recently_used_cache.h b/components/brave_shields/browser/https_everywhere_recently_used_cache.h
@@ -6,10 +6,12 @@
 #ifndef BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_HTTPS_EVERYWHERE_RECENTLY_USED_CACHE_H_
 #define BRAVE_COMPONENTS_BRAVE_SHIELDS_BROWSER_HTTPS_EVERYWHERE_RECENTLY_USED_CACHE_H_
 
-#include <unordered_map>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
+#include "base/synchronization/lock.h"
+
 template <class T> class RingBuffer {
  public:
   explicit RingBuffer(int fixedSize) : count(fixedSize), data(count) {}
@@ -42,21 +44,41 @@ template <class T> class HTTPSERecentlyUsedCache {
   explicit HTTPSERecentlyUsedCache(unsigned int size = 100) : keysByAge(size) {}
 
   void add(const std::string& key, const T& value) {
-    std::string old = keysByAge.oldest();
-    if (!old.empty()) {
-      keysByAge.data.erase(old);
+    base::AutoLock create(lock_);
+
+    data_[key] = value;
+    // https://github.com/brave/brave-browser/issues/3193
+    // std::string old = keysByAge.oldest();
+    // if (!old.empty()) {
+    //   keysByAge.data.erase(old);
+    // }
+    // keysByAge[key] = value;
+  }
+
+  bool get(const std::string& key, T* value) {
+    base::AutoLock create(lock_);
+
+    auto search = data_.find(key);
+    if (search != data_.end()) {
+      *value = search->second;
+      return true;
     }
-    keysByAge[key] = value;
+    return false;
   }
 
-  void clear() {
-    data.clear();
-    keysByAge.clear();
+  void remove(const std::string& key) {
+    data_.erase(key);
   }
 
-  std::unordered_map<std::string, T> data;
+  void clear() {
+    data_.clear();
+    // https://github.com/brave/brave-browser/issues/3193
+    // keysByAge.clear();
+  }
 
  private:
+  std::unordered_map<std::string, T> data_;
+  base::Lock lock_;
   RingBuffer<T> keysByAge;
 };
 

diff --git a/components/brave_shields/browser/https_everywhere_service.cc b/components/brave_shields/browser/https_everywhere_service.cc
@@ -163,9 +163,8 @@ bool HTTPSEverywhereService::GetHTTPSURL(
     return false;
   }
 
-  if (recently_used_cache_.data.count(url->spec()) > 0) {
+  if (recently_used_cache_.get(url->spec(), &new_url)) {
     AddHTTPSEUrlToRedirectList(request_identifier);
-    new_url = recently_used_cache_.data[url->spec()];
     return true;
   }
 
@@ -183,13 +182,13 @@ bool HTTPSEverywhereService::GetHTTPSURL(
     if (!value.empty()) {
       new_url = ApplyHTTPSRule(candidate_url.spec(), value);
       if (0 != new_url.length()) {
-        recently_used_cache_.data[candidate_url.spec()] = new_url;
+        recently_used_cache_.add(candidate_url.spec(), new_url);
         AddHTTPSEUrlToRedirectList(request_identifier);
         return true;
       }
     }
   }
-  recently_used_cache_.data[candidate_url.spec()].clear();
+  recently_used_cache_.remove(candidate_url.spec());
   return false;
 }
 
@@ -207,9 +206,8 @@ bool HTTPSEverywhereService::GetHTTPSURLFromCacheOnly(
     return false;
   }
 
-  if (recently_used_cache_.data.count(url->spec()) > 0) {
+  if (recently_used_cache_.get(url->spec(), &cached_url)) {
     AddHTTPSEUrlToRedirectList(request_identifier);
-    cached_url = recently_used_cache_.data[url->spec()];
     return true;
   }
   return false;