Bug 1354968 - Avoid concurrent access of mTableRefreshness. draft
authorHenry Chang <hchang@mozilla.com>
Tue, 11 Apr 2017 01:02:42 +0800
changeset 560327 9c5cfc6f7399ec13e58577dbcf6664c9e99ba5cd
parent 559365 45692c884fdd5136a64fb2f8a61a0c8183b69331
child 623659 b7a5675ea9dbcf93af97174242719a734a341078
push id53378
push userhchang@mozilla.com
push dateTue, 11 Apr 2017 08:47:54 +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 mTableRefreshnessForUpdate 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,19 @@ void
 Classifier::RemoveUpdateIntermediaries()
 {
   // Remove old LookupCaches.
   for (auto c: mNewLookupCaches) {
     delete c;
   }
   mNewLookupCaches.Clear();
 
+  // Reset mTableFreshnessForUpdate.
+  mTableFreshnessForUpdate.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 +683,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 = mTableFreshnessForUpdate.ConstIter(); !itr.Done(); itr.Next()) {
+    SetLastUpdateTime(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()
@@ -1274,17 +1282,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);
+  mTableFreshnessForUpdate.Put(store.TableName(), now);
 
   return NS_OK;
 }
 
 nsresult
 Classifier::UpdateTableV4(nsTArray<TableUpdate*>* aUpdates,
                           const nsACString& aTable)
 {
@@ -1371,17 +1379,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);
+  mTableFreshnessForUpdate.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 mTableFreshnessForUpdate;
 
   // 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;