Bug 1408631 - Release SafeBrowsing lookupcache in worker thread while shutdown. r?francois
MozReview-Commit-ID: HuPUyIDFLPX
--- a/toolkit/components/url-classifier/Classifier.cpp
+++ b/toolkit/components/url-classifier/Classifier.cpp
@@ -262,16 +262,18 @@ Classifier::Open(nsIFile& aCacheDirector
RegenActiveTables();
return NS_OK;
}
void
Classifier::Close()
{
+ // Close will be called by PreShutdown, so it is important to note that
+ // things put here should not affect an ongoing update thread.
DropStores();
}
void
Classifier::Reset()
{
MOZ_ASSERT(NS_GetCurrentThread() != mUpdateThread,
"Reset() MUST NOT be called on update thread");
@@ -1423,32 +1425,35 @@ Classifier::UpdateCache(TableUpdate* aUp
#endif
return NS_OK;
}
LookupCache *
Classifier::GetLookupCache(const nsACString& aTable, bool aForUpdate)
{
- if (aForUpdate) {
- MOZ_ASSERT(NS_GetCurrentThread() == mUpdateThread,
- "GetLookupCache(aForUpdate==true) can only be called on update thread.");
- }
+ // GetLookupCache(aForUpdate==true) can only be called on update thread.
+ MOZ_ASSERT_IF(aForUpdate, NS_GetCurrentThread() == mUpdateThread);
nsTArray<LookupCache*>& lookupCaches = aForUpdate ? mNewLookupCaches
: mLookupCaches;
auto& rootStoreDirectory = aForUpdate ? mUpdatingDirectory
: mRootStoreDirectory;
for (auto c: lookupCaches) {
if (c->TableName().Equals(aTable)) {
return c;
}
}
+ // We don't want to create lookupcache when shutdown is already happening.
+ if (nsUrlClassifierDBService::ShutdownHasStarted()) {
+ return nullptr;
+ }
+
// 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, rootStoreDirectory);
} else {
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp
@@ -813,16 +813,30 @@ nsUrlClassifierDBServiceWorker::CloseDb(
}
LOG(("urlclassifier db closed\n"));
return NS_OK;
}
nsresult
+nsUrlClassifierDBServiceWorker::PreShutdown()
+{
+ if (mClassifier) {
+ // Classifier close will release all lookup caches which may be a time-consuming job.
+ // See Bug 1408631.
+ mClassifier->Close();
+ }
+
+ // WARNING: nothing we put here should affect an ongoing update thread. When in doubt,
+ // put things in Shutdown() instead.
+ return NS_OK;
+}
+
+nsresult
nsUrlClassifierDBServiceWorker::CacheCompletions(CacheResultArray *results)
{
if (gShuttingDownThread) {
return NS_ERROR_ABORT;
}
LOG(("nsUrlClassifierDBServiceWorker::CacheCompletions [%p]", this));
if (!mClassifier) {
@@ -2422,26 +2436,49 @@ nsUrlClassifierDBService::Observe(nsISup
Unused << prefs;
if (kObservedPrefs.Contains(NS_ConvertUTF16toUTF8(aData))) {
ReadTablesFromPrefs();
}
} else if (!strcmp(aTopic, "quit-application")) {
// Tell the update thread to finish as soon as possible.
gShuttingDownThread = true;
+
+ // The code in ::Shutdown() is run on a 'profile-before-change' event and
+ // ensures that objects are freed by blocking on this freeing.
+ // We can however speed up the shutdown time by using the worker thread to
+ // release, in an earlier event, any objects that cannot affect an ongoing
+ // update on the update thread.
+ PreShutdown();
} else if (!strcmp(aTopic, "profile-before-change")) {
gShuttingDownThread = true;
Shutdown();
} else {
return NS_ERROR_UNEXPECTED;
}
return NS_OK;
}
+// Post a PreShutdown task to worker thread to release objects without blocking
+// main-thread. Notice that shutdown process may still be blocked by PreShutdown task
+// when ::Shutdown() is executed and synchronously waits for worker thread to finish
+// PreShutdown event.
+nsresult
+nsUrlClassifierDBService::PreShutdown()
+{
+ MOZ_ASSERT(XRE_IsParentProcess());
+
+ if (mWorkerProxy) {
+ mWorkerProxy->PreShutdown();
+ }
+
+ 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) {
--- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h
@@ -135,16 +135,19 @@ private:
// Disallow copy constructor
nsUrlClassifierDBService(nsUrlClassifierDBService&);
nsresult LookupURI(nsIPrincipal* aPrincipal,
const nsACString& tables,
nsIUrlClassifierCallback* c,
bool forceCheck, bool *didCheck);
+ // Post an event to worker thread to release objects when receive 'quit-application'
+ nsresult PreShutdown();
+
// Close db connection and join the background thread if it exists.
nsresult Shutdown();
// Check if the key is on a known-clean host.
nsresult CheckClean(const nsACString &lookupKey,
bool *clean);
nsresult ReadTablesFromPrefs();
@@ -215,16 +218,18 @@ public:
LookupResultArray* results);
// Open the DB connection
nsresult GCC_MANGLING_WORKAROUND OpenDb();
// Provide a way to forcibly close the db connection.
nsresult GCC_MANGLING_WORKAROUND CloseDb();
+ nsresult GCC_MANGLING_WORKAROUND PreShutdown();
+
nsresult CacheCompletions(CacheResultArray * aEntries);
// Used to probe the state of the worker thread. When the update begins,
// mUpdateObserver will be set. When the update finished, mUpdateObserver
// will be nulled out in NotifyUpdateObserver.
bool IsBusyUpdating() const { return !!mUpdateObserver; }
// Check the DB ready state of the worker thread
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp
@@ -210,16 +210,26 @@ UrlClassifierDBServiceWorkerProxy::Close
nsCOMPtr<nsIRunnable> r =
NewRunnableMethod("nsUrlClassifierDBServiceWorker::CloseDb",
mTarget,
&nsUrlClassifierDBServiceWorker::CloseDb);
return DispatchToWorkerThread(r);
}
nsresult
+UrlClassifierDBServiceWorkerProxy::PreShutdown()
+{
+ nsCOMPtr<nsIRunnable> r =
+ NewRunnableMethod("nsUrlClassifierDBServiceWorker::PreShutdown",
+ mTarget,
+ &nsUrlClassifierDBServiceWorker::PreShutdown);
+ return DispatchToWorkerThread(r);
+}
+
+nsresult
UrlClassifierDBServiceWorkerProxy::CacheCompletions(CacheResultArray * aEntries)
{
nsCOMPtr<nsIRunnable> r = new CacheCompletionsRunnable(mTarget, aEntries);
return DispatchToWorkerThread(r);
}
NS_IMETHODIMP
UrlClassifierDBServiceWorkerProxy::CacheCompletionsRunnable::Run()
--- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h
+++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h
@@ -224,16 +224,17 @@ public:
public:
nsresult DoLocalLookup(const nsACString& spec,
const nsACString& tables,
mozilla::safebrowsing::LookupResultArray* results);
nsresult OpenDb();
nsresult CloseDb();
+ nsresult PreShutdown();
nsresult CacheCompletions(mozilla::safebrowsing::CacheResultArray * aEntries);
nsresult GetCacheInfo(const nsACString& aTable,
nsIUrlClassifierGetCacheCallback* aCallback);
private:
~UrlClassifierDBServiceWorkerProxy() {}