Bug 1339760 - Split update process to background/foreground and run background on update thread **synchronously**.
MozReview-Commit-ID: J0phPC1nWsf
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -350,30 +350,16 @@ Classifier::DeleteTables(nsIFile* aDirec
leafName.get()).get());
}
}
}
NS_ENSURE_SUCCESS_VOID(rv);
}
void
-Classifier::AbortUpdateAndReset(const nsCString& aTable)
-{
- // We don't need to reset while shutting down. It will only slow us down.
- if (nsUrlClassifierDBService::ShutdownHasStarted()) {
- return;
- }
-
- LOG(("Abort updating table %s.", aTable.get()));
-
- // ResetTables will clear both in-memory & on-disk data.
- ResetTables(Clear_All, nsTArray<nsCString> { aTable });
-}
-
-void
Classifier::TableRequest(nsACString& aResult)
{
MOZ_ASSERT(!NS_IsMainThread(),
"TableRequest must be called on the classifier worker thread.");
// This function and all disk I/O are guaranteed to occur
// on the same thread so we don't need to add a lock around.
if (!mIsTableRequestResultOutdated) {
@@ -689,16 +675,26 @@ Classifier::SwapInNewTablesAndCleanup()
LOG(("Done swap in updated tables."));
return rv;
}
nsresult
Classifier::ApplyUpdates(nsTArray<TableUpdate*>* aUpdates)
{
+ nsCString failedTableName;
+
+ nsresult bgRv = ApplyUpdatesBackground(aUpdates, failedTableName);
+ return ApplyUpdatesForeground(bgRv, failedTableName);
+}
+
+nsresult
+Classifier::ApplyUpdatesBackground(nsTArray<TableUpdate*>* aUpdates,
+ nsACString& aFailedTableName)
+{
// Will run on the update thread after Bug 1339760.
// No lock is required except for CopyInUseDirForUpdate() since
// the table lookup may lead to database removal.
if (!aUpdates || aUpdates->Length() == 0) {
return NS_OK;
}
@@ -750,46 +746,53 @@ Classifier::ApplyUpdates(nsTArray<TableU
// Will update the mirrored in-memory and on-disk databases.
if (TableUpdate::Cast<TableUpdateV2>((*aUpdates)[i])) {
rv = UpdateHashStore(aUpdates, updateTable);
} else {
rv = UpdateTableV4(aUpdates, updateTable);
}
if (NS_FAILED(rv)) {
+ aFailedTableName = updateTable;
if (rv != NS_ERROR_OUT_OF_MEMORY) {
#ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
DumpFailedUpdate();
#endif
- // TODO: Bug 1339050 - Should be run on worker thread.
- AbortUpdateAndReset(updateTable);
}
RemoveUpdateIntermediaries();
return rv;
}
}
}
} // End of scopedUpdatesClearer scope.
- rv = SwapInNewTablesAndCleanup();
- if (NS_FAILED(rv)) {
- LOG(("Failed to swap in new tables."));
- }
-
if (LOG_ENABLED()) {
PRIntervalTime clockEnd = PR_IntervalNow();
LOG(("update took %dms\n",
PR_IntervalToMilliseconds(clockEnd - clockStart)));
}
return rv;
}
nsresult
+Classifier::ApplyUpdatesForeground(nsresult aBackgroundRv,
+ const nsACString& aFailedTableName)
+{
+ if (NS_SUCCEEDED(aBackgroundRv)) {
+ return SwapInNewTablesAndCleanup();
+ }
+ if (NS_ERROR_OUT_OF_MEMORY != aBackgroundRv) {
+ ResetTables(Clear_All, nsTArray<nsCString> { nsCString(aFailedTableName) });
+ }
+ return aBackgroundRv;
+}
+
+nsresult
Classifier::ApplyFullHashes(nsTArray<TableUpdate*>* aUpdates)
{
LOG(("Applying %" PRIuSIZE " table gethashes.", aUpdates->Length()));
ScopedUpdatesClearer scopedUpdatesClearer(aUpdates);
for (uint32_t i = 0; i < aUpdates->Length(); i++) {
TableUpdate *update = aUpdates->ElementAt(i);
--- a/toolkit/components/url-classifier/Classifier.h
+++ b/toolkit/components/url-classifier/Classifier.h
@@ -66,16 +66,38 @@ public:
/**
* Apply the table updates in the array. Takes ownership of
* the updates in the array and clears it. Wacky!
*/
nsresult ApplyUpdates(nsTArray<TableUpdate*>* aUpdates);
/**
+ * The "background" part of ApplyUpdates. Once the background update
+ * is called, the foreground update has to be called along with the
+ * background result no matter whether the background update is
+ * successful or not.
+ */
+ nsresult ApplyUpdatesBackground(nsTArray<TableUpdate*>* aUpdates,
+ nsACString& aFailedTableName);
+
+ /**
+ * The "foreground" part of ApplyUpdates. The in-use data (in-memory and
+ * on-disk) will be touched so this MUST be mutually exclusive to other
+ * member functions.
+ *
+ * If |aBackgroundRv| is successful, the return value is the result of
+ * bringing stuff to the foreground. Otherwise, the foreground table may
+ * be reset according to the background update failed reason and
+ * |aBackgroundRv| will be returned to forward the background update result.
+ */
+ nsresult ApplyUpdatesForeground(nsresult aBackgroundRv,
+ const nsACString& aFailedTableName);
+
+ /**
* Apply full hashes retrived from gethash to cache.
*/
nsresult ApplyFullHashes(nsTArray<TableUpdate*>* aUpdates);
void SetLastUpdateTime(const nsACString& aTableName, uint64_t updateTime);
int64_t GetLastUpdateTime(const nsACString& aTableName);
nsresult CacheCompletions(const CacheResultArray& aResults);
uint32_t GetHashKey(void) { return mHashKey; }
@@ -114,17 +136,16 @@ public:
nsresult SwapInNewTablesAndCleanup();
LookupCache *GetLookupCache(const nsACString& aTable,
bool aForUpdate = false);
private:
void DropStores();
void DeleteTables(nsIFile* aDirectory, const nsTArray<nsCString>& aTables);
- void AbortUpdateAndReset(const nsCString& aTable);
nsresult CreateStoreDirectory();
nsresult SetupPathNames();
nsresult RecoverBackups();
nsresult CleanToDelete();
nsresult CopyInUseDirForUpdate();
nsresult CopyInUseLookupCacheForUpdate();
nsresult RegenActiveTables();
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -130,16 +130,19 @@ LazyLogModule gUrlClassifierDbServiceLog
class nsUrlClassifierDBServiceWorker;
// Singleton instance.
static nsUrlClassifierDBService* sUrlClassifierDBService;
nsIThread* nsUrlClassifierDBService::gDbBackgroundThread = nullptr;
+// For update only.
+static nsIThread* gDbUpdateThread = nullptr;
+
// Once we've committed to shutting down, don't do work in the background
// thread.
static bool gShuttingDownThread = false;
static mozilla::Atomic<int32_t> gFreshnessGuarantee(CONFIRM_AGE_DEFAULT_SEC);
NS_IMPL_ISUPPORTS(nsUrlClassifierDBServiceWorker,
nsIUrlClassifierDBService)
@@ -607,25 +610,83 @@ nsUrlClassifierDBServiceWorker::FinishSt
NS_IMETHODIMP
nsUrlClassifierDBServiceWorker::FinishUpdate()
{
if (gShuttingDownThread) {
return NS_ERROR_NOT_INITIALIZED;
}
+ MOZ_ASSERT(!mProtocolParser, "Should have been nulled out in FinishStream() "
+ "or never created.");
+
NS_ENSURE_STATE(mUpdateObserver);
- if (NS_SUCCEEDED(mUpdateStatus)) {
- mUpdateStatus = ApplyUpdate();
- } else {
+ if (NS_FAILED(mUpdateStatus)) {
LOG(("nsUrlClassifierDBServiceWorker::FinishUpdate() Not running "
"ApplyUpdate() since the update has already failed."));
+ return NotifyUpdateObserver(mUpdateStatus);
}
+ if (mTableUpdates.IsEmpty()) {
+ LOG(("Nothing to update. Just notify update observer."));
+ return NotifyUpdateObserver(NS_OK);
+ }
+
+ RefPtr<nsUrlClassifierDBServiceWorker> self = this;
+
+ // TODO: Asynchronously dispatch |ApplyUpdatesBackground| to update thread.
+ // See Bug 1339050. For now we *synchronously* run
+ // ApplyUpdatesForeground() after ApplyUpdatesBackground().
+ nsresult backgroundRv;
+ nsCString failedTableName;
+ nsCOMPtr<nsIRunnable> r =
+ NS_NewRunnableFunction([self, &backgroundRv, &failedTableName] () -> void {
+ backgroundRv = self->ApplyUpdatesBackground(failedTableName);
+ });
+
+ LOG(("Bulding new tables..."));
+ mozilla::SyncRunnable::DispatchToThread(gDbUpdateThread, r);
+ LOG(("Done bulding new tables. Try to commit them."));
+
+ return ApplyUpdatesForeground(backgroundRv, failedTableName);
+}
+
+nsresult
+nsUrlClassifierDBServiceWorker::ApplyUpdatesForeground(nsresult aBackgroundRv,
+ const nsACString& aFailedTableName)
+{
+ if (gShuttingDownThread) {
+ return NS_ERROR_NOT_INITIALIZED;
+ }
+
+ MOZ_ASSERT(NS_GetCurrentThread() == nsUrlClassifierDBService::BackgroundThread(),
+ "nsUrlClassifierDBServiceWorker::ApplyUpdatesForeground MUST "
+ "run on worker thread!!");
+
+ nsresult rv =
+ mClassifier->ApplyUpdatesForeground(aBackgroundRv, aFailedTableName);
+
+ return NotifyUpdateObserver(rv);
+}
+
+nsresult
+nsUrlClassifierDBServiceWorker::NotifyUpdateObserver(nsresult aUpdateStatus)
+{
+ // We've either
+ // 1) failed starting a download stream
+ // 2) succeeded in starting a download stream but failed to obtain
+ // table updates
+ // 3) succeeded in obtaining table updates but failed to build new
+ // tables.
+ // 4) succeeded in building new tables but failed to take them.
+ // 5) succeeded in taking new tables.
+
+ mUpdateStatus = aUpdateStatus;
+
nsCOMPtr<nsIUrlClassifierUtils> urlUtil =
do_GetService(NS_URLCLASSIFIERUTILS_CONTRACTID);
nsCString provider;
// Assume that all the tables in update should have the same provider.
urlUtil->GetTelemetryProvider(mUpdateTables.SafeElementAt(0, EmptyCString()), provider);
nsresult updateStatus = mUpdateStatus;
@@ -666,20 +727,34 @@ nsUrlClassifierDBServiceWorker::FinishUp
mClassifier->ResetTables(Classifier::Clear_Cache, mUpdateTables);
}
mUpdateObserver = nullptr;
return NS_OK;
}
nsresult
-nsUrlClassifierDBServiceWorker::ApplyUpdate()
+nsUrlClassifierDBServiceWorker::ApplyUpdatesBackground(nsACString& aFailedTableName)
{
- LOG(("nsUrlClassifierDBServiceWorker::ApplyUpdate()"));
- nsresult rv = mClassifier->ApplyUpdates(&mTableUpdates);
+ // ********* Please be very careful while adding tasks. *********
+ // This function will run on the update thread (whereas most of the other
+ // functions will run on the worker thread) so any task that might mutate
+ // the in-use data is forbidden.
+
+ if (gShuttingDownThread) {
+ return NS_ERROR_NOT_INITIALIZED;
+ }
+
+ MOZ_ASSERT(NS_GetCurrentThread() == gDbUpdateThread,
+ "nsUrlClassifierDBServiceWorker::BuildNewTables MUST "
+ "run on update thread!!");
+
+ LOG(("nsUrlClassifierDBServiceWorker::BuildNewTables()"));
+ nsresult rv = mClassifier->ApplyUpdatesBackground(&mTableUpdates,
+ aFailedTableName);
#ifdef MOZ_SAFEBROWSING_DUMP_FAILED_UPDATES
if (NS_FAILED(rv) && NS_ERROR_OUT_OF_MEMORY != rv) {
mClassifier->DumpRawTableUpdates(mRawTableUpdates);
}
// Invalidate the raw table updates.
mRawTableUpdates = EmptyCString();
#endif
@@ -1509,16 +1584,23 @@ nsUrlClassifierDBService::Init()
}
}
// Start the background thread.
rv = NS_NewNamedThread("URL Classifier", &gDbBackgroundThread);
if (NS_FAILED(rv))
return rv;
+ // Start the update thread.
+ rv = NS_NewNamedThread(NS_LITERAL_CSTRING("SafeBrowsing DB Update"),
+ &gDbUpdateThread);
+ if (NS_FAILED(rv)) {
+ return rv;
+ }
+
mWorker = new nsUrlClassifierDBServiceWorker();
if (!mWorker)
return NS_ERROR_OUT_OF_MEMORY;
rv = mWorker->Init(gethashNoise, cacheDir);
if (NS_FAILED(rv)) {
mWorker = nullptr;
return rv;
@@ -2088,40 +2170,50 @@ nsUrlClassifierDBService::Observe(nsISup
Shutdown();
} else if (!strcmp(aTopic, "profile-before-change")) {
// Unit test does not receive "quit-application",
// need call shutdown in this case
Shutdown();
LOG(("joining background thread"));
mWorkerProxy = nullptr;
- if (!gDbBackgroundThread) {
- return NS_OK;
+ if (gDbBackgroundThread) {
+ nsIThread *backgroundThread = gDbBackgroundThread;
+ // Nulling out gDbBackgroundThread MUST be done before Shutdown()
+ // since Shutdown() will lead to processing pending events.
+ gDbBackgroundThread = nullptr;
+ backgroundThread->Shutdown();
+ NS_RELEASE(backgroundThread);
}
- nsIThread *backgroundThread = gDbBackgroundThread;
- gDbBackgroundThread = nullptr;
- backgroundThread->Shutdown();
- NS_RELEASE(backgroundThread);
+ if (gDbUpdateThread) {
+ nsIThread *updateThread = gDbUpdateThread;
+ gDbUpdateThread = nullptr;
+ updateThread->Shutdown();
+ NS_RELEASE(updateThread);
+ }
+
+ return NS_OK;
} else {
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
// Join the background thread if it exists.
nsresult
nsUrlClassifierDBService::Shutdown()
{
LOG(("shutting down db service\n"));
MOZ_ASSERT(XRE_IsParentProcess());
- if (!gDbBackgroundThread || gShuttingDownThread)
+ if ((!gDbBackgroundThread && !gDbUpdateThread) || gShuttingDownThread) {
return NS_OK;
+ }
gShuttingDownThread = true;
Telemetry::AutoTimer<Telemetry::URLCLASSIFIER_SHUTDOWN_TIME> timer;
mCompleters.Clear();
nsCOMPtr<nsIPrefBranch> prefs = do_GetService(NS_PREFSERVICE_CONTRACTID);
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -184,18 +184,24 @@ public:
private:
// No subclassing
~nsUrlClassifierDBServiceWorker();
// Disallow copy constructor
nsUrlClassifierDBServiceWorker(nsUrlClassifierDBServiceWorker&);
- // Applies the current transaction and resets the update/working times.
- nsresult ApplyUpdate();
+ // Perform update in the background. Can be run on/off the worker thread.
+ nsresult ApplyUpdatesBackground(nsACString& aFailedTableName);
+
+ // Perform update for the foreground part. MUST be run on the worker thread.
+ nsresult ApplyUpdatesForeground(nsresult aBackgroundRv,
+ const nsACString& aFailedTableName);
+
+ nsresult NotifyUpdateObserver(nsresult aUpdateStatus);
// Reset the in-progress update stream
void ResetStream();
// Reset the in-progress update
void ResetUpdate();
// Perform a classifier lookup for a given url.