Bug 1359299 - V4 caches in LookupCache need to be copied around in copy constructor. r?hchang
MozReview-Commit-ID: AjzUUmQKiPW
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -623,16 +623,44 @@ Classifier::RemoveUpdateIntermediaries()
// we will fail here and it doesn't matter until the next
// update. (the next udpate will fail due to the removable
// "safebrowsing-udpating" directory.)
LOG(("Failed to remove updating directory."));
}
}
void
+Classifier::CopyAndInvalidateFullHashCache()
+{
+ MOZ_ASSERT(NS_GetCurrentThread() != mUpdateThread,
+ "CopyAndInvalidateFullHashCache cannot be called on update thread "
+ "since it mutates mLookupCaches which is only safe on "
+ "worker thread.");
+
+ // New lookup caches are built from disk, data likes cache which is
+ // generated online won't exist. We have to manually copy cache from
+ // old LookupCache to new LookupCache.
+ for (auto& newCache: mNewLookupCaches) {
+ for (auto& oldCache: mLookupCaches) {
+ if (oldCache->TableName() == newCache->TableName()) {
+ newCache->CopyFullHashCache(oldCache);
+ break;
+ }
+ }
+ }
+
+ // Clear cache when update.
+ // Invalidate cache entries in CopyAndInvalidateFullHashCache because only
+ // at this point we will have cache data in LookupCache.
+ for (auto& newCache: mNewLookupCaches) {
+ newCache->InvalidateExpiredCacheEntries();
+ }
+}
+
+void
Classifier::MergeNewLookupCaches()
{
MOZ_ASSERT(NS_GetCurrentThread() != mUpdateThread,
"MergeNewLookupCaches cannot be called on update thread "
"since it mutates mLookupCaches which is only safe on "
"worker thread.");
for (auto& newCache: mNewLookupCaches) {
@@ -855,16 +883,20 @@ Classifier::ApplyUpdatesForeground(nsres
const nsACString& aFailedTableName)
{
if (mUpdateInterrupted) {
LOG(("Update is interrupted! Just remove update intermediaries."));
RemoveUpdateIntermediaries();
return NS_OK;
}
if (NS_SUCCEEDED(aBackgroundRv)) {
+ // Copy and Invalidate fullhash cache here because this call requires
+ // mLookupCaches which is only available on work-thread
+ CopyAndInvalidateFullHashCache();
+
return SwapInNewTablesAndCleanup();
}
if (NS_ERROR_OUT_OF_MEMORY != aBackgroundRv) {
ResetTables(Clear_All, nsTArray<nsCString> { nsCString(aFailedTableName) });
}
return aBackgroundRv;
}
@@ -1218,19 +1250,16 @@ Classifier::UpdateHashStore(nsTArray<Tab
// Read the part of the store that is (only) in the cache
LookupCacheV2* lookupCache =
LookupCache::Cast<LookupCacheV2>(GetLookupCacheForUpdate(store.TableName()));
if (!lookupCache) {
return NS_ERROR_UC_UPDATE_TABLE_NOT_FOUND;
}
- // Clear cache when update
- lookupCache->InvalidateExpiredCacheEntries();
-
FallibleTArray<uint32_t> AddPrefixHashes;
rv = lookupCache->GetPrefixes(AddPrefixHashes);
NS_ENSURE_SUCCESS(rv, rv);
rv = store.AugmentAdds(AddPrefixHashes);
NS_ENSURE_SUCCESS(rv, rv);
AddPrefixHashes.Clear();
uint32_t applied = 0;
@@ -1310,21 +1339,16 @@ Classifier::UpdateTableV4(nsTArray<Table
}
LookupCacheV4* lookupCache =
LookupCache::Cast<LookupCacheV4>(GetLookupCacheForUpdate(aTable));
if (!lookupCache) {
return NS_ERROR_UC_UPDATE_TABLE_NOT_FOUND;
}
- // Remove cache entries whose negative cache time is expired when update.
- // We don't check if positive cache time is expired here because we want to
- // keep the eviction rule simple when doing an update.
- lookupCache->InvalidateExpiredCacheEntries();
-
nsresult rv = NS_OK;
// If there are multiple updates for the same table, prefixes1 & prefixes2
// will act as input and output in turn to reduce memory copy overhead.
PrefixStringMap prefixes1, prefixes2;
PrefixStringMap* input = &prefixes1;
PrefixStringMap* output = &prefixes2;
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -135,16 +135,18 @@ private:
nsresult SetupPathNames();
nsresult RecoverBackups();
nsresult CleanToDelete();
nsresult CopyInUseDirForUpdate();
nsresult RegenActiveTables();
void MergeNewLookupCaches(); // Merge mNewLookupCaches into mLookupCaches.
+ void CopyAndInvalidateFullHashCache();
+
// Remove any intermediary for update, including in-memory
// and on-disk data.
void RemoveUpdateIntermediaries();
#ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
already_AddRefed<nsIFile> GetFailedUpdateDirectroy();
nsresult DumpFailedUpdate();
#endif
--- a/toolkit/components/url-classifier/Entries.h
+++ b/toolkit/components/url-classifier/Entries.h
@@ -351,12 +351,22 @@ struct CachedFullHashResponse {
}
}
return true;
}
};
typedef nsClassHashtable<nsUint32HashKey, CachedFullHashResponse> FullHashResponseMap;
+template<class T>
+void
+CopyClassHashTable(const T& aSource, T& aDestination)
+{
+ for (auto iter = aSource.ConstIter(); !iter.Done(); iter.Next()) {
+ auto value = aDestination.LookupOrAdd(iter.Key());
+ *value = *(iter.Data());
+ }
+}
+
} // namespace safebrowsing
} // namespace mozilla
#endif // SBEntries_h__
--- a/toolkit/components/url-classifier/LookupCache.cpp
+++ b/toolkit/components/url-classifier/LookupCache.cpp
@@ -143,17 +143,17 @@ LookupCache::CheckCache(const Completion
{
// Shouldn't call this function if prefix is not in the database.
MOZ_ASSERT(*aHas);
*aConfirmed = false;
uint32_t prefix = aCompletion.ToUint32();
- CachedFullHashResponse* fullHashResponse = mCache.Get(prefix);
+ CachedFullHashResponse* fullHashResponse = mFullHashCache.Get(prefix);
if (!fullHashResponse) {
return NS_OK;
}
int64_t nowSec = PR_Now() / PR_USEC_PER_SEC;
int64_t expiryTimeSec;
FullHashExpiryCache& fullHashes = fullHashResponse->fullHashes;
@@ -173,59 +173,70 @@ LookupCache::CheckCache(const Completion
// Remove fullhash entry from the cache when the negative cache
// is also expired because whether or not the fullhash is cached
// locally, we will need to consult the server next time we
// lookup this hash. We may as well remove it from our cache.
if (fullHashResponse->negativeCacheExpirySec < expiryTimeSec) {
fullHashes.Remove(completion);
if (fullHashes.Count() == 0 &&
fullHashResponse->negativeCacheExpirySec < nowSec) {
- mCache.Remove(prefix);
+ mFullHashCache.Remove(prefix);
}
}
}
return NS_OK;
}
// Check negative cache.
if (fullHashResponse->negativeCacheExpirySec >= nowSec) {
// Url is safe.
LOG(("Found a valid prefix in the negative cache"));
*aHas = false;
} else {
LOG(("Found an expired prefix in the negative cache"));
if (fullHashes.Count() == 0) {
- mCache.Remove(prefix);
+ mFullHashCache.Remove(prefix);
}
}
return NS_OK;
}
// This function remove cache entries whose negative cache time is expired.
// It is possible that a cache entry whose positive cache time is not yet
// expired but still being removed after calling this API. Right now we call
// this on every update.
void
LookupCache::InvalidateExpiredCacheEntries()
{
int64_t nowSec = PR_Now() / PR_USEC_PER_SEC;
- for (auto iter = mCache.Iter(); !iter.Done(); iter.Next()) {
+ for (auto iter = mFullHashCache.Iter(); !iter.Done(); iter.Next()) {
CachedFullHashResponse* response = iter.Data();
if (response->negativeCacheExpirySec < nowSec) {
iter.Remove();
}
}
}
void
+LookupCache::CopyFullHashCache(const LookupCache* aSource)
+{
+ if (!aSource) {
+ return;
+ }
+
+ CopyClassHashTable<FullHashResponseMap>(aSource->mFullHashCache,
+ mFullHashCache);
+}
+
+void
LookupCache::ClearCache()
{
- mCache.Clear();
+ mFullHashCache.Clear();
}
void
LookupCache::ClearAll()
{
ClearCache();
ClearPrefixes();
mPrimed = false;
@@ -234,17 +245,17 @@ LookupCache::ClearAll()
void
LookupCache::GetCacheInfo(nsIUrlClassifierCacheInfo** aCache)
{
MOZ_ASSERT(aCache);
RefPtr<nsUrlClassifierCacheInfo> info = new nsUrlClassifierCacheInfo;
info->table = mTableName;
- for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
+ for (auto iter = mFullHashCache.ConstIter(); !iter.Done(); iter.Next()) {
RefPtr<nsUrlClassifierCacheEntry> entry = new nsUrlClassifierCacheEntry;
// Set prefix of the cache entry.
nsAutoCString prefix(reinterpret_cast<const char*>(&iter.Key()), PREFIX_SIZE);
CStringToHexString(prefix, entry->prefix);
// Set expiry of the cache entry.
CachedFullHashResponse* response = iter.Data();
@@ -500,17 +511,17 @@ nsCString GetFormattedTimeString(int64_t
void
LookupCache::DumpCache()
{
if (!LOG_ENABLED()) {
return;
}
- for (auto iter = mCache.ConstIter(); !iter.Done(); iter.Next()) {
+ for (auto iter = mFullHashCache.ConstIter(); !iter.Done(); iter.Next()) {
CachedFullHashResponse* response = iter.Data();
nsAutoCString prefix;
CStringToHexString(
nsCString(reinterpret_cast<const char*>(&iter.Key()), PREFIX_SIZE), prefix);
LOG(("Cache prefix(%s): %s, Expiry: %s", mTableName.get(), prefix.get(),
GetFormattedTimeString(response->negativeCacheExpirySec).get()));
@@ -635,34 +646,37 @@ LookupCacheV2::GetPrefixes(FallibleTArra
return mPrefixSet->GetPrefixesNative(aAddPrefixes);
}
void
LookupCacheV2::AddGethashResultToCache(AddCompleteArray& aAddCompletes,
MissPrefixArray& aMissPrefixes,
int64_t aExpirySec)
{
- int64_t defaultExpirySec = PR_Now() / PR_USEC_PER_SEC + V2_CACHE_DURATION_SEC;
+ int64_t defaultExpirySec = PR_Now() / PR_USEC_PER_SEC + V2_CACHE_DURATION_SEC;
if (aExpirySec != 0) {
defaultExpirySec = aExpirySec;
}
for (const AddComplete& add : aAddCompletes) {
nsDependentCSubstring fullhash(
reinterpret_cast<const char*>(add.CompleteHash().buf), COMPLETE_SIZE);
- CachedFullHashResponse* response = mCache.LookupOrAdd(add.ToUint32());
+ CachedFullHashResponse* response =
+ mFullHashCache.LookupOrAdd(add.ToUint32());
response->negativeCacheExpirySec = defaultExpirySec;
FullHashExpiryCache& fullHashes = response->fullHashes;
fullHashes.Put(fullhash, defaultExpirySec);
}
for (const Prefix& prefix : aMissPrefixes) {
- CachedFullHashResponse* response = mCache.LookupOrAdd(prefix.ToUint32());
+ CachedFullHashResponse* response =
+ mFullHashCache.LookupOrAdd(prefix.ToUint32());
+
response->negativeCacheExpirySec = defaultExpirySec;
}
}
nsresult
LookupCacheV2::ReadCompletions()
{
HashStore store(mTableName, mProvider, mRootStoreDirectory);
--- a/toolkit/components/url-classifier/LookupCache.h
+++ b/toolkit/components/url-classifier/LookupCache.h
@@ -194,22 +194,25 @@ public:
// Write data stored in lookup cache to disk.
nsresult WriteFile();
bool IsPrimed() const { return mPrimed; };
// Called when update to clear expired entries.
void InvalidateExpiredCacheEntries();
- // Clear completions retrieved from gethash request.
+ // Copy fullhash cache from another LookupCache.
+ void CopyFullHashCache(const LookupCache* aSource);
+
+ // Clear fullhash cache from fullhash/gethash response.
void ClearCache();
// Check if completions can be found in cache.
// Currently this is only used by testcase.
- bool IsInCache(uint32_t key) { return mCache.Get(key); };
+ bool IsInCache(uint32_t key) { return mFullHashCache.Get(key); };
#if DEBUG
void DumpCache();
#endif
void GetCacheInfo(nsIUrlClassifierCacheInfo** aCache);
virtual nsresult Open();
@@ -249,18 +252,18 @@ protected:
nsCString mTableName;
nsCString mProvider;
nsCOMPtr<nsIFile> mRootStoreDirectory;
nsCOMPtr<nsIFile> mStoreDirectory;
// For gtest to inspect private members.
friend class PerProviderDirectoryTestUtils;
- // Cache gethash result.
- FullHashResponseMap mCache;
+ // Cache stores fullhash response(V4)/gethash response(V2)
+ FullHashResponseMap mFullHashCache;
};
class LookupCacheV2 final : public LookupCache
{
public:
explicit LookupCacheV2(const nsACString& aTableName,
const nsACString& aProvider,
nsIFile* aStoreFile)
--- a/toolkit/components/url-classifier/LookupCacheV4.cpp
+++ b/toolkit/components/url-classifier/LookupCacheV4.cpp
@@ -325,20 +325,17 @@ LookupCacheV4::ApplyUpdate(TableUpdateV4
}
return NS_OK;
}
nsresult
LookupCacheV4::AddFullHashResponseToCache(const FullHashResponseMap& aResponseMap)
{
- for (auto iter = aResponseMap.ConstIter(); !iter.Done(); iter.Next()) {
- CachedFullHashResponse* response = mCache.LookupOrAdd(iter.Key());
- *response = *(iter.Data());
- }
+ CopyClassHashTable<FullHashResponseMap>(aResponseMap, mFullHashCache);
return NS_OK;
}
nsresult
LookupCacheV4::InitCrypto(nsCOMPtr<nsICryptoHash>& aCrypto)
{
nsresult rv;
--- a/toolkit/components/url-classifier/tests/unit/test_partial.js
+++ b/toolkit/components/url-classifier/tests/unit/test_partial.js
@@ -553,56 +553,16 @@ function testCachedResultsWithExpire() {
var assertions = {
"urlsDontExist" : ["foo.com/a"],
"completerQueried" : [newCompleter, []]
}
doTest([expireUpdate], assertions);
});
}
-function testCachedResultsUpdate()
-{
- var existUrls = ["foo.com/a"];
- setupCachedResults(existUrls, function() {
- // This is called after setupCachedResults(). Verify that
- // checking the url again does not cause a completer request.
-
- // install a new completer, this one should never be queried.
- var newCompleter = installCompleter('test-phish-simple', [[1, []]], []);
-
- var assertions = {
- "urlsExist" : existUrls,
- "completerQueried" : [newCompleter, []]
- };
-
- var addUrls = ["foobar.org/a"];
-
- var update2 = buildPhishingUpdate(
- [
- { "chunkNum" : 2,
- "urls" : addUrls
- }],
- 4);
-
- checkAssertions(assertions, function () {
- // Apply the update. The cached completes should be gone.
- doStreamUpdate(update2, function() {
- // Now the completer gets queried again.
- var newCompleter2 = installCompleter('test-phish-simple', [[1, existUrls]], []);
- var assertions2 = {
- "tableData" : "test-phish-simple;a:1-2",
- "urlsExist" : existUrls,
- "completerQueried" : [newCompleter2, existUrls]
- };
- checkAssertions(assertions2, runNextTest);
- }, updateError);
- });
- });
-}
-
function testCachedResultsFailure()
{
var existUrls = ["foo.com/a"];
setupCachedResults(existUrls, function() {
// This is called after setupCachedResults(). Verify that
// checking the url again does not cause a completer request.
// install a new completer, this one should never be queried.
@@ -730,16 +690,15 @@ function run_test()
testCompleterFailure,
testMixedSizesSameDomain,
testMixedSizesDifferentDomains,
testInvalidHashSize,
testWrongTable,
testCachedResults,
testCachedResultsWithSub,
testCachedResultsWithExpire,
- testCachedResultsUpdate,
testCachedResultsFailure,
testErrorList,
testErrorListIndependent
]);
}
do_test_pending();