From 51e437838338a0c077c95d119f46de015bf21ea3 Mon Sep 17 00:00:00 2001 From: Monica Chew Date: Tue, 13 Jan 2015 17:09:13 -0800 Subject: [PATCH] Bug 1120499: Proxy DoLocalLookup to the worker thread (r=gcp) --- .../components/url-classifier/Classifier.cpp | 13 +- .../components/url-classifier/Classifier.h | 1 - .../nsUrlClassifierDBService.cpp | 120 ++---------------- .../url-classifier/nsUrlClassifierDBService.h | 118 ++++++++++++++++- .../url-classifier/nsUrlClassifierProxies.cpp | 27 ++++ .../url-classifier/nsUrlClassifierProxies.h | 60 ++++++--- 6 files changed, 194 insertions(+), 145 deletions(-) diff --git a/toolkit/components/url-classifier/Classifier.cpp b/toolkit/components/url-classifier/Classifier.cpp index aeb1208c8ef..e4f0167d992 100644 --- a/toolkit/components/url-classifier/Classifier.cpp +++ b/toolkit/components/url-classifier/Classifier.cpp @@ -144,9 +144,6 @@ Classifier::Open(nsIFile& aCacheDirectory) rv = CreateStoreDirectory(); NS_ENSURE_SUCCESS(rv, rv); - // Classifier keeps its own cryptoHash for doing operations on the background - // thread. Callers can optionally pass in an nsICryptoHash for working on the - // main thread. mCryptoHash = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); @@ -220,14 +217,8 @@ nsresult Classifier::Check(const nsACString& aSpec, const nsACString& aTables, uint32_t aFreshnessGuarantee, - nsICryptoHash* aCryptoHash, LookupResultArray& aResults) { - nsCOMPtr cryptoHash = aCryptoHash; - if (!aCryptoHash) { - MOZ_ASSERT(!NS_IsMainThread(), "mCryptoHash must be used on worker thread"); - cryptoHash = mCryptoHash; - } Telemetry::AutoTimer timer; // Get the set of fragments based on the url. This is necessary because we @@ -254,11 +245,11 @@ Classifier::Check(const nsACString& aSpec, // Now check each lookup fragment against the entries in the DB. for (uint32_t i = 0; i < fragments.Length(); i++) { Completion lookupHash; - lookupHash.FromPlaintext(fragments[i], cryptoHash); + lookupHash.FromPlaintext(fragments[i], mCryptoHash); // Get list of host keys to look up Completion hostKey; - rv = LookupCache::GetKey(fragments[i], &hostKey, cryptoHash); + rv = LookupCache::GetKey(fragments[i], &hostKey, mCryptoHash); if (NS_FAILED(rv)) { // Local host on the network. continue; diff --git a/toolkit/components/url-classifier/Classifier.h b/toolkit/components/url-classifier/Classifier.h index eff8afb4dc7..f936bdfe859 100644 --- a/toolkit/components/url-classifier/Classifier.h +++ b/toolkit/components/url-classifier/Classifier.h @@ -48,7 +48,6 @@ public: nsresult Check(const nsACString& aSpec, const nsACString& tables, uint32_t aFreshnessGuarantee, - nsICryptoHash* aCryptoHash, LookupResultArray& aResults); /** diff --git a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp index 20448d67a60..904529131c4 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp +++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp @@ -41,6 +41,9 @@ #include "prprf.h" #include "prnetdb.h" #include "Entries.h" +#include "HashStore.h" +#include "Classifier.h" +#include "ProtocolParser.h" #include "mozilla/Attributes.h" #include "nsIPrincipal.h" #include "Classifier.h" @@ -97,108 +100,6 @@ static bool gShuttingDownThread = false; static mozilla::Atomic gFreshnessGuarantee(CONFIRM_AGE_DEFAULT_SEC); -// ------------------------------------------------------------------------- -// Actual worker implementation -class nsUrlClassifierDBServiceWorker MOZ_FINAL : - public nsIUrlClassifierDBServiceWorker -{ -public: - nsUrlClassifierDBServiceWorker(); - - NS_DECL_THREADSAFE_ISUPPORTS - NS_DECL_NSIURLCLASSIFIERDBSERVICE - NS_DECL_NSIURLCLASSIFIERDBSERVICEWORKER - - nsresult Init(uint32_t aGethashNoise, nsCOMPtr aCacheDir); - - // Queue a lookup for the worker to perform, called in the main thread. - // tables is a comma-separated list of tables to query - nsresult QueueLookup(const nsACString& lookupKey, - const nsACString& tables, - nsIUrlClassifierLookupCallback* callback); - - // Handle any queued-up lookups. We call this function during long-running - // update operations to prevent lookups from blocking for too long. - nsresult HandlePendingLookups(); - - // Perform a blocking classifier lookup for a given url. Can be called on - // either the main thread or the worker thread. - nsresult DoLocalLookup(const nsACString& spec, - const nsACString& tables, - nsICryptoHash* cryptoHash, - LookupResultArray* results); - -private: - // No subclassing - ~nsUrlClassifierDBServiceWorker(); - - // Disallow copy constructor - nsUrlClassifierDBServiceWorker(nsUrlClassifierDBServiceWorker&); - - // Applies the current transaction and resets the update/working times. - nsresult ApplyUpdate(); - - // Reset the in-progress update stream - void ResetStream(); - - // Reset the in-progress update - void ResetUpdate(); - - // Perform a classifier lookup for a given url. - nsresult DoLookup(const nsACString& spec, - const nsACString& tables, - nsIUrlClassifierLookupCallback* c); - - nsresult AddNoise(const Prefix aPrefix, - const nsCString tableName, - uint32_t aCount, - LookupResultArray& results); - - // Can only be used on the background thread - nsCOMPtr mCryptoHash; - - nsAutoPtr mClassifier; - // The class that actually parses the update chunks. - nsAutoPtr mProtocolParser; - - // Directory where to store the SB databases. - nsCOMPtr mCacheDir; - - // XXX: maybe an array of autoptrs. Or maybe a class specifically - // storing a series of updates. - nsTArray mTableUpdates; - - int32_t mUpdateWait; - - // Entries that cannot be completed. We expect them to die at - // the next update - PrefixArray mMissCache; - - nsresult mUpdateStatus; - nsTArray mUpdateTables; - - nsCOMPtr mUpdateObserver; - bool mInStream; - - // The number of noise entries to add to the set of lookup results. - uint32_t mGethashNoise; - - // Pending lookups are stored in a queue for processing. The queue - // is protected by mPendingLookupLock. - Mutex mPendingLookupLock; - - class PendingLookup { - public: - TimeStamp mStartTime; - nsCString mKey; - nsCString mTables; - nsCOMPtr mCallback; - }; - - // list of pending lookups - nsTArray mPendingLookups; -}; - NS_IMPL_ISUPPORTS(nsUrlClassifierDBServiceWorker, nsIUrlClassifierDBServiceWorker, nsIUrlClassifierDBService) @@ -250,11 +151,9 @@ nsUrlClassifierDBServiceWorker::QueueLookup(const nsACString& spec, nsresult nsUrlClassifierDBServiceWorker::DoLocalLookup(const nsACString& spec, const nsACString& tables, - nsICryptoHash* cryptoHash, LookupResultArray* results) { - LOG(("nsUrlClassifierDBServiceWorker::DoLocalLookup %s (main=%s) %p", - spec.Data(), NS_IsMainThread() ? "true" : "false", this)); + MOZ_ASSERT(!NS_IsMainThread(), "DoLocalLookup must be on background thread"); if (!results) { return NS_ERROR_FAILURE; } @@ -265,7 +164,7 @@ nsUrlClassifierDBServiceWorker::DoLocalLookup(const nsACString& spec, // We ignore failures from Check because we'd rather return the // results that were found than fail. - mClassifier->Check(spec, tables, gFreshnessGuarantee, cryptoHash, *results); + mClassifier->Check(spec, tables, gFreshnessGuarantee, *results); LOG(("Found %d results.", results->Length())); return NS_OK; @@ -351,7 +250,7 @@ nsUrlClassifierDBServiceWorker::DoLookup(const nsACString& spec, return NS_ERROR_OUT_OF_MEMORY; } - nsresult rv = DoLocalLookup(spec, tables, nullptr, results); + nsresult rv = DoLocalLookup(spec, tables, results); if (NS_FAILED(rv)) { c->LookupComplete(nullptr); return rv; @@ -1223,7 +1122,7 @@ nsUrlClassifierDBService::Init() // Force PSM loading on main thread nsresult rv; - mCryptoHashMain = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv); + nsCOMPtr dummy = do_CreateInstance(NS_CRYPTO_HASH_CONTRACTID, &rv); NS_ENSURE_SUCCESS(rv, rv); // Directory providers must also be accessed on the main thread. @@ -1357,9 +1256,8 @@ nsUrlClassifierDBService::ClassifyLocal(nsIPrincipal* aPrincipal, return NS_ERROR_OUT_OF_MEMORY; } - // We don't use the proxy, since this is a blocking lookup. In unittests, we - // may not have been initalized, so don't crash. - rv = mWorker->DoLocalLookup(key, tables, mCryptoHashMain, results); + // In unittests, we may not have been initalized, so don't crash. + rv = mWorkerProxy->DoLocalLookup(key, tables, results); if (NS_SUCCEEDED(rv)) { rv = ProcessLookupResults(results, mCheckMalware, mCheckPhishing, mCheckTracking); diff --git a/toolkit/components/url-classifier/nsUrlClassifierDBService.h b/toolkit/components/url-classifier/nsUrlClassifierDBService.h index 398f72b5f3f..3e94616dd47 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierDBService.h +++ b/toolkit/components/url-classifier/nsUrlClassifierDBService.h @@ -17,10 +17,12 @@ #include "nsIUrlClassifierDBService.h" #include "nsIURIClassifier.h" #include "nsToolkitCompsCID.h" -#include "nsICryptoHash.h" #include "nsICryptoHMAC.h" #include "mozilla/Attributes.h" +#include "mozilla/Mutex.h" +#include "mozilla/TimeStamp.h" +#include "Entries.h" #include "LookupCache.h" // The hash length for a domain key. @@ -32,10 +34,19 @@ // The hash length of a complete hash entry. #define COMPLETE_LENGTH 32 +using namespace mozilla::safebrowsing; + class nsUrlClassifierDBServiceWorker; -class nsICryptoHash; class nsIThread; class nsIURI; +class UrlClassifierDBServiceWorkerProxy; +namespace mozilla { +namespace safebrowsing { +class Classifier; +class ProtocolParser; +class TableUpdate; +} +} // This is a proxy class that just creates a background thread and delagates // calls to the background thread. @@ -91,7 +102,7 @@ private: void BuildTables(bool trackingProtectionEnabled, nsCString& tables); nsRefPtr mWorker; - nsCOMPtr mWorkerProxy; + nsRefPtr mWorkerProxy; nsInterfaceHashtable mCompleters; @@ -121,10 +132,105 @@ private: // Thread that we do the updates on. static nsIThread* gDbBackgroundThread; +}; - // nsICryptoHash for doing hash operations on the main thread. This is only - // used for nsIURIClassifier.ClassifyLocal - nsCOMPtr mCryptoHashMain; +class nsUrlClassifierDBServiceWorker MOZ_FINAL : + public nsIUrlClassifierDBServiceWorker +{ +public: + nsUrlClassifierDBServiceWorker(); + + NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_NSIURLCLASSIFIERDBSERVICE + NS_DECL_NSIURLCLASSIFIERDBSERVICEWORKER + + nsresult Init(uint32_t aGethashNoise, nsCOMPtr aCacheDir); + + // Queue a lookup for the worker to perform, called in the main thread. + // tables is a comma-separated list of tables to query + nsresult QueueLookup(const nsACString& lookupKey, + const nsACString& tables, + nsIUrlClassifierLookupCallback* callback); + + // Handle any queued-up lookups. We call this function during long-running + // update operations to prevent lookups from blocking for too long. + nsresult HandlePendingLookups(); + + // Perform a blocking classifier lookup for a given url. Can be called on + // either the main thread or the worker thread. + nsresult DoLocalLookup(const nsACString& spec, + const nsACString& tables, + LookupResultArray* results); + +private: + // No subclassing + ~nsUrlClassifierDBServiceWorker(); + + // Disallow copy constructor + nsUrlClassifierDBServiceWorker(nsUrlClassifierDBServiceWorker&); + + // Applies the current transaction and resets the update/working times. + nsresult ApplyUpdate(); + + // Reset the in-progress update stream + void ResetStream(); + + // Reset the in-progress update + void ResetUpdate(); + + // Perform a classifier lookup for a given url. + nsresult DoLookup(const nsACString& spec, + const nsACString& tables, + nsIUrlClassifierLookupCallback* c); + + nsresult AddNoise(const Prefix aPrefix, + const nsCString tableName, + uint32_t aCount, + LookupResultArray& results); + + // Can only be used on the background thread + nsCOMPtr mCryptoHash; + + nsAutoPtr mClassifier; + // The class that actually parses the update chunks. + nsAutoPtr mProtocolParser; + + // Directory where to store the SB databases. + nsCOMPtr mCacheDir; + + // XXX: maybe an array of autoptrs. Or maybe a class specifically + // storing a series of updates. + nsTArray mTableUpdates; + + int32_t mUpdateWait; + + // Entries that cannot be completed. We expect them to die at + // the next update + PrefixArray mMissCache; + + nsresult mUpdateStatus; + nsTArray mUpdateTables; + + nsCOMPtr mUpdateObserver; + bool mInStream; + + // The number of noise entries to add to the set of lookup results. + uint32_t mGethashNoise; + + // Pending lookups are stored in a queue for processing. The queue + // is protected by mPendingLookupLock. + mozilla::Mutex mPendingLookupLock; + + class PendingLookup { + public: + mozilla::TimeStamp mStartTime; + nsCString mKey; + nsCString mTables; + nsCOMPtr mCallback; + }; + + // list of pending lookups + nsTArray mPendingLookups; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsUrlClassifierDBService, NS_URLCLASSIFIERDBSERVICE_CID) diff --git a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp index fba5ada85be..9c34eb5c6c4 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp +++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.cpp @@ -6,6 +6,8 @@ #include "nsUrlClassifierProxies.h" #include "nsUrlClassifierDBService.h" +#include "mozilla/SyncRunnable.h" + using namespace mozilla::safebrowsing; static nsresult @@ -116,6 +118,31 @@ UrlClassifierDBServiceWorkerProxy::FinishStream() return DispatchToWorkerThread(r); } +NS_IMETHODIMP +UrlClassifierDBServiceWorkerProxy::DoLocalLookupRunnable::Run() +{ + mTarget->DoLocalLookup(mSpec, mTables, mResults); + return NS_OK; +} + +nsresult +UrlClassifierDBServiceWorkerProxy::DoLocalLookup(const nsACString& spec, + const nsACString& tables, + LookupResultArray* results) + +{ + // Run synchronously on background thread. NS_DISPATCH_SYNC does *not* do + // what we want -- it continues processing events on the main thread loop + // before the Dispatch returns. + nsCOMPtr r = new DoLocalLookupRunnable(mTarget, spec, tables, results); + nsIThread* t = nsUrlClassifierDBService::BackgroundThread(); + if (!t) + return NS_ERROR_FAILURE; + + mozilla::SyncRunnable::DispatchToThread(t, r); + return NS_OK; +} + NS_IMETHODIMP UrlClassifierDBServiceWorkerProxy::FinishUpdate() { diff --git a/toolkit/components/url-classifier/nsUrlClassifierProxies.h b/toolkit/components/url-classifier/nsUrlClassifierProxies.h index 3ab1948a1e5..c4e55f6339a 100644 --- a/toolkit/components/url-classifier/nsUrlClassifierProxies.h +++ b/toolkit/components/url-classifier/nsUrlClassifierProxies.h @@ -7,6 +7,7 @@ #define nsUrlClassifierProxies_h #include "nsIUrlClassifierDBService.h" +#include "nsUrlClassifierDBService.h" #include "nsProxyRelease.h" #include "nsThreadUtils.h" #include "mozilla/Attributes.h" @@ -21,7 +22,7 @@ class UrlClassifierDBServiceWorkerProxy MOZ_FINAL : public nsIUrlClassifierDBServiceWorker { public: - explicit UrlClassifierDBServiceWorkerProxy(nsIUrlClassifierDBServiceWorker* aTarget) + explicit UrlClassifierDBServiceWorkerProxy(nsUrlClassifierDBServiceWorker* aTarget) : mTarget(aTarget) { } @@ -32,7 +33,7 @@ public: class LookupRunnable : public nsRunnable { public: - LookupRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + LookupRunnable(nsUrlClassifierDBServiceWorker* aTarget, nsIPrincipal* aPrincipal, const nsACString& aTables, nsIUrlClassifierCallback* aCB) @@ -45,7 +46,7 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; nsCOMPtr mPrincipal; nsCString mLookupTables; nsCOMPtr mCB; @@ -54,7 +55,7 @@ public: class GetTablesRunnable : public nsRunnable { public: - GetTablesRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + GetTablesRunnable(nsUrlClassifierDBServiceWorker* aTarget, nsIUrlClassifierCallback* aCB) : mTarget(aTarget) , mCB(aCB) @@ -63,14 +64,14 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; nsCOMPtr mCB; }; class BeginUpdateRunnable : public nsRunnable { public: - BeginUpdateRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + BeginUpdateRunnable(nsUrlClassifierDBServiceWorker* aTarget, nsIUrlClassifierUpdateObserver* aUpdater, const nsACString& aTables) : mTarget(aTarget) @@ -81,7 +82,7 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; nsCOMPtr mUpdater; nsCString mTables; }; @@ -89,7 +90,7 @@ public: class BeginStreamRunnable : public nsRunnable { public: - BeginStreamRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + BeginStreamRunnable(nsUrlClassifierDBServiceWorker* aTarget, const nsACString& aTable) : mTarget(aTarget) , mTable(aTable) @@ -98,14 +99,14 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; nsCString mTable; }; class UpdateStreamRunnable : public nsRunnable { public: - UpdateStreamRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + UpdateStreamRunnable(nsUrlClassifierDBServiceWorker* aTarget, const nsACString& aUpdateChunk) : mTarget(aTarget) , mUpdateChunk(aUpdateChunk) @@ -114,14 +115,14 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; nsCString mUpdateChunk; }; class CacheCompletionsRunnable : public nsRunnable { public: - CacheCompletionsRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + CacheCompletionsRunnable(nsUrlClassifierDBServiceWorker* aTarget, mozilla::safebrowsing::CacheResultArray *aEntries) : mTarget(aTarget) , mEntries(aEntries) @@ -130,14 +131,14 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; mozilla::safebrowsing::CacheResultArray *mEntries; }; class CacheMissesRunnable : public nsRunnable { public: - CacheMissesRunnable(nsIUrlClassifierDBServiceWorker* aTarget, + CacheMissesRunnable(nsUrlClassifierDBServiceWorker* aTarget, mozilla::safebrowsing::PrefixArray *aEntries) : mTarget(aTarget) , mEntries(aEntries) @@ -146,14 +147,41 @@ public: NS_DECL_NSIRUNNABLE private: - nsCOMPtr mTarget; + nsRefPtr mTarget; mozilla::safebrowsing::PrefixArray *mEntries; }; + class DoLocalLookupRunnable : public nsRunnable + { + public: + DoLocalLookupRunnable(nsUrlClassifierDBServiceWorker* aTarget, + const nsACString& spec, + const nsACString& tables, + mozilla::safebrowsing::LookupResultArray* results) + : mTarget(aTarget) + , mSpec(spec) + , mTables(tables) + , mResults(results) + { } + + NS_DECL_NSIRUNNABLE + private: + nsRefPtr mTarget; + + nsCString mSpec; + nsCString mTables; + mozilla::safebrowsing::LookupResultArray* mResults; + }; + +public: + nsresult DoLocalLookup(const nsACString& spec, + const nsACString& tables, + mozilla::safebrowsing::LookupResultArray* results); + private: ~UrlClassifierDBServiceWorkerProxy() {} - nsCOMPtr mTarget; + nsRefPtr mTarget; }; // The remaining classes here are all proxies to the main thread