commit 48a63f5601f6 Author: dimi Date: Fri Oct 20 10:18:59 2017 +0800 Bug 1408631 - Release SafeBrowsing lookupcache in worker thread while shutdown. r=francois MozReview-Commit-ID: HuPUyIDFLPX --HG-- extra : rebase_source : d6e4f5bbcf96c97541792e23447f0810150c5ac9 --- toolkit/components/url-classifier/Classifier.cpp | 13 +++++--- .../url-classifier/nsUrlClassifierDBService.cpp | 37 ++++++++++++++++++++++ .../url-classifier/nsUrlClassifierDBService.h | 5 +++ .../url-classifier/nsUrlClassifierProxies.cpp | 10 ++++++ .../url-classifier/nsUrlClassifierProxies.h | 1 + 5 files changed, 62 insertions(+), 4 deletions(-) diff --git toolkit/components/url-classifier/Classifier.cpp toolkit/components/url-classifier/Classifier.cpp index 404e31e2421e..9946469268fa 100644 --- toolkit/components/url-classifier/Classifier.cpp +++ toolkit/components/url-classifier/Classifier.cpp @@ -267,6 +267,8 @@ Classifier::Open(nsIFile& aCacheDirectory) 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(); } @@ -1428,10 +1430,8 @@ Classifier::UpdateCache(TableUpdate* aUpdate) 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& lookupCaches = aForUpdate ? mNewLookupCaches : mLookupCaches; @@ -1444,6 +1444,11 @@ Classifier::GetLookupCache(const nsACString& aTable, bool aForUpdate) } } + // 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'. diff --git toolkit/components/url-classifier/nsUrlClassifierDBService.cpp toolkit/components/url-classifier/nsUrlClassifierDBService.cpp index 78dbfaeaf046..e1a1be065aaf 100644 --- toolkit/components/url-classifier/nsUrlClassifierDBService.cpp +++ toolkit/components/url-classifier/nsUrlClassifierDBService.cpp @@ -817,6 +817,20 @@ nsUrlClassifierDBServiceWorker::CloseDb() 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) { @@ -2427,6 +2441,13 @@ nsUrlClassifierDBService::Observe(nsISupports *aSubject, const char *aTopic, } 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(); @@ -2437,6 +2458,22 @@ nsUrlClassifierDBService::Observe(nsISupports *aSubject, const char *aTopic, 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() diff --git toolkit/components/url-classifier/nsUrlClassifierDBService.h toolkit/components/url-classifier/nsUrlClassifierDBService.h index a4c5952e91bb..9d9671d9d8fa 100644 --- toolkit/components/url-classifier/nsUrlClassifierDBService.h +++ toolkit/components/url-classifier/nsUrlClassifierDBService.h @@ -140,6 +140,9 @@ private: 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(); @@ -220,6 +223,8 @@ public: // 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, diff --git toolkit/components/url-classifier/nsUrlClassifierProxies.cpp toolkit/components/url-classifier/nsUrlClassifierProxies.cpp index 1af4f9266aeb..38e294df7320 100644 --- toolkit/components/url-classifier/nsUrlClassifierProxies.cpp +++ toolkit/components/url-classifier/nsUrlClassifierProxies.cpp @@ -214,6 +214,16 @@ UrlClassifierDBServiceWorkerProxy::CloseDb() return DispatchToWorkerThread(r); } +nsresult +UrlClassifierDBServiceWorkerProxy::PreShutdown() +{ + nsCOMPtr r = + NewRunnableMethod("nsUrlClassifierDBServiceWorker::PreShutdown", + mTarget, + &nsUrlClassifierDBServiceWorker::PreShutdown); + return DispatchToWorkerThread(r); +} + nsresult UrlClassifierDBServiceWorkerProxy::CacheCompletions(CacheResultArray * aEntries) { diff --git toolkit/components/url-classifier/nsUrlClassifierProxies.h toolkit/components/url-classifier/nsUrlClassifierProxies.h index f8cf5229e1ca..39cf47f837a2 100644 --- toolkit/components/url-classifier/nsUrlClassifierProxies.h +++ toolkit/components/url-classifier/nsUrlClassifierProxies.h @@ -229,6 +229,7 @@ public: nsresult OpenDb(); nsresult CloseDb(); + nsresult PreShutdown(); nsresult CacheCompletions(mozilla::safebrowsing::CacheResultArray * aEntries);