Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-21 09:15:35 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1591112 - Do not release Classifier in the Safe Browsing update thread. r=gcp

Browse Source 
In AsyncApplyUpdate, the Safe Browsing update thread holds a reference
to 'Classifier'. In some scenarios(See Bug 1591112), the update
thread may be the last one holding the reference; hence the update
thread releases the 'Classifier' when the task is ended.

Classifier has to be created and destroyed in the same thread
because of the constrain of LazyIdelThread, in the current
implementation, it should be released by the worker thread.

This patch transfers the ownership of the reference of 'Classifier 'from the
update thread to the worker thread before its task is finished to make
sure we release 'Classifier' in the right thread.

Differential Revision: https://phabricator.services.mozilla.com/D53156

--HG--
extra : moz-landing-system : lando
This commit is contained in:
DimiDL 2019-11-20 08:34:53 +00:00
parent 31672fe924
commit 83eed8b61a
2 changed files with 12 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
toolkit/components/url-classifier/Classifier.cpp
View File

@ -753,7 +753,7 @@ nsresult Classifier::AsyncApplyUpdates(const TableUpdateArray& aUpdates,
RefPtr<Classifier> self = this;
nsCOMPtr<nsIRunnable> bgRunnable = NS_NewRunnableFunction(
"safebrowsing::Classifier::AsyncApplyUpdates",
[self, aUpdates, aCallback, callerThread] {
[self, aUpdates, aCallback, callerThread] () mutable {
MOZ_ASSERT(self->OnUpdateThread(), "MUST be on update thread");
nsresult bgRv;
@ -773,19 +773,27 @@ nsresult Classifier::AsyncApplyUpdates(const TableUpdateArray& aUpdates,
bgRv = NS_ERROR_OUT_OF_MEMORY;
}
// Classifier is created in the worker thread and it has to be released
// in the worker thread(because of the constrain that LazyIdelThread has
// to be created and released in the same thread). We transfer the ownership
// to the caller thread here to gurantee that we don't release it in
// the udpate thread.
nsCOMPtr<nsIRunnable> fgRunnable = NS_NewRunnableFunction(
"safebrowsing::Classifier::AsyncApplyUpdates",
[self, aCallback, bgRv, failedTableNames, callerThread] {
[self = std::move(self), aCallback, bgRv, failedTableNames, callerThread] () mutable {
RefPtr<Classifier> classifier = std::move(self);
MOZ_ASSERT(NS_GetCurrentThread() == callerThread,
"MUST be on caller thread");
LOG(("Step 2. ApplyUpdatesForeground on caller thread"));
nsresult rv =
self->ApplyUpdatesForeground(bgRv, failedTableNames);
classifier->ApplyUpdatesForeground(bgRv, failedTableNames);
LOG(("Step 3. Updates applied! Fire callback."));
aCallback(rv);
});
callerThread->Dispatch(fgRunnable, NS_DISPATCH_NORMAL);
});

3
toolkit/components/url-classifier/tests/mochitest/mochitest.ini
View File

@ -47,9 +47,8 @@ skip-if = (verify && debug && (os == 'win' || os == 'mac'))
[test_classify_track.html]
[test_gethash.html]
[test_bug1254766.html]
skip-if = (debug && os == "linux") #Bug 1591112
[test_cachemiss.html]
skip-if = verify || (debug && os == "linux") #Bug 1591112
skip-if = verify
[test_annotation_vs_TP.html]
[test_fingerprinting.html]
[test_fingerprinting_annotate.html]