Bug 1354968 - Avoid concurrent access of mTableRefreshness. draft
authorHenry Chang <hchang@mozilla.com>
Tue, 11 Apr 2017 01:02:42 +0800
changeset 561050 67ad34ca0a97344cc9559811a9109af7b98b5bcc
parent 560921 f40e24f40b4c4556944c762d4764eace261297f5
child 623863 e0d519224ae9f583b5845f7afc66abc908f4de25
push id53608
push userhchang@mozilla.com
push dateWed, 12 Apr 2017 05:29:07 +0000
bugs1354968
milestone55.0a1
Bug 1354968 - Avoid concurrent access of mTableRefreshness. mTableRefreshness, a non-thread-safe object, might be accessed on worker thread and update thread cocurrently. To solve this issue, on update thread we only insert data to mNewTableRefreshness and merge to mTableRefreshness on the worker thread later. MozReview-Commit-ID: 9WgoeYfWVfK
toolkit/components/url-classifier/Classifier.cpp
toolkit/components/url-classifier/Classifier.h
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -616,16 +616,18 @@ void
 Classifier::RemoveUpdateIntermediaries()
 {
   // Remove old LookupCaches.
   for (auto c: mNewLookupCaches) {
     delete c;
   }
   mNewLookupCaches.Clear();
 
+  mNewTableFreshness.Clear();
+
   // Remove the "old" directory. (despite its looking-new name)
   if (NS_FAILED(mUpdatingDirectory->Remove(true))) {
     // If the directory is locked from removal for some reason,
     // 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."));
   }
@@ -680,26 +682,31 @@ Classifier::SwapInNewTablesAndCleanup()
     return rv;
   }
 
   // Step 2. Merge mNewLookupCaches into mLookupCaches. The outdated
   // LookupCaches will be stored in mNewLookupCaches and be cleaned
   // up later.
   MergeNewLookupCaches();
 
-  // Step 3. Re-generate active tables based on on-disk tables.
+  // Step 3. Merge mTableFreshnessForUpdate.
+  for (auto itr = mNewTableFreshness.ConstIter(); !itr.Done(); itr.Next()) {
+    mTableFreshness.Put(itr.Key(), itr.Data());
+  }
+
+  // Step 4. Re-generate active tables based on on-disk tables.
   rv = RegenActiveTables();
   if (NS_FAILED(rv)) {
     LOG(("Failed to re-generate active tables!"));
   }
 
-  // Step 4. Clean up intermediaries for update.
+  // Step 5. Clean up intermediaries for update.
   RemoveUpdateIntermediaries();
 
-  // Step 5. Invalidate cached tableRequest request.
+  // Step 6. Invalidate cached tableRequest request.
   mIsTableRequestResultOutdated = true;
 
   LOG(("Done swap in updated tables."));
 
   return rv;
 }
 
 void Classifier::FlushAndDisableAsyncUpdate()
@@ -1294,17 +1301,17 @@ Classifier::UpdateHashStore(nsTArray<Tab
 #if defined(DEBUG)
   lookupCache->DumpCompletions();
 #endif
   rv = lookupCache->WriteFile();
   NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
 
   int64_t now = (PR_Now() / PR_USEC_PER_SEC);
   LOG(("Successfully updated %s", store.TableName().get()));
-  mTableFreshness.Put(store.TableName(), now);
+  mNewTableFreshness.Put(store.TableName(), now);
 
   return NS_OK;
 }
 
 nsresult
 Classifier::UpdateTableV4(nsTArray<TableUpdate*>* aUpdates,
                           const nsACString& aTable)
 {
@@ -1396,17 +1403,17 @@ Classifier::UpdateTableV4(nsTArray<Table
     LOG(("Write meta data of the last applied update."));
     rv = lookupCache->WriteMetadata(lastAppliedUpdate);
     NS_ENSURE_SUCCESS(rv, NS_ERROR_UC_UPDATE_FAIL_TO_WRITE_DISK);
   }
 
 
   int64_t now = (PR_Now() / PR_USEC_PER_SEC);
   LOG(("Successfully updated %s\n", PromiseFlatCString(aTable).get()));
-  mTableFreshness.Put(aTable, now);
+  mNewTableFreshness.Put(aTable, now);
 
   return NS_OK;
 }
 
 nsresult
 Classifier::UpdateCache(TableUpdate* aUpdate)
 {
   if (!aUpdate) {
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -209,16 +209,17 @@ private:
   nsCOMPtr<nsIFile> mUpdatingDirectory; // For update only.
   nsCOMPtr<nsIFile> mToDeleteDirectory;
   nsCOMPtr<nsICryptoHash> mCryptoHash;
   nsTArray<LookupCache*> mLookupCaches; // For query only.
   nsTArray<nsCString> mActiveTablesCache;
   uint32_t mHashKey;
   // Stores the last time a given table was updated (seconds).
   TableFreshnessMap mTableFreshness;
+  TableFreshnessMap mNewTableFreshness;
 
   // In-memory cache for the result of TableRequest. See
   // nsIUrlClassifierDBService.getTables for the format.
   nsCString mTableRequestResult;
 
   // Whether mTableRequestResult is outdated and needs to
   // be reloaded from disk.
   bool mIsTableRequestResultOutdated;