From 43d3425903ed93b60eb551b4fc0f6f0001af8558 Mon Sep 17 00:00:00 2001 From: "reed@reedloden.com" Date: Wed, 7 May 2008 03:21:11 -0700 Subject: [PATCH] Bug 432492 - "rate limit long-running safebrowsing updates" [p=dcamp@mozilla.com (Dave Camp) r=tony a1.9=beltzner] --- .../public/nsIUrlClassifierDBService.idl | 4 +- .../src/nsUrlClassifierDBService.cpp | 154 ++++++++++++++---- .../src/nsUrlClassifierStreamUpdater.cpp | 91 ++++++++--- .../src/nsUrlClassifierStreamUpdater.h | 8 +- 4 files changed, 200 insertions(+), 57 deletions(-) diff --git a/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl b/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl index e9386fb060e2..0f7574a9a065 100644 --- a/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl +++ b/toolkit/components/url-classifier/public/nsIUrlClassifierDBService.idl @@ -83,8 +83,10 @@ interface nsIUrlClassifierUpdateObserver : nsISupports { * A stream update has completed. * * @param status The state of the update process. + * @param delay The amount of time the updater should wait to fetch the + * next URL in ms. */ - void streamFinished(in nsresult status); + void streamFinished(in nsresult status, in unsigned long delay); /* The update has encountered an error and should be cancelled */ void updateError(in nsresult error); diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp index 7abcd41c25a7..fbb73fb656b3 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp +++ b/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp @@ -154,6 +154,17 @@ static const PRLogModuleInfo *gUrlClassifierDbServiceLog = nsnull; #define UPDATE_CACHE_SIZE_PREF "urlclassifier.updatecachemax" #define UPDATE_CACHE_SIZE_DEFAULT -1 +// Amount of time to spend updating before committing and delaying, in +// seconds. This is checked after each update stream, so the actual +// time spent can be higher than this, depending on update stream size. +#define UPDATE_WORKING_TIME "urlclassifier.workingtime" +#define UPDATE_WORKING_TIME_DEFAULT 5 + +// The amount of time to delay after hitting UPDATE_WORKING_TIME, in +// seconds. +#define UPDATE_DELAY_TIME "urlclassifier.updatetime" +#define UPDATE_DELAY_TIME_DEFAULT 60 + #define PAGE_SIZE 4096 class nsUrlClassifierDBServiceWorker; @@ -172,6 +183,9 @@ static PRInt32 gFreshnessGuarantee = CONFIRM_AGE_DEFAULT_SEC; static PRInt32 gUpdateCacheSize = UPDATE_CACHE_SIZE_DEFAULT; +static PRInt32 gWorkingTimeThreshold = UPDATE_WORKING_TIME_DEFAULT; +static PRInt32 gDelayTime = UPDATE_DELAY_TIME_DEFAULT; + static void SplitTables(const nsACString& str, nsTArray& tables) { @@ -1102,6 +1116,12 @@ private: // Handle chunk data from a stream update nsresult ProcessChunk(PRBool* done); + // Sets up a transaction and begins counting update time. + nsresult SetupUpdate(); + + // Applies the current transaction and resets the update/working times. + nsresult ApplyUpdate(); + // Reset the in-progress update stream void ResetStream(); @@ -1217,6 +1237,10 @@ private: // The MAC stated by the server. nsCString mServerMAC; + // Start time of the current update interval. This will be reset + // every time weapply the update. + PRIntervalTime mUpdateStartTime; + nsCOMPtr mHMAC; // The number of noise entries to add to the set of lookup results. PRInt32 mGethashNoise; @@ -1256,6 +1280,7 @@ nsUrlClassifierDBServiceWorker::nsUrlClassifierDBServiceWorker() , mCachedListsTable(PR_UINT32_MAX) , mHaveCachedAddChunks(PR_FALSE) , mHaveCachedSubChunks(PR_FALSE) + , mUpdateStartTime(0) , mGethashNoise(0) , mPendingLookupLock(nsnull) { @@ -2757,19 +2782,7 @@ nsUrlClassifierDBServiceWorker::BeginUpdate(nsIUrlClassifierUpdateObserver *obse return rv; } - if (gUpdateCacheSize > 0) { - PRUint32 cachePages = gUpdateCacheSize / PAGE_SIZE; - nsCAutoString cacheSizePragma("PRAGMA cache_size="); - cacheSizePragma.AppendInt(cachePages); - rv = mConnection->ExecuteSimpleSQL(cacheSizePragma); - if (NS_FAILED(rv)) { - mUpdateStatus = rv; - return rv; - } - mGrewCache = PR_TRUE; - } - - rv = mConnection->BeginTransaction(); + rv = SetupUpdate(); if (NS_FAILED(rv)) { mUpdateStatus = rv; return rv; @@ -2801,9 +2814,15 @@ nsUrlClassifierDBServiceWorker::BeginStream(const nsACString &table, NS_ENSURE_STATE(mUpdateObserver); NS_ENSURE_STATE(!mInStream); - mInStream = PR_TRUE; + // We may have committed the update in FinishStream, if so set it up + // again here. + nsresult rv = SetupUpdate(); + if (NS_FAILED(rv)) { + mUpdateStatus = rv; + return rv; + } - nsresult rv; + mInStream = PR_TRUE; // If we're expecting a MAC, create the nsICryptoHMAC component now. if (!mUpdateClientKey.IsEmpty()) { @@ -2941,6 +2960,8 @@ nsUrlClassifierDBServiceWorker::FinishStream() NS_ENSURE_STATE(mInStream); NS_ENSURE_STATE(mUpdateObserver); + PRInt32 nextStreamDelay = 0; + if (NS_SUCCEEDED(mUpdateStatus) && mHMAC) { nsCAutoString clientMAC; mHMAC->Finish(PR_TRUE, clientMAC); @@ -2951,15 +2972,80 @@ nsUrlClassifierDBServiceWorker::FinishStream() mServerMAC.get(), clientMAC.get())); mUpdateStatus = NS_ERROR_FAILURE; } + PRIntervalTime updateTime = PR_IntervalNow() - mUpdateStartTime; + if (PR_IntervalToSeconds(updateTime) >= + static_cast(gWorkingTimeThreshold)) { + // We've spent long enough working that we should commit what we + // have and hold off for a bit. + ApplyUpdate(); + + nextStreamDelay = gDelayTime * 1000; + } } - mUpdateObserver->StreamFinished(mUpdateStatus); + mUpdateObserver->StreamFinished(mUpdateStatus, + static_cast(nextStreamDelay)); ResetStream(); return NS_OK; } +nsresult +nsUrlClassifierDBServiceWorker::SetupUpdate() +{ + LOG(("nsUrlClassifierDBServiceWorker::SetupUpdate")); + PRBool inProgress; + nsresult rv = mConnection->GetTransactionInProgress(&inProgress); + if (inProgress) { + return NS_OK; + } + + mUpdateStartTime = PR_IntervalNow(); + + rv = mConnection->BeginTransaction(); + NS_ENSURE_SUCCESS(rv, rv); + + if (gUpdateCacheSize > 0) { + PRUint32 cachePages = gUpdateCacheSize / PAGE_SIZE; + nsCAutoString cacheSizePragma("PRAGMA cache_size="); + cacheSizePragma.AppendInt(cachePages); + rv = mConnection->ExecuteSimpleSQL(cacheSizePragma); + NS_ENSURE_SUCCESS(rv, rv); + mGrewCache = PR_TRUE; + } + + return NS_OK; +} + +nsresult +nsUrlClassifierDBServiceWorker::ApplyUpdate() +{ + LOG(("nsUrlClassifierDBServiceWorker::ApplyUpdate")); + + if (NS_FAILED(mUpdateStatus)) { + mConnection->RollbackTransaction(); + } else { + mUpdateStatus = FlushChunkLists(); + if (NS_SUCCEEDED(mUpdateStatus)) { + mUpdateStatus = mConnection->CommitTransaction(); + } + } + + if (mGrewCache) { + // During the update we increased the page cache to bigger than we + // want to keep around. At the moment, the only reliable way to make + // sure that the page cache is freed is to reopen the connection. + mGrewCache = PR_FALSE; + CloseDb(); + OpenDb(); + } + + mUpdateStartTime = 0; + + return NS_OK; +} + NS_IMETHODIMP nsUrlClassifierDBServiceWorker::FinishUpdate() { @@ -2969,16 +3055,7 @@ nsUrlClassifierDBServiceWorker::FinishUpdate() NS_ENSURE_STATE(!mInStream); NS_ENSURE_STATE(mUpdateObserver); - if (NS_SUCCEEDED(mUpdateStatus)) { - mUpdateStatus = FlushChunkLists(); - } - - nsCAutoString arg; - if (NS_SUCCEEDED(mUpdateStatus)) { - mUpdateStatus = mConnection->CommitTransaction(); - } else { - mConnection->RollbackTransaction(); - } + ApplyUpdate(); if (NS_SUCCEEDED(mUpdateStatus)) { mUpdateObserver->UpdateSuccess(mUpdateWait); @@ -3011,13 +3088,6 @@ nsUrlClassifierDBServiceWorker::FinishUpdate() // database reset. if (NS_SUCCEEDED(mUpdateStatus) && resetRequested) { ResetDatabase(); - } else if (mGrewCache) { - // During the update we increased the page cache to bigger than we - // want to keep around. At the moment, the only reliable way to make - // sure that the page cache is freed is to reopen the connection. - mGrewCache = PR_FALSE; - CloseDb(); - OpenDb(); } return NS_OK; @@ -3668,6 +3738,14 @@ nsUrlClassifierDBService::Init() rv = prefs->GetIntPref(UPDATE_CACHE_SIZE_PREF, &tmpint); PR_AtomicSet(&gUpdateCacheSize, NS_SUCCEEDED(rv) ? tmpint : UPDATE_CACHE_SIZE_DEFAULT); + + rv = prefs->GetIntPref(UPDATE_WORKING_TIME, &tmpint); + PR_AtomicSet(&gWorkingTimeThreshold, + NS_SUCCEEDED(rv) ? tmpint : UPDATE_WORKING_TIME_DEFAULT); + + rv = prefs->GetIntPref(UPDATE_DELAY_TIME, &tmpint); + PR_AtomicSet(&gDelayTime, + NS_SUCCEEDED(rv) ? tmpint : UPDATE_DELAY_TIME_DEFAULT); } // Start the background thread. @@ -3960,6 +4038,16 @@ nsUrlClassifierDBService::Observe(nsISupports *aSubject, const char *aTopic, PRInt32 tmpint; rv = prefs->GetIntPref(UPDATE_CACHE_SIZE_PREF, &tmpint); PR_AtomicSet(&gUpdateCacheSize, NS_SUCCEEDED(rv) ? tmpint : UPDATE_CACHE_SIZE_DEFAULT); + } else if (NS_LITERAL_STRING(UPDATE_WORKING_TIME).Equals(aData)) { + PRInt32 tmpint; + rv = prefs->GetIntPref(UPDATE_WORKING_TIME, &tmpint); + PR_AtomicSet(&gWorkingTimeThreshold, + NS_SUCCEEDED(rv) ? tmpint : UPDATE_WORKING_TIME_DEFAULT); + } else if (NS_LITERAL_STRING(UPDATE_DELAY_TIME).Equals(aData)) { + PRInt32 tmpint; + rv = prefs->GetIntPref(UPDATE_DELAY_TIME, &tmpint); + PR_AtomicSet(&gDelayTime, + NS_SUCCEEDED(rv) ? tmpint : UPDATE_DELAY_TIME_DEFAULT); } } else if (!strcmp(aTopic, "profile-before-change") || !strcmp(aTopic, "xpcom-shutdown-threads")) { diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp index 5dad171e349a..fa1086a7cdcc 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp +++ b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.cpp @@ -75,7 +75,7 @@ nsUrlClassifierStreamUpdater::nsUrlClassifierStreamUpdater() } -NS_IMPL_THREADSAFE_ISUPPORTS8(nsUrlClassifierStreamUpdater, +NS_IMPL_THREADSAFE_ISUPPORTS9(nsUrlClassifierStreamUpdater, nsIUrlClassifierStreamUpdater, nsIUrlClassifierUpdateObserver, nsIRequestObserver, @@ -83,7 +83,8 @@ NS_IMPL_THREADSAFE_ISUPPORTS8(nsUrlClassifierStreamUpdater, nsIObserver, nsIBadCertListener2, nsISSLErrorListener, - nsIInterfaceRequestor) + nsIInterfaceRequestor, + nsITimerCallback) /** * Clear out the update. @@ -271,29 +272,54 @@ nsUrlClassifierStreamUpdater::RekeyRequested() nsnull); } -NS_IMETHODIMP -nsUrlClassifierStreamUpdater::StreamFinished(nsresult status) +nsresult +nsUrlClassifierStreamUpdater::FetchNext() { - nsresult rv; + if (mPendingUpdates.Length() == 0) { + return NS_OK; + } - // Pop off a pending URL and update it. - if (NS_SUCCEEDED(status) && mPendingUpdates.Length() > 0) { - PendingUpdate &update = mPendingUpdates[0]; - rv = FetchUpdate(update.mUrl, EmptyCString(), - update.mTable, update.mServerMAC); - if (NS_FAILED(rv)) { - LOG(("Error fetching update url: %s\n", update.mUrl.get())); - // We can commit the urls that we've applied so far. This is - // probably a transient server problem, so trigger backoff. - mDownloadErrorCallback->HandleEvent(EmptyCString()); - mDownloadError = PR_TRUE; - mDBService->FinishUpdate(); - return rv; - } - - mPendingUpdates.RemoveElementAt(0); - } else { + PendingUpdate &update = mPendingUpdates[0]; + LOG(("Fetching update url: %s\n", update.mUrl.get())); + nsresult rv = FetchUpdate(update.mUrl, EmptyCString(), + update.mTable, update.mServerMAC); + if (NS_FAILED(rv)) { + LOG(("Error fetching update url: %s\n", update.mUrl.get())); + // We can commit the urls that we've applied so far. This is + // probably a transient server problem, so trigger backoff. + mDownloadErrorCallback->HandleEvent(EmptyCString()); + mDownloadError = PR_TRUE; mDBService->FinishUpdate(); + return rv; + } + + mPendingUpdates.RemoveElementAt(0); + + return NS_OK; +} + +NS_IMETHODIMP +nsUrlClassifierStreamUpdater::StreamFinished(nsresult status, + PRUint32 requestedDelay) +{ + LOG(("nsUrlClassifierStreamUpdater::StreamFinished [%x, %d]", status, requestedDelay)); + if (NS_FAILED(status) || mPendingUpdates.Length() == 0) { + // We're done. + mDBService->FinishUpdate(); + return NS_OK; + } + + // Wait the requested amount of time before starting a new stream. + nsresult rv; + mTimer = do_CreateInstance("@mozilla.org/timer;1", &rv); + if (NS_SUCCEEDED(rv)) { + rv = mTimer->InitWithCallback(this, requestedDelay, + nsITimer::TYPE_ONE_SHOT); + } + + if (NS_FAILED(rv)) { + NS_WARNING("Unable to initialize timer, fetching next safebrowsing item immediately"); + return FetchNext(); } return NS_OK; @@ -500,6 +526,10 @@ nsUrlClassifierStreamUpdater::Observe(nsISupports *aSubject, const char *aTopic, mIsUpdating = PR_FALSE; mChannel = nsnull; } + if (mTimer) { + mTimer->Cancel(); + mTimer = nsnull; + } } return NS_OK; } @@ -538,3 +568,20 @@ nsUrlClassifierStreamUpdater::GetInterface(const nsIID & eventSinkIID, void* *_r { return QueryInterface(eventSinkIID, _retval); } + + +/////////////////////////////////////////////////////////////////////////////// +// nsITimerCallback implementation +NS_IMETHODIMP +nsUrlClassifierStreamUpdater::Notify(nsITimer *timer) +{ + LOG(("nsUrlClassifierStreamUpdater::Notify [%p]", this)); + + mTimer = nsnull; + + // Start the update process up again. + FetchNext(); + + return NS_OK; +} + diff --git a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h index 5048b322f37a..7279d72e8bbc 100644 --- a/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h +++ b/toolkit/components/url-classifier/src/nsUrlClassifierStreamUpdater.h @@ -49,6 +49,7 @@ #include "nsTArray.h" #include "nsIBadCertListener2.h" #include "nsISSLErrorListener.h" +#include "nsITimer.h" // Forward declare pointers class nsIURI; @@ -59,7 +60,8 @@ class nsUrlClassifierStreamUpdater : public nsIUrlClassifierStreamUpdater, public nsIObserver, public nsIBadCertListener2, public nsISSLErrorListener, - public nsIInterfaceRequestor + public nsIInterfaceRequestor, + public nsITimerCallback { public: nsUrlClassifierStreamUpdater(); @@ -73,6 +75,7 @@ public: NS_DECL_NSIBADCERTLISTENER2 NS_DECL_NSISSLERRORLISTENER NS_DECL_NSIOBSERVER + NS_DECL_NSITIMERCALLBACK private: // No subclassing @@ -96,6 +99,8 @@ private: const nsACString &aTable, const nsACString &aServerMAC); + nsresult FetchNext(); + PRBool mIsUpdating; PRBool mInitialized; PRBool mDownloadError; @@ -105,6 +110,7 @@ private: nsCString mServerMAC; nsCOMPtr mChannel; nsCOMPtr mDBService; + nsCOMPtr mTimer; struct PendingUpdate { nsCString mUrl;