Bug 1342397 - Properly reset stuff when failing to create LookupCache for update.
MozReview-Commit-ID: 1ecI2lFIj2U
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -708,17 +708,21 @@ Classifier::ApplyUpdates(nsTArray<TableU
// LookupCache::Open() and HashStore::Open() since those operations
// might fail and cause the database to be removed. It'll be fine
// until we move "apply update" code off the worker thread.
rv = CopyInUseDirForUpdate(); // i.e. mUpdatingDirectory will be setup.
if (NS_FAILED(rv)) {
LOG(("Failed to copy in-use directory for update."));
return rv;
}
- CopyInUseLookupCacheForUpdate(); // i.e. mNewLookupCaches will be setup.
+ rv = CopyInUseLookupCacheForUpdate(); // i.e. mNewLookupCaches will be setup.
+ if (NS_FAILED(rv)) {
+ LOG(("Failed to create lookup caches from copied files."));
+ return rv;
+ }
}
LOG(("Applying %" PRIuSIZE " table updates.", aUpdates->Length()));
for (uint32_t i = 0; i < aUpdates->Length(); i++) {
// Previous UpdateHashStore() may have consumed this update..
if ((*aUpdates)[i]) {
// Run all updates for one table
@@ -993,29 +997,33 @@ Classifier::CopyInUseDirForUpdate()
// We moved some things to new places, so move the handles around, too.
rv = SetupPathNames();
NS_ENSURE_SUCCESS(rv, rv);
return NS_OK;
}
-void
+nsresult
Classifier::CopyInUseLookupCacheForUpdate()
{
MOZ_ASSERT(mNewLookupCaches.IsEmpty(), "Update intermediaries is forgotten to "
"be removed in the previous update.");
// TODO: Bug 1341183 - Implement deep copy function or copy ctor.
// For now we just re-open every table we have opened.
// Another option is to NOT pre-open these LookupCaches and
// do a "merge" instead of "swap" after update.
for (auto c: mLookupCaches) {
- Unused << GetLookupCacheForUpdate(c->TableName());
+ if (!GetLookupCacheForUpdate(c->TableName())) {
+ return NS_ERROR_FAILURE;
+ }
}
+
+ return NS_OK;
}
nsresult
Classifier::RecoverBackups()
{
bool backupExists;
nsresult rv = mBackupDirectory->Exists(&backupExists);
NS_ENSURE_SUCCESS(rv, rv);
@@ -1309,57 +1317,65 @@ Classifier::UpdateCache(TableUpdate* aUp
#endif
return NS_OK;
}
LookupCache *
Classifier::GetLookupCache(const nsACString& aTable, bool aForUpdate)
{
- if (aForUpdate) {
- return GetLookupCacheFrom(aTable, mNewLookupCaches, mUpdatingDirectory);
- }
- return GetLookupCacheFrom(aTable, mLookupCaches, mRootStoreDirectory);
-}
+ nsTArray<LookupCache*>& lookupCaches = aForUpdate ? mNewLookupCaches
+ : mLookupCaches;
+ auto& rootStoreDirectory = aForUpdate ? mUpdatingDirectory
+ : mRootStoreDirectory;
-LookupCache *
-Classifier::GetLookupCacheFrom(const nsACString& aTable,
- nsTArray<LookupCache*>& aLookupCaches,
- nsIFile* aRootStoreDirectory)
-{
- for (uint32_t i = 0; i < aLookupCaches.Length(); i++) {
- if (aLookupCaches[i]->TableName().Equals(aTable)) {
- return aLookupCaches[i];
+ for (auto c: lookupCaches) {
+ if (c->TableName().Equals(aTable)) {
+ return c;
}
}
// TODO : Bug 1302600, It would be better if we have a more general non-main
// thread method to convert table name to protocol version. Currently
// we can only know this by checking if the table name ends with '-proto'.
UniquePtr<LookupCache> cache;
nsCString provider = GetProvider(aTable);
if (StringEndsWith(aTable, NS_LITERAL_CSTRING("-proto"))) {
- cache = MakeUnique<LookupCacheV4>(aTable, provider, aRootStoreDirectory);
+ cache = MakeUnique<LookupCacheV4>(aTable, provider, rootStoreDirectory);
} else {
- cache = MakeUnique<LookupCacheV2>(aTable, provider, aRootStoreDirectory);
+ cache = MakeUnique<LookupCacheV2>(aTable, provider, rootStoreDirectory);
}
nsresult rv = cache->Init();
if (NS_FAILED(rv)) {
return nullptr;
}
rv = cache->Open();
- if (NS_FAILED(rv)) {
- if (rv == NS_ERROR_FILE_CORRUPTED) {
- Reset();
- }
+ if (NS_SUCCEEDED(rv)) {
+ lookupCaches.AppendElement(cache.get());
+ return cache.release();
+ }
+
+ // At this point we failed to open LookupCache.
+ //
+ // GetLookupCache for update and for other usage will run on update thread
+ // and worker thread respectively (Bug 1339760). Removing stuff only in
+ // their own realms potentially increases the concurrency.
+
+ if (aForUpdate) {
+ // Remove intermediaries no matter if it's due to file corruption or not.
+ RemoveUpdateIntermediaries();
return nullptr;
}
- aLookupCaches.AppendElement(cache.get());
- return cache.release();
+
+ // Non-update case.
+ if (rv == NS_ERROR_FILE_CORRUPTED) {
+ Reset(); // Not including the update intermediaries.
+ }
+ return nullptr;
}
nsresult
Classifier::ReadNoiseEntries(const Prefix& aPrefix,
const nsACString& aTableName,
uint32_t aCount,
PrefixArray* aNoiseEntries)
{
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -27,17 +27,17 @@ public:
typedef nsClassHashtable<nsCStringHashKey, nsCString> ProviderDictType;
public:
Classifier();
~Classifier();
nsresult Open(nsIFile& aCacheDirectory);
void Close();
- void Reset();
+ void Reset(); // Not including any intermediary for update.
/**
* Clear data for specific tables.
* If ClearType is Clear_Cache, this function will only clear cache in lookup
* cache, otherwise, it will clear data in lookup cache and data stored on disk.
*/
enum ClearType {
Clear_Cache,
@@ -118,17 +118,17 @@ private:
void DeleteTables(nsIFile* aDirectory, const nsTArray<nsCString>& aTables);
void AbortUpdateAndReset(const nsCString& aTable);
nsresult CreateStoreDirectory();
nsresult SetupPathNames();
nsresult RecoverBackups();
nsresult CleanToDelete();
nsresult CopyInUseDirForUpdate();
- void CopyInUseLookupCacheForUpdate();
+ nsresult CopyInUseLookupCacheForUpdate();
nsresult RegenActiveTables();
// Remove any intermediary for update, including in-memory
// and on-disk data.
void RemoveUpdateIntermediaries();