From aec27853c62536c31aac4f8faab00f7e62559eea Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 22 Dec 2024 15:55:22 +0100 Subject: [PATCH 1/6] Issue #2672 - Part 1: Load nsCookieService OMT and use sync reads. No more lazy-loading on the main thread, thanks. Everyone needs this. --- netwerk/base/nsIOService.cpp | 5 + netwerk/cookie/nsCookieService.cpp | 918 ++++++++++++----------------- netwerk/cookie/nsCookieService.h | 71 ++- 3 files changed, 426 insertions(+), 568 deletions(-) diff --git a/netwerk/base/nsIOService.cpp b/netwerk/base/nsIOService.cpp index 2baa89e905..2357d8b7ae 100644 --- a/netwerk/base/nsIOService.cpp +++ b/netwerk/base/nsIOService.cpp @@ -1437,6 +1437,11 @@ nsIOService::Observe(nsISupports *subject, nsCOMPtr prefBranch; GetPrefBranch(getter_AddRefs(prefBranch)); PrefsChanged(prefBranch, MANAGE_OFFLINE_STATUS_PREF); + + // Issue #2672 - Read cookie database at an early-as-possible time + // off main thread. Hence, we have more chance to finish db query + // before something calls into the cookie service. + nsCOMPtr cookieServ = do_GetService(NS_COOKIESERVICE_CONTRACTID); } } else if (!strcmp(topic, NS_XPCOM_SHUTDOWN_OBSERVER_ID)) { // Remember we passed XPCOM shutdown notification to prevent any diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index e493a70e1e..e78b50d527 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -28,6 +28,7 @@ #include "nsILineInputStream.h" #include "nsIEffectiveTLDService.h" #include "nsIIDNService.h" +#include "nsIThread.h" #include "mozIThirdPartyUtil.h" #include "nsTArray.h" @@ -75,8 +76,7 @@ static nsCookieService *gCookieService; #define COOKIES_FILE "cookies.sqlite" #define COOKIES_SCHEMA_VERSION 7 -// parameter indexes; see EnsureReadDomain, EnsureReadComplete and -// ReadCookieDBListener::HandleResult +// parameter indexes; see |Read| #define IDX_NAME 0 #define IDX_VALUE 1 #define IDX_HOST 2 @@ -447,92 +447,6 @@ public: NS_IMPL_ISUPPORTS(RemoveCookieDBListener, mozIStorageStatementCallback) -/****************************************************************************** - * ReadCookieDBListener impl: - * mozIStorageStatementCallback used to track asynchronous removal operations. - ******************************************************************************/ -class ReadCookieDBListener final : public DBListenerErrorHandler -{ -private: - const char *GetOpType() override { return "READ"; } - bool mCanceled; - - ~ReadCookieDBListener() = default; - -public: - NS_DECL_ISUPPORTS - - explicit ReadCookieDBListener(DBState* dbState) - : DBListenerErrorHandler(dbState) - , mCanceled(false) - { - } - - void Cancel() { mCanceled = true; } - - NS_IMETHOD HandleResult(mozIStorageResultSet *aResult) override - { - nsCOMPtr row; - - while (true) { - DebugOnly rv = aResult->GetNextRow(getter_AddRefs(row)); - NS_ASSERT_SUCCESS(rv); - - if (!row) - break; - - CookieDomainTuple *tuple = mDBState->hostArray.AppendElement(); - row->GetUTF8String(IDX_BASE_DOMAIN, tuple->key.mBaseDomain); - - nsAutoCString suffix; - row->GetUTF8String(IDX_ORIGIN_ATTRIBUTES, suffix); - DebugOnly success = tuple->key.mOriginAttributes.PopulateFromSuffix(suffix); - MOZ_ASSERT(success); - - tuple->cookie = - gCookieService->GetCookieFromRow(row, tuple->key.mOriginAttributes); - } - - return NS_OK; - } - NS_IMETHOD HandleCompletion(uint16_t aReason) override - { - // Process the completion of the read operation. If we have been canceled, - // we cannot assume that the cookieservice still has an open connection - // or that it even refers to the same database, so we must return early. - // Conversely, the cookieservice guarantees that if we have not been - // canceled, the database connection is still alive and we can safely - // operate on it. - - if (mCanceled) { - // We may receive a REASON_FINISHED after being canceled; - // tweak the reason accordingly. - aReason = mozIStorageStatementCallback::REASON_CANCELED; - } - - switch (aReason) { - case mozIStorageStatementCallback::REASON_FINISHED: - gCookieService->AsyncReadComplete(); - break; - case mozIStorageStatementCallback::REASON_CANCELED: - // Nothing more to do here. The partially read data has already been - // thrown away. - COOKIE_LOGSTRING(LogLevel::Debug, ("Read canceled")); - break; - case mozIStorageStatementCallback::REASON_ERROR: - // Nothing more to do here. DBListenerErrorHandler::HandleError() - // can handle it. - COOKIE_LOGSTRING(LogLevel::Debug, ("Read error")); - break; - default: - NS_NOTREACHED("invalid reason"); - } - return NS_OK; - } -}; - -NS_IMPL_ISUPPORTS(ReadCookieDBListener, mozIStorageStatementCallback) - /****************************************************************************** * CloseCookieDBListener imp: * Static mozIStorageCompletionCallback used to notify when the database is @@ -575,17 +489,6 @@ nsCookieEntry::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const return amount; } -size_t -CookieDomainTuple::SizeOfExcludingThis(MallocSizeOf aMallocSizeOf) const -{ - size_t amount = 0; - - amount += key.SizeOfExcludingThis(aMallocSizeOf); - amount += cookie->SizeOfIncludingThis(aMallocSizeOf); - - return amount; -} - size_t DBState::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const { @@ -593,11 +496,6 @@ DBState::SizeOfIncludingThis(MallocSizeOf aMallocSizeOf) const amount += aMallocSizeOf(this); amount += hostTable.SizeOfExcludingThis(aMallocSizeOf); - amount += hostArray.ShallowSizeOfExcludingThis(aMallocSizeOf); - for (uint32_t i = 0; i < hostArray.Length(); ++i) { - amount += hostArray[i].SizeOfExcludingThis(aMallocSizeOf); - } - amount += readSet.SizeOfExcludingThis(aMallocSizeOf); return amount; } @@ -664,6 +562,10 @@ nsCookieService::nsCookieService() , mMaxNumberOfCookies(kMaxNumberOfCookies) , mMaxCookiesPerHost(kMaxCookiesPerHost) , mCookiePurgeAge(kCookiePurgeAge) + , mThread(nullptr) + , mMonitor("CookieThread") + , mInitializedDBStates(false) + , mInitializedDBConn(false) { } @@ -695,6 +597,9 @@ nsCookieService::Init() mStorageService = do_GetService("@mozilla.org/storage/service;1", &rv); NS_ENSURE_SUCCESS(rv, rv); + rv = NS_NewNamedThread("Cookie", getter_AddRefs(mThread)); + NS_ENSURE_SUCCESS(rv, rv); + // Init our default, and possibly private DBStates. InitDBStates(); @@ -721,6 +626,7 @@ nsCookieService::InitDBStates() NS_ASSERTION(!mDBState, "already have a DBState"); NS_ASSERTION(!mDefaultDBState, "already have a default DBState"); NS_ASSERTION(!mPrivateDBState, "already have a private DBState"); + NS_ASSERTION(!mInitializedDBStates, "already initialized"); // Create a new default DBState and set our current one. mDefaultDBState = new DBState(); @@ -735,35 +641,62 @@ nsCookieService::InitDBStates() // We've already set up our DBStates appropriately; nothing more to do. COOKIE_LOGSTRING(LogLevel::Warning, ("InitDBStates(): couldn't get cookie file")); + + mInitializedDBConn = true; + mInitializedDBStates = true; return; } mDefaultDBState->cookieFile->AppendNative(NS_LITERAL_CSTRING(COOKIES_FILE)); - // Attempt to open and read the database. If TryInitDB() returns RESULT_RETRY, - // do so. - OpenDBResult result = TryInitDB(false); - if (result == RESULT_RETRY) { - // Database may be corrupt. Synchronously close the connection, clean up the - // default DBState, and try again. - COOKIE_LOGSTRING(LogLevel::Warning, ("InitDBStates(): retrying TryInitDB()")); - CleanupCachedStatements(); - CleanupDefaultDBConnection(); - result = TryInitDB(true); + nsCOMPtr runnable = NS_NewRunnableFunction([] { + NS_ENSURE_TRUE_VOID(gCookieService && + gCookieService->mDBState && + gCookieService->mDefaultDBState); + + MonitorAutoLock lock(gCookieService->mMonitor); + + // Attempt to open and read the database. If TryInitDB() returns RESULT_RETRY, + // do so. + OpenDBResult result = gCookieService->TryInitDB(false); if (result == RESULT_RETRY) { - // We're done. Change the code to failure so we clean up below. - result = RESULT_FAILURE; + // Database may be corrupt. Synchronously close the connection, clean up the + // default DBState, and try again. + COOKIE_LOGSTRING(LogLevel::Warning, ("InitDBStates(): retrying TryInitDB()")); + gCookieService->CleanupCachedStatements(); + gCookieService->CleanupDefaultDBConnection(); + result = gCookieService->TryInitDB(true); + if (result == RESULT_RETRY) { + // We're done. Change the code to failure so we clean up below. + result = RESULT_FAILURE; + } } - } - if (result == RESULT_FAILURE) { - COOKIE_LOGSTRING(LogLevel::Warning, - ("InitDBStates(): TryInitDB() failed, closing connection")); - // Connection failure is unrecoverable. Clean up our connection. We can run - // fine without persistent storage -- e.g. if there's no profile. - CleanupCachedStatements(); - CleanupDefaultDBConnection(); - } + if (result == RESULT_FAILURE) { + COOKIE_LOGSTRING(LogLevel::Warning, + ("InitDBStates(): TryInitDB() failed, closing connection")); + + // Connection failure is unrecoverable. Clean up our connection. We can run + // fine without persistent storage -- e.g. if there's no profile. + gCookieService->CleanupCachedStatements(); + gCookieService->CleanupDefaultDBConnection(); + + // No need to initialize dbConn + gCookieService->mInitializedDBConn = true; + } + + gCookieService->mInitializedDBStates = true; + + NS_DispatchToMainThread( + NS_NewRunnableFunction([] { + NS_ENSURE_TRUE_VOID(gCookieService); + gCookieService->InitDBConn(); + }) + ); + gCookieService->mMonitor.Notify(); + }); + + mThread->Dispatch(runnable, NS_DISPATCH_NORMAL); } namespace { @@ -889,6 +822,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) NS_ASSERTION(!mDefaultDBState->stmtInsert, "nonnull stmtInsert"); NS_ASSERTION(!mDefaultDBState->insertListener, "nonnull insertListener"); NS_ASSERTION(!mDefaultDBState->syncConn, "nonnull syncConn"); + NS_ASSERTION(NS_GetCurrentThread() == mThread, "not on cookie thread"); // Ditch an existing db, if we've been told to (i.e. it's corrupt). We don't // want to delete it outright, since it may be useful for debugging purposes, @@ -908,20 +842,11 @@ nsCookieService::TryInitDB(bool aRecreateDB) // and statements upon success. The connection is opened unshared to eliminate // cache contention between the main and background threads. rv = mStorageService->OpenUnsharedDatabase(mDefaultDBState->cookieFile, - getter_AddRefs(mDefaultDBState->dbConn)); + getter_AddRefs(mDefaultDBState->syncConn)); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - // Set up our listeners. - mDefaultDBState->insertListener = new InsertCookieDBListener(mDefaultDBState); - mDefaultDBState->updateListener = new UpdateCookieDBListener(mDefaultDBState); - mDefaultDBState->removeListener = new RemoveCookieDBListener(mDefaultDBState); - mDefaultDBState->closeListener = new CloseCookieDBListener(mDefaultDBState); - - // Grow cookie db in 512KB increments - mDefaultDBState->dbConn->SetGrowthIncrement(512 * 1024, EmptyCString()); - bool tableExists = false; - mDefaultDBState->dbConn->TableExists(NS_LITERAL_CSTRING("moz_cookies"), + mDefaultDBState->syncConn->TableExists(NS_LITERAL_CSTRING("moz_cookies"), &tableExists); if (!tableExists) { rv = CreateTable(); @@ -930,11 +855,11 @@ nsCookieService::TryInitDB(bool aRecreateDB) } else { // table already exists; check the schema version before reading int32_t dbSchemaVersion; - rv = mDefaultDBState->dbConn->GetSchemaVersion(&dbSchemaVersion); + rv = mDefaultDBState->syncConn->GetSchemaVersion(&dbSchemaVersion); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Start a transaction for the whole migration block. - mozStorageTransaction transaction(mDefaultDBState->dbConn, true); + mozStorageTransaction transaction(mDefaultDBState->syncConn, true); switch (dbSchemaVersion) { // Upgrading. @@ -946,7 +871,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) case 1: { // Add the lastAccessed column to the table. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies ADD lastAccessed INTEGER")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); } @@ -956,7 +881,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) case 2: { // Add the baseDomain column and index to the table. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies ADD baseDomain TEXT")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -966,12 +891,12 @@ nsCookieService::TryInitDB(bool aRecreateDB) const int64_t SCHEMA2_IDX_ID = 0; const int64_t SCHEMA2_IDX_HOST = 1; nsCOMPtr select; - rv = mDefaultDBState->dbConn->CreateStatement(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( "SELECT id, host FROM moz_cookies"), getter_AddRefs(select)); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); nsCOMPtr update; - rv = mDefaultDBState->dbConn->CreateStatement(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( "UPDATE moz_cookies SET baseDomain = :baseDomain WHERE id = :id"), getter_AddRefs(update)); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1005,7 +930,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) } // Create an index on baseDomain. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE INDEX moz_basedomain ON moz_cookies (baseDomain)")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); } @@ -1030,14 +955,14 @@ nsCookieService::TryInitDB(bool aRecreateDB) const int64_t SCHEMA3_IDX_HOST = 2; const int64_t SCHEMA3_IDX_PATH = 3; nsCOMPtr select; - rv = mDefaultDBState->dbConn->CreateStatement(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( "SELECT id, name, host, path FROM moz_cookies " "ORDER BY name ASC, host ASC, path ASC, expiry ASC"), getter_AddRefs(select)); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); nsCOMPtr deleteExpired; - rv = mDefaultDBState->dbConn->CreateStatement(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( "DELETE FROM moz_cookies WHERE id = :id"), getter_AddRefs(deleteExpired)); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1090,18 +1015,18 @@ nsCookieService::TryInitDB(bool aRecreateDB) } // Add the creationTime column to the table. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies ADD creationTime INTEGER")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Copy the id of each row into the new creationTime column. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "UPDATE moz_cookies SET creationTime = " "(SELECT id WHERE id = moz_cookies.id)")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Create a unique index on (name, host, path) to allow fast lookup. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE UNIQUE INDEX moz_uniqueid " "ON moz_cookies (name, host, path)")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1123,12 +1048,12 @@ nsCookieService::TryInitDB(bool aRecreateDB) // the only namespace used by a non-Firefox-OS implementation. // Rename existing table - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies RENAME TO moz_cookies_old")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Drop existing index (CreateTable will create new one for new table) - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "DROP INDEX moz_basedomain")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1137,7 +1062,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Copy data from old table, using appId/inBrowser=0 for existing rows - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "INSERT INTO moz_cookies " "(baseDomain, appId, inBrowserElement, name, value, host, path, expiry," " lastAccessed, creationTime, isSecure, isHttpOnly) " @@ -1173,12 +1098,12 @@ nsCookieService::TryInitDB(bool aRecreateDB) // inBrowserElement to originAttributes in the meantime. // Rename existing table. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies RENAME TO moz_cookies_old")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Drop existing index (CreateTable will create new one for new table). - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "DROP INDEX moz_basedomain")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1195,11 +1120,11 @@ nsCookieService::TryInitDB(bool aRecreateDB) NS_NAMED_LITERAL_CSTRING(convertToOriginAttrsName, "CONVERT_TO_ORIGIN_ATTRIBUTES"); - rv = mDefaultDBState->dbConn->CreateFunction(convertToOriginAttrsName, + rv = mDefaultDBState->syncConn->CreateFunction(convertToOriginAttrsName, 2, convertToOriginAttrs); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "INSERT INTO moz_cookies " "(baseDomain, originAttributes, name, value, host, path, expiry," " lastAccessed, creationTime, isSecure, isHttpOnly) " @@ -1210,11 +1135,11 @@ nsCookieService::TryInitDB(bool aRecreateDB) "FROM moz_cookies_old")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - rv = mDefaultDBState->dbConn->RemoveFunction(convertToOriginAttrsName); + rv = mDefaultDBState->syncConn->RemoveFunction(convertToOriginAttrsName); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Drop old table - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "DROP TABLE moz_cookies_old")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1233,11 +1158,11 @@ nsCookieService::TryInitDB(bool aRecreateDB) // This version simply restores appId and inBrowserElement columns in // order to fix downgrading issue even though these two columns are no // longer used in the latest schema. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies ADD appId INTEGER DEFAULT 0;")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "ALTER TABLE moz_cookies ADD inBrowserElement INTEGER DEFAULT 0;")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1249,7 +1174,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) NS_NAMED_LITERAL_CSTRING(setAppIdName, "SET_APP_ID"); - rv = mDefaultDBState->dbConn->CreateFunction(setAppIdName, 1, setAppId); + rv = mDefaultDBState->syncConn->CreateFunction(setAppIdName, 1, setAppId); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); nsCOMPtr @@ -1258,20 +1183,20 @@ nsCookieService::TryInitDB(bool aRecreateDB) NS_NAMED_LITERAL_CSTRING(setInBrowserName, "SET_IN_BROWSER"); - rv = mDefaultDBState->dbConn->CreateFunction(setInBrowserName, 1, + rv = mDefaultDBState->syncConn->CreateFunction(setInBrowserName, 1, setInBrowser); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "UPDATE moz_cookies SET appId = SET_APP_ID(originAttributes), " "inBrowserElement = SET_IN_BROWSER(originAttributes);" )); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - rv = mDefaultDBState->dbConn->RemoveFunction(setAppIdName); + rv = mDefaultDBState->syncConn->RemoveFunction(setAppIdName); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - rv = mDefaultDBState->dbConn->RemoveFunction(setInBrowserName); + rv = mDefaultDBState->syncConn->RemoveFunction(setInBrowserName); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); COOKIE_LOGSTRING(LogLevel::Debug, @@ -1279,7 +1204,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) } // No more upgrades. Update the schema version. - rv = mDefaultDBState->dbConn->SetSchemaVersion(COOKIES_SCHEMA_VERSION); + rv = mDefaultDBState->syncConn->SetSchemaVersion(COOKIES_SCHEMA_VERSION); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); MOZ_FALLTHROUGH; @@ -1295,7 +1220,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) // below, by verifying the columns we care about are all there. for now, // re-set the schema version in the db, in case the checks succeed (if // they don't, we're dropping the table anyway). - rv = mDefaultDBState->dbConn->SetSchemaVersion(COOKIES_SCHEMA_VERSION); + rv = mDefaultDBState->syncConn->SetSchemaVersion(COOKIES_SCHEMA_VERSION); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); } // fall through to downgrade check @@ -1311,7 +1236,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) { // check if all the expected columns exist nsCOMPtr stmt; - rv = mDefaultDBState->dbConn->CreateStatement(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( "SELECT " "id, " "baseDomain, " @@ -1330,7 +1255,7 @@ nsCookieService::TryInitDB(bool aRecreateDB) break; // our columns aren't there - drop the table! - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "DROP TABLE moz_cookies")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); @@ -1341,6 +1266,113 @@ nsCookieService::TryInitDB(bool aRecreateDB) } } + // if we deleted a corrupt db, don't attempt to import - return now + if (aRecreateDB) { + return RESULT_OK; + } + + // check whether to import or just read in the db + if (tableExists) { + return Read(); + } + + nsCOMPtr runnable = + NS_NewRunnableFunction([] { + NS_ENSURE_TRUE_VOID(gCookieService); + NS_ENSURE_TRUE_VOID(gCookieService->mDefaultDBState); + nsCOMPtr oldCookieFile; + nsresult rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, + getter_AddRefs(oldCookieFile)); + if (NS_FAILED(rv)) { + return; + } + + // Import cookies, and clean up the old file regardless of success or failure. + // Note that we have to switch out our DBState temporarily, in case we're in + // private browsing mode; otherwise ImportCookies() won't be happy. + DBState* initialState = gCookieService->mDBState; + gCookieService->mDBState = gCookieService->mDefaultDBState; + oldCookieFile->AppendNative(NS_LITERAL_CSTRING(OLD_COOKIE_FILE_NAME)); + gCookieService->ImportCookies(oldCookieFile); + oldCookieFile->Remove(false); + gCookieService->mDBState = initialState; + }); + + NS_DispatchToMainThread(runnable); + + return RESULT_OK; +} + +void +nsCookieService::InitDBConn() +{ + MOZ_ASSERT(NS_IsMainThread()); + + // We should skip InitDBConn if we close profile during initializing DBStates + // and then InitDBConn is called after we close the DBStates. + if (!mInitializedDBStates || mInitializedDBConn || !mDefaultDBState) { + return; + } + + for (uint32_t i = 0; i < mReadArray.Length(); ++i) { + CookieDomainTuple& tuple = mReadArray[i]; + RefPtr cookie = nsCookie::Create(tuple.cookie->name, + tuple.cookie->value, + tuple.cookie->host, + tuple.cookie->path, + tuple.cookie->expiry, + tuple.cookie->lastAccessed, + tuple.cookie->creationTime, + false, + tuple.cookie->isSecure, + tuple.cookie->isHttpOnly, + tuple.cookie->originAttributes); + + AddCookieToList(tuple.key, cookie, mDefaultDBState, nullptr, false); + } + + if (NS_FAILED(InitDBConnInternal())) { + COOKIE_LOGSTRING(LogLevel::Warning, ("InitDBConn(): retrying InitDBConnInternal()")); + CleanupCachedStatements(); + CleanupDefaultDBConnection(); + if (NS_FAILED(InitDBConnInternal())) { + COOKIE_LOGSTRING(LogLevel::Warning, + ("InitDBConn(): InitDBConnInternal() failed, closing connection")); + + // Game over, clean the connections. + CleanupCachedStatements(); + CleanupDefaultDBConnection(); + } + } + mInitializedDBConn = true; + + COOKIE_LOGSTRING(LogLevel::Debug, ("InitDBConn(): mInitializedDBConn = true")); + + nsCOMPtr os = mozilla::services::GetObserverService(); + if (os && !mReadArray.IsEmpty()) { + os->NotifyObservers(nullptr, "cookie-db-read", nullptr); + mReadArray.Clear(); + } +} + +nsresult +nsCookieService::InitDBConnInternal() +{ + MOZ_ASSERT(NS_IsMainThread()); + + nsresult rv = mStorageService->OpenUnsharedDatabase(mDefaultDBState->cookieFile, + getter_AddRefs(mDefaultDBState->dbConn)); + NS_ENSURE_SUCCESS(rv, rv); + + // Set up our listeners. + mDefaultDBState->insertListener = new InsertCookieDBListener(mDefaultDBState); + mDefaultDBState->updateListener = new UpdateCookieDBListener(mDefaultDBState); + mDefaultDBState->removeListener = new RemoveCookieDBListener(mDefaultDBState); + mDefaultDBState->closeListener = new CloseCookieDBListener(mDefaultDBState); + + // Grow cookie db in 512KB increments + mDefaultDBState->dbConn->SetGrowthIncrement(512 * 1024, EmptyCString()); + // make operations on the table asynchronous, for performance mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "PRAGMA synchronous = OFF")); @@ -1380,44 +1412,20 @@ nsCookieService::TryInitDB(bool aRecreateDB) ":isHttpOnly" ")"), getter_AddRefs(mDefaultDBState->stmtInsert)); - NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + NS_ENSURE_SUCCESS(rv, rv); rv = mDefaultDBState->dbConn->CreateAsyncStatement(NS_LITERAL_CSTRING( "DELETE FROM moz_cookies " "WHERE name = :name AND host = :host AND path = :path"), getter_AddRefs(mDefaultDBState->stmtDelete)); - NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + NS_ENSURE_SUCCESS(rv, rv); rv = mDefaultDBState->dbConn->CreateAsyncStatement(NS_LITERAL_CSTRING( "UPDATE moz_cookies SET lastAccessed = :lastAccessed " "WHERE name = :name AND host = :host AND path = :path"), getter_AddRefs(mDefaultDBState->stmtUpdate)); - NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - // if we deleted a corrupt db, don't attempt to import - return now - if (aRecreateDB) - return RESULT_OK; - - // check whether to import or just read in the db - if (tableExists) - return Read(); - - nsCOMPtr oldCookieFile; - rv = NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR, - getter_AddRefs(oldCookieFile)); - if (NS_FAILED(rv)) return RESULT_OK; - - // Import cookies, and clean up the old file regardless of success or failure. - // Note that we have to switch out our DBState temporarily, in case we're in - // private browsing mode; otherwise ImportCookies() won't be happy. - DBState* initialState = mDBState; - mDBState = mDefaultDBState; - oldCookieFile->AppendNative(NS_LITERAL_CSTRING(OLD_COOKIE_FILE_NAME)); - ImportCookies(oldCookieFile); - oldCookieFile->Remove(false); - mDBState = initialState; - - return RESULT_OK; + return rv; } // Sets the schema version and creates the moz_cookies table. @@ -1425,7 +1433,7 @@ nsresult nsCookieService::CreateTable() { // Set the schema version, before creating the table. - nsresult rv = mDefaultDBState->dbConn->SetSchemaVersion( + nsresult rv = mDefaultDBState->syncConn->SetSchemaVersion( COOKIES_SCHEMA_VERSION); if (NS_FAILED(rv)) return rv; @@ -1433,7 +1441,7 @@ nsCookieService::CreateTable() // We default originAttributes to empty string: this is so if users revert to // an older Firefox version that doesn't know about this field, any cookies // set will still work once they upgrade back. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE TABLE moz_cookies (" "id INTEGER PRIMARY KEY, " "baseDomain TEXT, " @@ -1454,7 +1462,7 @@ nsCookieService::CreateTable() if (NS_FAILED(rv)) return rv; // Create an index on baseDomain. - return mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + return mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE INDEX moz_basedomain ON moz_cookies (baseDomain, " "originAttributes)")); } @@ -1464,14 +1472,14 @@ nsresult nsCookieService::CreateTableForSchemaVersion6() { // Set the schema version, before creating the table. - nsresult rv = mDefaultDBState->dbConn->SetSchemaVersion(6); + nsresult rv = mDefaultDBState->syncConn->SetSchemaVersion(6); if (NS_FAILED(rv)) return rv; // Create the table. // We default originAttributes to empty string: this is so if users revert to // an older Firefox version that doesn't know about this field, any cookies // set will still work once they upgrade back. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE TABLE moz_cookies (" "id INTEGER PRIMARY KEY, " "baseDomain TEXT, " @@ -1490,7 +1498,7 @@ nsCookieService::CreateTableForSchemaVersion6() if (NS_FAILED(rv)) return rv; // Create an index on baseDomain. - return mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + return mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE INDEX moz_basedomain ON moz_cookies (baseDomain, " "originAttributes)")); } @@ -1500,13 +1508,13 @@ nsresult nsCookieService::CreateTableForSchemaVersion5() { // Set the schema version, before creating the table. - nsresult rv = mDefaultDBState->dbConn->SetSchemaVersion(5); + nsresult rv = mDefaultDBState->syncConn->SetSchemaVersion(5); if (NS_FAILED(rv)) return rv; // Create the table. We default appId/inBrowserElement to 0: this is so if // users revert to an older Firefox version that doesn't know about these // fields, any cookies set will still work once they upgrade back. - rv = mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE TABLE moz_cookies (" "id INTEGER PRIMARY KEY, " "baseDomain TEXT, " @@ -1526,7 +1534,7 @@ nsCookieService::CreateTableForSchemaVersion5() if (NS_FAILED(rv)) return rv; // Create an index on baseDomain. - return mDefaultDBState->dbConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + return mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( "CREATE INDEX moz_basedomain ON moz_cookies (baseDomain, " "appId, " "inBrowserElement)")); @@ -1535,6 +1543,13 @@ nsCookieService::CreateTableForSchemaVersion5() void nsCookieService::CloseDBStates() { + // return if we already closed + if (!mDBState) { + return; + } + + EnsureReadComplete(false); + // Null out our private and pointer DBStates regardless. mPrivateDBState = nullptr; mDBState = nullptr; @@ -1547,12 +1562,6 @@ nsCookieService::CloseDBStates() CleanupCachedStatements(); if (mDefaultDBState->dbConn) { - // Cancel any pending read. No further results will be received by our - // read listener. - if (mDefaultDBState->pendingRead) { - CancelAsyncRead(true); - } - // Asynchronously close the connection. We will null it below. mDefaultDBState->dbConn->AsyncClose(mDefaultDBState->closeListener); } @@ -1560,6 +1569,7 @@ nsCookieService::CloseDBStates() CleanupDefaultDBConnection(); mDefaultDBState = nullptr; + mInitializedDBStates = false; } // Null out the statements. @@ -1592,11 +1602,12 @@ nsCookieService::CleanupDefaultDBConnection() // Manually null out our listeners. This is necessary because they hold a // strong ref to the DBState itself. They'll stay alive until whatever // statements are still executing complete. - mDefaultDBState->readListener = nullptr; mDefaultDBState->insertListener = nullptr; mDefaultDBState->updateListener = nullptr; mDefaultDBState->removeListener = nullptr; mDefaultDBState->closeListener = nullptr; + + mInitializedDBConn = false; } void @@ -1663,16 +1674,6 @@ nsCookieService::HandleCorruptDB(DBState* aDBState) // Move to 'closing' state. mDefaultDBState->corruptFlag = DBState::CLOSING_FOR_REBUILD; - // Cancel any pending read and close the database. If we do have an - // in-flight read we want to throw away all the results so far -- we have no - // idea how consistent the database is. Note that we may have already - // canceled the read but not emptied our readSet; do so now. - mDefaultDBState->readSet.Clear(); - if (mDefaultDBState->pendingRead) { - CancelAsyncRead(true); - mDefaultDBState->syncConn = nullptr; - } - CleanupCachedStatements(); mDefaultDBState->dbConn->AsyncClose(mDefaultDBState->closeListener); CleanupDefaultDBConnection(); @@ -1723,61 +1724,80 @@ nsCookieService::RebuildCorruptDB(DBState* aDBState) COOKIE_LOGSTRING(LogLevel::Debug, ("RebuildCorruptDB(): creating new database")); - // The database has been closed, and we're ready to rebuild. Open a - // connection. - OpenDBResult result = TryInitDB(true); - if (result != RESULT_OK) { - // We're done. Reset our DB connection and statements, and notify of - // closure. - COOKIE_LOGSTRING(LogLevel::Warning, - ("RebuildCorruptDB(): TryInitDB() failed with result %u", result)); - CleanupCachedStatements(); - CleanupDefaultDBConnection(); - mDefaultDBState->corruptFlag = DBState::OK; - if (os) { - os->NotifyObservers(nullptr, "cookie-db-closed", nullptr); - } - return; - } + nsCOMPtr runnable = + NS_NewRunnableFunction([] { + NS_ENSURE_TRUE_VOID(gCookieService && gCookieService->mDefaultDBState); - // Notify observers that we're beginning the rebuild. - if (os) { - os->NotifyObservers(nullptr, "cookie-db-rebuilding", nullptr); - } + // The database has been closed, and we're ready to rebuild. Open a + // connection. + OpenDBResult result = gCookieService->TryInitDB(true); - // Enumerate the hash, and add cookies to the params array. - mozIStorageAsyncStatement* stmt = aDBState->stmtInsert; - nsCOMPtr paramsArray; - stmt->NewBindingParamsArray(getter_AddRefs(paramsArray)); - for (auto iter = aDBState->hostTable.Iter(); !iter.Done(); iter.Next()) { - nsCookieEntry* entry = iter.Get(); + nsCOMPtr innerRunnable = + NS_NewRunnableFunction([result] { + NS_ENSURE_TRUE_VOID(gCookieService && gCookieService->mDefaultDBState); - const nsCookieEntry::ArrayType& cookies = entry->GetCookies(); - for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) { - nsCookie* cookie = cookies[i]; + nsCOMPtr os = mozilla::services::GetObserverService(); + if (result != RESULT_OK) { + // We're done. Reset our DB connection and statements, and notify of + // closure. + COOKIE_LOGSTRING(LogLevel::Warning, + ("RebuildCorruptDB(): TryInitDB() failed with result %u", result)); + gCookieService->CleanupCachedStatements(); + gCookieService->CleanupDefaultDBConnection(); + gCookieService->mDefaultDBState->corruptFlag = DBState::OK; + if (os) { + os->NotifyObservers(nullptr, "cookie-db-closed", nullptr); + } + return; + } - if (!cookie->IsSession()) { - bindCookieParameters(paramsArray, nsCookieKey(entry), cookie); - } - } - } + // Notify observers that we're beginning the rebuild. + if (os) { + os->NotifyObservers(nullptr, "cookie-db-rebuilding", nullptr); + } - // Make sure we've got something to write. If we don't, we're done. - uint32_t length; - paramsArray->GetLength(&length); - if (length == 0) { - COOKIE_LOGSTRING(LogLevel::Debug, - ("RebuildCorruptDB(): nothing to write, rebuild complete")); - mDefaultDBState->corruptFlag = DBState::OK; - return; - } + gCookieService->InitDBConn(); - // Execute the statement. If any errors crop up, we won't try again. - DebugOnly rv = stmt->BindParameters(paramsArray); - NS_ASSERT_SUCCESS(rv); - nsCOMPtr handle; - rv = stmt->ExecuteAsync(aDBState->insertListener, getter_AddRefs(handle)); - NS_ASSERT_SUCCESS(rv); + // Enumerate the hash, and add cookies to the params array. + mozIStorageAsyncStatement* stmt = gCookieService->mDefaultDBState->stmtInsert; + nsCOMPtr paramsArray; + stmt->NewBindingParamsArray(getter_AddRefs(paramsArray)); + for (auto iter = gCookieService->mDefaultDBState->hostTable.Iter(); + !iter.Done(); + iter.Next()) { + nsCookieEntry* entry = iter.Get(); + + const nsCookieEntry::ArrayType& cookies = entry->GetCookies(); + for (nsCookieEntry::IndexType i = 0; i < cookies.Length(); ++i) { + nsCookie* cookie = cookies[i]; + + if (!cookie->IsSession()) { + bindCookieParameters(paramsArray, nsCookieKey(entry), cookie); + } + } + } + + // Make sure we've got something to write. If we don't, we're done. + uint32_t length; + paramsArray->GetLength(&length); + if (length == 0) { + COOKIE_LOGSTRING(LogLevel::Debug, + ("RebuildCorruptDB(): nothing to write, rebuild complete")); + gCookieService->mDefaultDBState->corruptFlag = DBState::OK; + return; + } + + // Execute the statement. If any errors crop up, we won't try again. + DebugOnly rv = stmt->BindParameters(paramsArray); + NS_ASSERT_SUCCESS(rv); + nsCOMPtr handle; + rv = stmt->ExecuteAsync(gCookieService->mDefaultDBState->insertListener, + getter_AddRefs(handle)); + NS_ASSERT_SUCCESS(rv); + }); + NS_DispatchToMainThread(innerRunnable); + }); + mThread->Dispatch(runnable, NS_DISPATCH_NORMAL); } nsCookieService::~nsCookieService() @@ -1787,6 +1807,9 @@ nsCookieService::~nsCookieService() UnregisterWeakMemoryReporter(this); gCookieService = nullptr; + if (mThread) { + mThread->Shutdown(); + } } NS_IMETHODIMP @@ -1978,6 +2001,8 @@ nsCookieService::SetCookieStringInternal(nsIURI *aHostURI, return; } + EnsureReadComplete(true); + AutoRestore savePrevDBState(mDBState); mDBState = aIsPrivate ? mPrivateDBState : mDefaultDBState; @@ -2182,18 +2207,14 @@ nsCookieService::RemoveAll() return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + RemoveAllFromMemory(); // clear the cookie file if (mDBState->dbConn) { NS_ASSERTION(mDBState == mDefaultDBState, "not in default DB state"); - // Cancel any pending read. No further results will be received by our - // read listener. - if (mDefaultDBState->pendingRead) { - CancelAsyncRead(true); - } - nsCOMPtr stmt; nsresult rv = mDefaultDBState->dbConn->CreateAsyncStatement(NS_LITERAL_CSTRING( "DELETE FROM moz_cookies"), getter_AddRefs(stmt)); @@ -2222,7 +2243,7 @@ nsCookieService::GetEnumerator(nsISimpleEnumerator **aEnumerator) return NS_ERROR_NOT_AVAILABLE; } - EnsureReadComplete(); + EnsureReadComplete(true); nsCOMArray cookieList(mDBState->cookieCount); for (auto iter = mDBState->hostTable.Iter(); !iter.Done(); iter.Next()) { @@ -2321,6 +2342,8 @@ nsCookieService::AddNative(const nsACString &aHost, NS_WARNING("No DBState! Profile already closed?"); return NS_ERROR_NOT_AVAILABLE; } + + EnsureReadComplete(true); // first, normalize the hostname, and fail if it contains illegal characters. nsAutoCString host(aHost); @@ -2364,6 +2387,8 @@ nsCookieService::Remove(const nsACString& aHost, const NeckoOriginAttributes& aA return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + // first, normalize the hostname, and fail if it contains illegal characters. nsAutoCString host(aHost); nsresult rv = NormalizeHost(host); @@ -2473,69 +2498,10 @@ nsCookieService::UsePrivateMode(bool aIsPrivate, * private file I/O functions ******************************************************************************/ -// Begin an asynchronous read from the database. -OpenDBResult -nsCookieService::Read() -{ - // Set up a statement for the read. Note that our query specifies that - // 'baseDomain' not be nullptr -- see below for why. - nsCOMPtr stmtRead; - nsresult rv = mDefaultDBState->dbConn->CreateAsyncStatement(NS_LITERAL_CSTRING( - "SELECT " - "name, " - "value, " - "host, " - "path, " - "expiry, " - "lastAccessed, " - "creationTime, " - "isSecure, " - "isHttpOnly, " - "baseDomain, " - "originAttributes " - "FROM moz_cookies " - "WHERE baseDomain NOTNULL"), getter_AddRefs(stmtRead)); - NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - - // Set up a statement to delete any rows with a nullptr 'baseDomain' - // column. This takes care of any cookies set by browsers that don't - // understand the 'baseDomain' column, where the database schema version - // is from one that does. (This would occur when downgrading.) - nsCOMPtr stmtDeleteNull; - rv = mDefaultDBState->dbConn->CreateAsyncStatement(NS_LITERAL_CSTRING( - "DELETE FROM moz_cookies WHERE baseDomain ISNULL"), - getter_AddRefs(stmtDeleteNull)); - NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - - // Start a new connection for sync reads, to reduce contention with the - // background thread. We need to do this before we kick off write statements, - // since they can lock the database and prevent connections from being opened. - rv = mStorageService->OpenUnsharedDatabase(mDefaultDBState->cookieFile, - getter_AddRefs(mDefaultDBState->syncConn)); - NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - - // Init our readSet hash and execute the statements. Note that, after this - // point, we cannot fail without altering the cleanup code in InitDBStates() - // to handle closing of the now-asynchronous connection. - mDefaultDBState->hostArray.SetCapacity(kMaxNumberOfCookies); - - mDefaultDBState->readListener = new ReadCookieDBListener(mDefaultDBState); - rv = stmtRead->ExecuteAsync(mDefaultDBState->readListener, - getter_AddRefs(mDefaultDBState->pendingRead)); - NS_ASSERT_SUCCESS(rv); - - nsCOMPtr handle; - rv = stmtDeleteNull->ExecuteAsync(mDefaultDBState->removeListener, - getter_AddRefs(handle)); - NS_ASSERT_SUCCESS(rv); - - return RESULT_OK; -} - // Extract data from a single result row and create an nsCookie. -// This is templated since 'T' is different for sync vs async results. -template nsCookie* -nsCookieService::GetCookieFromRow(T &aRow, const OriginAttributes& aOriginAttributes) +mozilla::UniquePtr +nsCookieService::GetCookieFromRow(mozIStorageStatement *aRow, + const OriginAttributes &aOriginAttributes) { // Skip reading 'baseDomain' -- up to the caller. nsCString name, value, host, path; @@ -2554,197 +2520,55 @@ nsCookieService::GetCookieFromRow(T &aRow, const OriginAttributes& aOriginAttrib bool isSecure = 0 != aRow->AsInt32(IDX_SECURE); bool isHttpOnly = 0 != aRow->AsInt32(IDX_HTTPONLY); - // Create a new nsCookie and assign the data. - return nsCookie::Create(name, value, host, path, - expiry, - lastAccessed, - creationTime, - false, - isSecure, - isHttpOnly, - aOriginAttributes); + // Create a new constCookie and assign the data. + return mozilla::MakeUnique(name, + value, + host, + path, + expiry, + lastAccessed, + creationTime, + isSecure, + isHttpOnly, + aOriginAttributes); } void -nsCookieService::AsyncReadComplete() +nsCookieService::EnsureReadComplete(bool aInitDBConn) { - // We may be in the private browsing DB state, with a pending read on the - // default DB state. (This would occur if we started up in private browsing - // mode.) As long as we do all our operations on the default state, we're OK. - NS_ASSERTION(mDefaultDBState, "no default DBState"); - NS_ASSERTION(mDefaultDBState->pendingRead, "no pending read"); - NS_ASSERTION(mDefaultDBState->readListener, "no read listener"); + MOZ_ASSERT(NS_IsMainThread()); - // Merge the data read on the background thread with the data synchronously - // read on the main thread. Note that transactions on the cookie table may - // have occurred on the main thread since, making the background data stale. - for (uint32_t i = 0; i < mDefaultDBState->hostArray.Length(); ++i) { - const CookieDomainTuple &tuple = mDefaultDBState->hostArray[i]; + if (!mInitializedDBStates) { + TimeStamp startBlockTime = TimeStamp::Now(); + MonitorAutoLock lock(mMonitor); - // Tiebreak: if the given base domain has already been read in, ignore - // the background data. Note that readSet may contain domains that were - // queried but found not to be in the db -- that's harmless. - if (mDefaultDBState->readSet.GetEntry(tuple.key)) - continue; - - AddCookieToList(tuple.key, tuple.cookie, mDefaultDBState, nullptr, false); - } - - mDefaultDBState->stmtReadDomain = nullptr; - mDefaultDBState->pendingRead = nullptr; - mDefaultDBState->readListener = nullptr; - mDefaultDBState->syncConn = nullptr; - mDefaultDBState->hostArray.Clear(); - mDefaultDBState->readSet.Clear(); - - COOKIE_LOGSTRING(LogLevel::Debug, ("Read(): %ld cookies read", - mDefaultDBState->cookieCount)); - - nsCOMPtr os = mozilla::services::GetObserverService(); - if (os) { - os->NotifyObservers(nullptr, "cookie-db-read", nullptr); - } -} - -void -nsCookieService::CancelAsyncRead(bool aPurgeReadSet) -{ - // We may be in the private browsing DB state, with a pending read on the - // default DB state. (This would occur if we started up in private browsing - // mode.) As long as we do all our operations on the default state, we're OK. - NS_ASSERTION(mDefaultDBState, "no default DBState"); - NS_ASSERTION(mDefaultDBState->pendingRead, "no pending read"); - NS_ASSERTION(mDefaultDBState->readListener, "no read listener"); - - // Cancel the pending read, kill the read listener, and empty the array - // of data already read in on the background thread. - mDefaultDBState->readListener->Cancel(); - DebugOnly rv = mDefaultDBState->pendingRead->Cancel(); - NS_ASSERT_SUCCESS(rv); - - mDefaultDBState->stmtReadDomain = nullptr; - mDefaultDBState->pendingRead = nullptr; - mDefaultDBState->readListener = nullptr; - mDefaultDBState->hostArray.Clear(); - - // Only clear the 'readSet' table if we no longer need to know what set of - // data is already accounted for. - if (aPurgeReadSet) - mDefaultDBState->readSet.Clear(); -} - -void -nsCookieService::EnsureReadDomain(const nsCookieKey &aKey) -{ - NS_ASSERTION(!mDBState->dbConn || mDBState == mDefaultDBState, - "not in default db state"); - - // Fast path 1: nothing to read, or we've already finished reading. - if (MOZ_LIKELY(!mDBState->dbConn || !mDefaultDBState->pendingRead)) - return; - - // Fast path 2: already read in this particular domain. - if (MOZ_LIKELY(mDefaultDBState->readSet.GetEntry(aKey))) - return; - - // Read in the data synchronously. - // see IDX_NAME, etc. for parameter indexes - nsresult rv; - if (!mDefaultDBState->stmtReadDomain) { - // Cache the statement, since it's likely to be used again. - rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( - "SELECT " - "name, " - "value, " - "host, " - "path, " - "expiry, " - "lastAccessed, " - "creationTime, " - "isSecure, " - "isHttpOnly " - "FROM moz_cookies " - "WHERE baseDomain = :baseDomain " - " AND originAttributes = :originAttributes"), - getter_AddRefs(mDefaultDBState->stmtReadDomain)); - - if (NS_FAILED(rv)) { - // Recreate the database. - COOKIE_LOGSTRING(LogLevel::Debug, - ("EnsureReadDomain(): corruption detected when creating statement " - "with rv 0x%x", rv)); - HandleCorruptDB(mDefaultDBState); - return; + while (!mInitializedDBStates) { + mMonitor.Wait(); } } - NS_ASSERTION(mDefaultDBState->syncConn, "should have a sync db connection"); - - mozStorageStatementScoper scoper(mDefaultDBState->stmtReadDomain); - - rv = mDefaultDBState->stmtReadDomain->BindUTF8StringByName( - NS_LITERAL_CSTRING("baseDomain"), aKey.mBaseDomain); - NS_ASSERT_SUCCESS(rv); - - nsAutoCString suffix; - aKey.mOriginAttributes.CreateSuffix(suffix); - rv = mDefaultDBState->stmtReadDomain->BindUTF8StringByName( - NS_LITERAL_CSTRING("originAttributes"), suffix); - NS_ASSERT_SUCCESS(rv); - - bool hasResult; - nsCString name, value, host, path; - AutoTArray, kMaxCookiesPerHost> array; - while (true) { - rv = mDefaultDBState->stmtReadDomain->ExecuteStep(&hasResult); - if (NS_FAILED(rv)) { - // Recreate the database. - COOKIE_LOGSTRING(LogLevel::Debug, - ("EnsureReadDomain(): corruption detected when reading result " - "with rv 0x%x", rv)); - HandleCorruptDB(mDefaultDBState); - return; - } - - if (!hasResult) - break; - - array.AppendElement(GetCookieFromRow(mDefaultDBState->stmtReadDomain, - aKey.mOriginAttributes)); + if (!mInitializedDBConn && aInitDBConn && mDefaultDBState) { + InitDBConn(); } - - // Add the cookies to the table in a single operation. This makes sure that - // either all the cookies get added, or in the case of corruption, none. - for (uint32_t i = 0; i < array.Length(); ++i) { - AddCookieToList(aKey, array[i], mDefaultDBState, nullptr, false); - } - - // Add it to the hashset of read entries, so we don't read it again. - mDefaultDBState->readSet.PutEntry(aKey); - - COOKIE_LOGSTRING(LogLevel::Debug, - ("EnsureReadDomain(): %ld cookies read for base domain %s, " - " originAttributes = %s", array.Length(), aKey.mBaseDomain.get(), - suffix.get())); } -void -nsCookieService::EnsureReadComplete() +OpenDBResult +nsCookieService::Read() { - NS_ASSERTION(!mDBState->dbConn || mDBState == mDefaultDBState, - "not in default db state"); + MOZ_ASSERT(NS_GetCurrentThread() == mThread); - // Fast path 1: nothing to read, or we've already finished reading. - if (MOZ_LIKELY(!mDBState->dbConn || !mDefaultDBState->pendingRead)) - return; - - // Cancel the pending read, so we don't get any more results. - CancelAsyncRead(false); + // Set up a statement to delete any rows with a nullptr 'baseDomain' + // column. This takes care of any cookies set by browsers that don't + // understand the 'baseDomain' column, where the database schema version + // is from one that does. (This would occur when downgrading.) + nsresult rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + "DELETE FROM moz_cookies WHERE baseDomain ISNULL")); + NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Read in the data synchronously. // see IDX_NAME, etc. for parameter indexes nsCOMPtr stmt; - nsresult rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( + rv = mDefaultDBState->syncConn->CreateStatement(NS_LITERAL_CSTRING( "SELECT " "name, " "value, " @@ -2760,27 +2584,20 @@ nsCookieService::EnsureReadComplete() "FROM moz_cookies " "WHERE baseDomain NOTNULL"), getter_AddRefs(stmt)); - if (NS_FAILED(rv)) { - // Recreate the database. - COOKIE_LOGSTRING(LogLevel::Debug, - ("EnsureReadComplete(): corruption detected when creating statement " - "with rv 0x%x", rv)); - HandleCorruptDB(mDefaultDBState); - return; + NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + + if (NS_WARN_IF(!mReadArray.IsEmpty())) { + mReadArray.Clear(); } + mReadArray.SetCapacity(kMaxNumberOfCookies); nsCString baseDomain, name, value, host, path; bool hasResult; - nsTArray array(kMaxNumberOfCookies); while (true) { rv = stmt->ExecuteStep(&hasResult); - if (NS_FAILED(rv)) { - // Recreate the database. - COOKIE_LOGSTRING(LogLevel::Debug, - ("EnsureReadComplete(): corruption detected when reading result " - "with rv 0x%x", rv)); - HandleCorruptDB(mDefaultDBState); - return; + if (NS_WARN_IF(NS_FAILED(rv))) { + mReadArray.Clear(); + return RESULT_RETRY; } if (!hasResult) @@ -2797,27 +2614,16 @@ nsCookieService::EnsureReadComplete() Unused << attrs.PopulateFromSuffix(suffix); nsCookieKey key(baseDomain, attrs); - if (mDefaultDBState->readSet.GetEntry(key)) - continue; - - CookieDomainTuple* tuple = array.AppendElement(); + CookieDomainTuple* tuple = mReadArray.AppendElement(); tuple->key = key; tuple->cookie = GetCookieFromRow(stmt, attrs); } - // Add the cookies to the table in a single operation. This makes sure that - // either all the cookies get added, or in the case of corruption, none. - for (uint32_t i = 0; i < array.Length(); ++i) { - CookieDomainTuple& tuple = array[i]; - AddCookieToList(tuple.key, tuple.cookie, mDefaultDBState, nullptr, - false); - } - mDefaultDBState->syncConn = nullptr; - mDefaultDBState->readSet.Clear(); + + COOKIE_LOGSTRING(LogLevel::Debug, ("Read(): %zu cookies read", mReadArray.Length())); - COOKIE_LOGSTRING(LogLevel::Debug, - ("EnsureReadComplete(): %ld cookies read", array.Length())); + return RESULT_OK; } NS_IMETHODIMP @@ -2827,6 +2633,8 @@ nsCookieService::ImportCookies(nsIFile *aCookieFile) NS_WARNING("No DBState! Profile already closed?"); return NS_ERROR_NOT_AVAILABLE; } + + EnsureReadComplete(true); // Make sure we're in the default DB state. We don't want people importing // cookies into a private browsing session! @@ -2843,9 +2651,6 @@ nsCookieService::ImportCookies(nsIFile *aCookieFile) nsCOMPtr lineInputStream = do_QueryInterface(fileInputStream, &rv); if (NS_FAILED(rv)) return rv; - // First, ensure we've read in everything from the database, if we have one. - EnsureReadComplete(); - static const char kTrue[] = "TRUE"; nsAutoCString buffer, baseDomain; @@ -3137,7 +2942,6 @@ nsCookieService::GetCookieStringInternal(nsIURI *aHostURI, bool stale = false; nsCookieKey key(baseDomain, aOriginAttrs); - EnsureReadDomain(key); // perform the hash lookup nsCookieEntry *entry = mDBState->hostTable.GetEntry(key); @@ -3400,6 +3204,9 @@ nsCookieService::AddInternal(const nsCookieKey &aKey, const char *aCookieHeader, bool aFromHttp) { + MOZ_ASSERT(mInitializedDBStates); + MOZ_ASSERT(mInitializedDBConn); + int64_t currentTime = aCurrentTimeInUsec / PR_USEC_PER_SEC; // if the new cookie is httponly, make sure we're not coming from script @@ -4303,7 +4110,6 @@ already_AddRefed nsCookieService::PurgeCookies(int64_t aCurrentTimeInUsec) { NS_ASSERTION(mDBState->hostTable.Count() > 0, "table is empty"); - EnsureReadComplete(); uint32_t initialCookieCount = mDBState->cookieCount; COOKIE_LOGSTRING(LogLevel::Debug, @@ -4457,6 +4263,8 @@ nsCookieService::CookieExistsNative(nsICookie2* aCookie, return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + nsAutoCString host, name, path; nsresult rv = aCookie->GetHost(host); NS_ENSURE_SUCCESS(rv, rv); @@ -4609,6 +4417,8 @@ nsCookieService::CountCookiesFromHost(const nsACString &aHost, return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + // first, normalize the hostname, and fail if it contains illegal characters. nsAutoCString host(aHost); nsresult rv = NormalizeHost(host); @@ -4619,7 +4429,6 @@ nsCookieService::CountCookiesFromHost(const nsACString &aHost, NS_ENSURE_SUCCESS(rv, rv); nsCookieKey key = DEFAULT_APP_KEY(baseDomain); - EnsureReadDomain(key); // Return a count of all cookies, including expired. nsCookieEntry *entry = mDBState->hostTable.GetEntry(key); @@ -4643,6 +4452,8 @@ nsCookieService::GetCookiesFromHost(const nsACString &aHost, return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + // first, normalize the hostname, and fail if it contains illegal characters. nsAutoCString host(aHost); nsresult rv = NormalizeHost(host); @@ -4662,7 +4473,6 @@ nsCookieService::GetCookiesFromHost(const nsACString &aHost, NS_ENSURE_SUCCESS(rv, rv); nsCookieKey key = nsCookieKey(baseDomain, attrs); - EnsureReadDomain(key); nsCookieEntry *entry = mDBState->hostTable.GetEntry(key); if (!entry) @@ -4709,6 +4519,8 @@ nsCookieService::GetCookiesWithOriginAttributes( return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + if (aPattern.mAppId.WasPassed() && aPattern.mAppId.Value() == NECKO_UNKNOWN_APP_ID) { return NS_ERROR_INVALID_ARG; } @@ -4767,6 +4579,8 @@ nsCookieService::RemoveCookiesWithOriginAttributes( return NS_ERROR_NOT_AVAILABLE; } + EnsureReadComplete(true); + // Iterate the hash table of nsCookieEntry. for (auto iter = mDBState->hostTable.Iter(); !iter.Done(); iter.Next()) { nsCookieEntry* entry = iter.Get(); @@ -4804,8 +4618,6 @@ bool nsCookieService::FindSecureCookie(const nsCookieKey &aKey, nsCookie *aCookie) { - EnsureReadDomain(aKey); - nsCookieEntry *entry = mDBState->hostTable.GetEntry(aKey); if (!entry) return false; @@ -4841,7 +4653,9 @@ nsCookieService::FindCookie(const nsCookieKey &aKey, const nsAFlatCString &aPath, nsListIter &aIter) { - EnsureReadDomain(aKey); + // Should |EnsureReadComplete| before. + MOZ_ASSERT(mInitializedDBStates); + MOZ_ASSERT(mInitializedDBConn); nsCookieEntry *entry = mDBState->hostTable.GetEntry(aKey); if (!entry) diff --git a/netwerk/cookie/nsCookieService.h b/netwerk/cookie/nsCookieService.h index df83045243..80408ed7c5 100644 --- a/netwerk/cookie/nsCookieService.h +++ b/netwerk/cookie/nsCookieService.h @@ -28,9 +28,12 @@ #include "mozIStorageFunction.h" #include "nsIVariant.h" #include "nsIFile.h" +#include "mozilla/Atomics.h" #include "mozilla/BasePrincipal.h" #include "mozilla/MemoryReporting.h" #include "mozilla/Maybe.h" +#include "mozilla/Monitor.h" +#include "mozilla/UniquePtr.h" using mozilla::NeckoOriginAttributes; using mozilla::OriginAttributes; @@ -43,6 +46,7 @@ class nsIObserverService; class nsIURI; class nsIChannel; class nsIArray; +class nsIThread; class mozIStorageService; class mozIThirdPartyUtil; class ReadCookieDBListener; @@ -145,13 +149,49 @@ class nsCookieEntry : public nsCookieKey ArrayType mCookies; }; +// struct for a constant cookie for threadsafe +struct ConstCookie +{ + ConstCookie(const nsCString& aName, + const nsCString& aValue, + const nsCString& aHost, + const nsCString& aPath, + int64_t aExpiry, + int64_t aLastAccessed, + int64_t aCreationTime, + bool aIsSecure, + bool aIsHttpOnly, + const OriginAttributes &aOriginAttributes) + : name(aName) + , value(aValue) + , host(aHost) + , path(aPath) + , expiry(aExpiry) + , lastAccessed(aLastAccessed) + , creationTime(aCreationTime) + , isSecure(aIsSecure) + , isHttpOnly(aIsHttpOnly) + , originAttributes(aOriginAttributes) + { + } + + const nsCString name; + const nsCString value; + const nsCString host; + const nsCString path; + const int64_t expiry; + const int64_t lastAccessed; + const int64_t creationTime; + const bool isSecure; + const bool isHttpOnly; + const OriginAttributes originAttributes; +}; + // encapsulates a (key, nsCookie) tuple for temporary storage purposes. struct CookieDomainTuple { nsCookieKey key; - RefPtr cookie; - - size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const; + mozilla::UniquePtr cookie; }; // encapsulates in-memory and on-disk DB states, so we can @@ -194,17 +234,9 @@ public: // while the background read is taking place. nsCOMPtr syncConn; nsCOMPtr stmtReadDomain; - nsCOMPtr pendingRead; // The asynchronous read listener. This is a weak ref (storage has ownership) // since it may need to outlive the DBState's database connection. ReadCookieDBListener* readListener; - // An array of (baseDomain, cookie) tuples representing data read in - // asynchronously. This is merged into hostTable once read is complete. - nsTArray hostArray; - // A hashset of baseDomains read in synchronously, while the async read is - // in flight. This is used to keep track of which data in hostArray is stale - // when the time comes to merge. - nsTHashtable readSet; // DB completion handlers. nsCOMPtr insertListener; @@ -266,6 +298,8 @@ class nsCookieService final : public nsICookieService void PrefChanged(nsIPrefBranch *aPrefBranch); void InitDBStates(); OpenDBResult TryInitDB(bool aDeleteExistingDB); + void InitDBConn(); + nsresult InitDBConnInternal(); nsresult CreateTable(); nsresult CreateTableForSchemaVersion6(); nsresult CreateTableForSchemaVersion5(); @@ -276,11 +310,8 @@ class nsCookieService final : public nsICookieService void HandleCorruptDB(DBState* aDBState); void RebuildCorruptDB(DBState* aDBState); OpenDBResult Read(); - template nsCookie* GetCookieFromRow(T &aRow, const OriginAttributes& aOriginAttributes); - void AsyncReadComplete(); - void CancelAsyncRead(bool aPurgeReadSet); - void EnsureReadDomain(const nsCookieKey &aKey); - void EnsureReadComplete(); + mozilla::UniquePtr GetCookieFromRow(mozIStorageStatement *aRow, const OriginAttributes &aOriginAttributes); + void EnsureReadComplete(bool aInitDBConn); nsresult NormalizeHost(nsCString &aHost); nsresult GetBaseDomain(nsIURI *aHostURI, nsCString &aBaseDomain, bool &aRequireHostMatch); nsresult GetBaseDomainFromHost(const nsACString &aHost, nsCString &aBaseDomain); @@ -351,6 +382,14 @@ class nsCookieService final : public nsICookieService uint16_t mMaxCookiesPerHost; int64_t mCookiePurgeAge; + // thread props + nsCOMPtr mThread; + mozilla::Monitor mMonitor; + mozilla::Atomic mInitializedDBStates; + mozilla::Atomic mInitializedDBConn; + bool mAccumulatedWaitTelemetry; + nsTArray mReadArray; + // friends! friend class DBListenerErrorHandler; friend class ReadCookieDBListener; From fe53a0bb1cff5f8c155b0aec2c5b9bc63f1981dd Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 22 Dec 2024 16:05:33 +0100 Subject: [PATCH 2/6] Issue #2672 - Part 2: Auto-close `syncConn` for edge cases. --- netwerk/cookie/nsCookieService.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index e78b50d527..74683e7a49 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -49,6 +49,7 @@ #include "mozilla/storage.h" #include "mozilla/AutoRestore.h" #include "mozilla/FileUtils.h" +#include "mozilla/ScopeExit.h" #include "nsIConsoleService.h" #include "nsVariant.h" @@ -845,6 +846,10 @@ nsCookieService::TryInitDB(bool aRecreateDB) getter_AddRefs(mDefaultDBState->syncConn)); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + auto guard = MakeScopeExit([&] { + mDefaultDBState->syncConn = nullptr; + }); + bool tableExists = false; mDefaultDBState->syncConn->TableExists(NS_LITERAL_CSTRING("moz_cookies"), &tableExists); @@ -1596,8 +1601,8 @@ nsCookieService::CleanupDefaultDBConnection() // Null out the database connections. If 'dbConn' has not been used for any // asynchronous operations yet, this will synchronously close it; otherwise, // it's expected that the caller has performed an AsyncClose prior. + // Note 'syncConn' is auto-closed on scope exit. mDefaultDBState->dbConn = nullptr; - mDefaultDBState->syncConn = nullptr; // Manually null out our listeners. This is necessary because they hold a // strong ref to the DBState itself. They'll stay alive until whatever @@ -2619,7 +2624,7 @@ nsCookieService::Read() tuple->cookie = GetCookieFromRow(stmt, attrs); } - mDefaultDBState->syncConn = nullptr; + // Note: 'syncConn' is auto-closed on scope exit. COOKIE_LOGSTRING(LogLevel::Debug, ("Read(): %zu cookies read", mReadArray.Length())); From 032b0ae0d88dab1dde12736656ec477dea5ee993 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 22 Dec 2024 16:57:19 +0100 Subject: [PATCH 3/6] Issue #2672 - Part 3: Ensure thread lifetimes are in tandem. --- netwerk/cookie/nsCookieService.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 74683e7a49..7d1ad7d536 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -598,9 +598,6 @@ nsCookieService::Init() mStorageService = do_GetService("@mozilla.org/storage/service;1", &rv); NS_ENSURE_SUCCESS(rv, rv); - rv = NS_NewNamedThread("Cookie", getter_AddRefs(mThread)); - NS_ENSURE_SUCCESS(rv, rv); - // Init our default, and possibly private DBStates. InitDBStates(); @@ -628,6 +625,7 @@ nsCookieService::InitDBStates() NS_ASSERTION(!mDefaultDBState, "already have a default DBState"); NS_ASSERTION(!mPrivateDBState, "already have a private DBState"); NS_ASSERTION(!mInitializedDBStates, "already initialized"); + NS_ASSERTION(!mThread, "already have a cookie service thread"); // Create a new default DBState and set our current one. mDefaultDBState = new DBState(); @@ -649,6 +647,8 @@ nsCookieService::InitDBStates() } mDefaultDBState->cookieFile->AppendNative(NS_LITERAL_CSTRING(COOKIES_FILE)); + NS_ENSURE_SUCCESS_VOID(NS_NewNamedThread("Cookie", getter_AddRefs(mThread))); + nsCOMPtr runnable = NS_NewRunnableFunction([] { NS_ENSURE_TRUE_VOID(gCookieService && gCookieService->mDBState && @@ -1553,7 +1553,10 @@ nsCookieService::CloseDBStates() return; } - EnsureReadComplete(false); + if (mThread) { + mThread->Shutdown(); + mThread = nullptr; + } // Null out our private and pointer DBStates regardless. mPrivateDBState = nullptr; @@ -1812,9 +1815,6 @@ nsCookieService::~nsCookieService() UnregisterWeakMemoryReporter(this); gCookieService = nullptr; - if (mThread) { - mThread->Shutdown(); - } } NS_IMETHODIMP From d69e2a3779841bbe7c02bf691e331e64070f36b1 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 22 Dec 2024 16:58:11 +0100 Subject: [PATCH 4/6] Issue #2672 - Part 4: Prevent db access while rebuilding is underway. --- netwerk/cookie/nsCookieService.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 7d1ad7d536..4f6476bba8 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -1577,6 +1577,7 @@ nsCookieService::CloseDBStates() CleanupDefaultDBConnection(); mDefaultDBState = nullptr; + mInitializedDBConn = false; mInitializedDBStates = false; } @@ -1614,8 +1615,6 @@ nsCookieService::CleanupDefaultDBConnection() mDefaultDBState->updateListener = nullptr; mDefaultDBState->removeListener = nullptr; mDefaultDBState->closeListener = nullptr; - - mInitializedDBConn = false; } void @@ -1764,7 +1763,7 @@ nsCookieService::RebuildCorruptDB(DBState* aDBState) os->NotifyObservers(nullptr, "cookie-db-rebuilding", nullptr); } - gCookieService->InitDBConn(); + gCookieService->InitDBConnInternal(); // Enumerate the hash, and add cookies to the params array. mozIStorageAsyncStatement* stmt = gCookieService->mDefaultDBState->stmtInsert; From d935c4f54ccde39c7759871578da4bde8c8dcfc4 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 24 Dec 2024 13:53:16 +0100 Subject: [PATCH 5/6] Issue #2672 - Follow-up: Remove unused telemetry variable. --- netwerk/cookie/nsCookieService.h | 1 - 1 file changed, 1 deletion(-) diff --git a/netwerk/cookie/nsCookieService.h b/netwerk/cookie/nsCookieService.h index 80408ed7c5..85443483a2 100644 --- a/netwerk/cookie/nsCookieService.h +++ b/netwerk/cookie/nsCookieService.h @@ -387,7 +387,6 @@ class nsCookieService final : public nsICookieService mozilla::Monitor mMonitor; mozilla::Atomic mInitializedDBStates; mozilla::Atomic mInitializedDBConn; - bool mAccumulatedWaitTelemetry; nsTArray mReadArray; // friends! From 2e1c4875639ab11f81989660eaaae007b1abca2b Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 22 Dec 2024 20:27:41 +0100 Subject: [PATCH 6/6] Issue #2670 - Compute baseDomain when cookies are read from the database baseDomain is used as a key for cookies in a hashmap. For cookie operations to work properly, the baseDomain generation must be reliable. This is, however, not the case, because the result can change by updates to the public suffix list (PSL). Since the stored baseDomain is not reliable, the value must be recomputed when the database is read. This causes a minor performance hit but is needed if we want to keep abreast of PSL changes without clobbering the cookies database. Resolves #2670 --- netwerk/cookie/nsCookieService.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 4f6476bba8..516261fa22 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -2607,8 +2607,16 @@ nsCookieService::Read() if (!hasResult) break; - // Make sure we haven't already read the data. - stmt->GetUTF8String(IDX_BASE_DOMAIN, baseDomain); + // IDX_BASE_DOMAIN cannot be used, because updates to the public suffix list + // may invalidate the value of the stored baseDomain. + stmt->GetUTF8String(IDX_HOST, host); + + rv = GetBaseDomainFromHost(host, baseDomain); + if (NS_FAILED(rv)) { + COOKIE_LOGSTRING(LogLevel::Debug, + ("Read(): Ignoring invalid host '%s'", host.get())); + continue; + } nsAutoCString suffix; NeckoOriginAttributes attrs;