diff --git a/dom/cache/Connection.cpp b/dom/cache/Connection.cpp index a2ecba997883..9676e9a6af7a 100644 --- a/dom/cache/Connection.cpp +++ b/dom/cache/Connection.cpp @@ -49,12 +49,6 @@ Connection::Close() { // mozIStorageAsyncConnection methods -NS_IMETHODIMP -Connection::AsyncVacuum(mozIStorageCompletionCallback*, bool, int32_t) { - // async methods are not supported - return NS_ERROR_NOT_IMPLEMENTED; -} - NS_IMETHODIMP Connection::AsyncClose(mozIStorageCompletionCallback*) { // async methods are not supported diff --git a/storage/VacuumManager.cpp b/storage/VacuumManager.cpp index 805333947ea2..26fcdcd838e4 100644 --- a/storage/VacuumManager.cpp +++ b/storage/VacuumManager.cpp @@ -18,63 +18,104 @@ #include "mozilla/StaticPrefs_storage.h" #include "mozStorageConnection.h" -#include "mozStoragePrivateHelpers.h" #include "mozIStorageStatement.h" -#include "mozIStorageCompletionCallback.h" +#include "mozIStorageStatementCallback.h" #include "mozIStorageAsyncStatement.h" #include "mozIStoragePendingStatement.h" #include "mozIStorageError.h" #include "mozStorageHelper.h" #include "nsXULAppAPI.h" -#include "xpcpublic.h" #define OBSERVER_TOPIC_IDLE_DAILY "idle-daily" +#define OBSERVER_TOPIC_XPCOM_SHUTDOWN "xpcom-shutdown" -// Used to notify the begin and end of a vacuum operation. -#define OBSERVER_TOPIC_VACUUM_BEGIN "vacuum-begin" -#define OBSERVER_TOPIC_VACUUM_END "vacuum-end" -// This notification is sent only in automation when vacuum for a database is -// skipped, and can thus be used to verify that. -#define OBSERVER_TOPIC_VACUUM_SKIP "vacuum-skip" +// Used to notify begin and end of a heavy IO task. +#define OBSERVER_TOPIC_HEAVY_IO "heavy-io-task" +#define OBSERVER_DATA_VACUUM_BEGIN u"vacuum-begin" +#define OBSERVER_DATA_VACUUM_END u"vacuum-end" // This preferences root will contain last vacuum timestamps (in seconds) for // each database. The database filename is used as a key. #define PREF_VACUUM_BRANCH "storage.vacuum.last." // Time between subsequent vacuum calls for a certain database. -#define VACUUM_INTERVAL_SECONDS (30 * 86400) // 30 days. +#define VACUUM_INTERVAL_SECONDS 30 * 86400 // 30 days. extern mozilla::LazyLogModule gStorageLog; -namespace mozilla::storage { +namespace mozilla { +namespace storage { namespace { //////////////////////////////////////////////////////////////////////////////// -//// Vacuumer declaration. +//// BaseCallback -class Vacuumer final : public mozIStorageCompletionCallback { +class BaseCallback : public mozIStorageStatementCallback { public: NS_DECL_ISUPPORTS - NS_DECL_MOZISTORAGECOMPLETIONCALLBACK + NS_DECL_MOZISTORAGESTATEMENTCALLBACK + BaseCallback() {} + + protected: + virtual ~BaseCallback() {} +}; + +NS_IMETHODIMP +BaseCallback::HandleError(mozIStorageError* aError) { +#ifdef DEBUG + int32_t result; + nsresult rv = aError->GetResult(&result); + NS_ENSURE_SUCCESS(rv, rv); + nsAutoCString message; + rv = aError->GetMessage(message); + NS_ENSURE_SUCCESS(rv, rv); + + nsAutoCString warnMsg; + warnMsg.AppendLiteral("An error occured during async execution: "); + warnMsg.AppendInt(result); + warnMsg.Append(' '); + warnMsg.Append(message); + NS_WARNING(warnMsg.get()); +#endif + return NS_OK; +} + +NS_IMETHODIMP +BaseCallback::HandleResult(mozIStorageResultSet* aResultSet) { + // We could get results from PRAGMA statements, but we don't mind them. + return NS_OK; +} + +NS_IMETHODIMP +BaseCallback::HandleCompletion(uint16_t aReason) { + // By default BaseCallback will just be silent on completion. + return NS_OK; +} + +NS_IMPL_ISUPPORTS(BaseCallback, mozIStorageStatementCallback) + +//////////////////////////////////////////////////////////////////////////////// +//// Vacuumer declaration. + +class Vacuumer : public BaseCallback { + public: + NS_DECL_MOZISTORAGESTATEMENTCALLBACK explicit Vacuumer(mozIStorageVacuumParticipant* aParticipant); + bool execute(); + nsresult notifyCompletion(bool aSucceeded); private: - nsresult notifyCompletion(bool aSucceeded); - ~Vacuumer() = default; - nsCOMPtr mParticipant; nsCString mDBFilename; - nsCOMPtr mDBConn; + nsCOMPtr mDBConn; }; //////////////////////////////////////////////////////////////////////////////// //// Vacuumer implementation. -NS_IMPL_ISUPPORTS(Vacuumer, mozIStorageCompletionCallback) - Vacuumer::Vacuumer(mozIStorageVacuumParticipant* aParticipant) : mParticipant(aParticipant) {} @@ -83,21 +124,30 @@ bool Vacuumer::execute() { // Get the connection and check its validity. nsresult rv = mParticipant->GetDatabaseConnection(getter_AddRefs(mDBConn)); - if (NS_FAILED(rv) || !mDBConn) return false; + NS_ENSURE_SUCCESS(rv, false); + bool ready = false; + if (!mDBConn || NS_FAILED(mDBConn->GetConnectionReady(&ready)) || !ready) { + NS_WARNING("Unable to get a connection to vacuum database"); + return false; + } - nsCOMPtr os = mozilla::services::GetObserverService(); + // Ask for the expected page size. Vacuum can change the page size, unless + // the database is using WAL journaling. + // TODO Bug 634374: figure out a strategy to fix page size with WAL. + int32_t expectedPageSize = 0; + rv = mParticipant->GetExpectedDatabasePageSize(&expectedPageSize); + if (NS_FAILED(rv) || !Service::pageSizeIsValid(expectedPageSize)) { + NS_WARNING("Invalid page size requested for database, will use default "); + NS_WARNING(mDBFilename.get()); + expectedPageSize = Service::kDefaultPageSize; + } - bool inAutomation = xpc::IsInAutomation(); // Get the database filename. Last vacuum time is stored under this name // in PREF_VACUUM_BRANCH. nsCOMPtr databaseFile; mDBConn->GetDatabaseFile(getter_AddRefs(databaseFile)); if (!databaseFile) { NS_WARNING("Trying to vacuum a in-memory database!"); - if (inAutomation && os) { - mozilla::Unused << os->NotifyObservers( - nullptr, OBSERVER_TOPIC_VACUUM_SKIP, u":memory:"); - } return false; } nsAutoString databaseFilename; @@ -114,11 +164,6 @@ bool Vacuumer::execute() { rv = Preferences::GetInt(prefName.get(), &lastVacuum); if (NS_SUCCEEDED(rv) && (now - lastVacuum) < VACUUM_INTERVAL_SECONDS) { // This database was vacuumed recently, skip it. - if (inAutomation && os) { - mozilla::Unused << os->NotifyObservers( - nullptr, OBSERVER_TOPIC_VACUUM_SKIP, - NS_ConvertUTF8toUTF16(mDBFilename).get()); - } return false; } @@ -129,48 +174,86 @@ bool Vacuumer::execute() { rv = mParticipant->OnBeginVacuum(&vacuumGranted); NS_ENSURE_SUCCESS(rv, false); if (!vacuumGranted) { - if (inAutomation && os) { - mozilla::Unused << os->NotifyObservers( - nullptr, OBSERVER_TOPIC_VACUUM_SKIP, - NS_ConvertUTF8toUTF16(mDBFilename).get()); - } return false; } - // Ask for the expected page size. Vacuum can change the page size, unless - // the database is using WAL journaling. - // TODO Bug 634374: figure out a strategy to fix page size with WAL. - int32_t expectedPageSize = 0; - rv = mParticipant->GetExpectedDatabasePageSize(&expectedPageSize); - if (NS_FAILED(rv) || !Service::pageSizeIsValid(expectedPageSize)) { - NS_WARNING("Invalid page size requested for database, won't set it. "); - NS_WARNING(mDBFilename.get()); - expectedPageSize = 0; - } - - bool incremental = false; - mozilla::Unused << mParticipant->GetUseIncrementalVacuum(&incremental); - - // Notify vacuum is about to start. + // Notify a heavy IO task is about to start. + nsCOMPtr os = mozilla::services::GetObserverService(); if (os) { - mozilla::Unused << os->NotifyObservers( - nullptr, OBSERVER_TOPIC_VACUUM_BEGIN, - NS_ConvertUTF8toUTF16(mDBFilename).get()); + rv = os->NotifyObservers(nullptr, OBSERVER_TOPIC_HEAVY_IO, + OBSERVER_DATA_VACUUM_BEGIN); + MOZ_ASSERT(NS_SUCCEEDED(rv), "Should be able to notify"); } - rv = mDBConn->AsyncVacuum(this, incremental, expectedPageSize); - if (NS_FAILED(rv)) { - // The connection is not ready. - mozilla::Unused << Complete(rv, nullptr); - return false; - } + // Execute the statements separately, since the pragma may conflict with the + // vacuum, if they are executed in the same transaction. + nsCOMPtr pageSizeStmt; + nsAutoCString pageSizeQuery(MOZ_STORAGE_UNIQUIFY_QUERY_STR + "PRAGMA page_size = "); + pageSizeQuery.AppendInt(expectedPageSize); + rv = mDBConn->CreateAsyncStatement(pageSizeQuery, + getter_AddRefs(pageSizeStmt)); + NS_ENSURE_SUCCESS(rv, false); + RefPtr callback = new BaseCallback(); + nsCOMPtr ps; + rv = pageSizeStmt->ExecuteAsync(callback, getter_AddRefs(ps)); + NS_ENSURE_SUCCESS(rv, false); + + nsCOMPtr stmt; + rv = mDBConn->CreateAsyncStatement("VACUUM"_ns, getter_AddRefs(stmt)); + NS_ENSURE_SUCCESS(rv, false); + rv = stmt->ExecuteAsync(this, getter_AddRefs(ps)); + NS_ENSURE_SUCCESS(rv, false); return true; } +//////////////////////////////////////////////////////////////////////////////// +//// mozIStorageStatementCallback + NS_IMETHODIMP -Vacuumer::Complete(nsresult aStatus, nsISupports* aValue) { - if (NS_SUCCEEDED(aStatus)) { +Vacuumer::HandleError(mozIStorageError* aError) { + int32_t result; + nsresult rv; + nsAutoCString message; + +#ifdef DEBUG + rv = aError->GetResult(&result); + NS_ENSURE_SUCCESS(rv, rv); + rv = aError->GetMessage(message); + NS_ENSURE_SUCCESS(rv, rv); + + nsAutoCString warnMsg; + warnMsg.AppendLiteral("Unable to vacuum database: "); + warnMsg.Append(mDBFilename); + warnMsg.AppendLiteral(" - "); + warnMsg.AppendInt(result); + warnMsg.Append(' '); + warnMsg.Append(message); + NS_WARNING(warnMsg.get()); +#endif + + if (MOZ_LOG_TEST(gStorageLog, LogLevel::Error)) { + rv = aError->GetResult(&result); + NS_ENSURE_SUCCESS(rv, rv); + rv = aError->GetMessage(message); + NS_ENSURE_SUCCESS(rv, rv); + MOZ_LOG(gStorageLog, LogLevel::Error, + ("Vacuum failed with error: %d '%s'. Database was: '%s'", result, + message.get(), mDBFilename.get())); + } + return NS_OK; +} + +NS_IMETHODIMP +Vacuumer::HandleResult(mozIStorageResultSet* aResultSet) { + MOZ_ASSERT_UNREACHABLE("Got a resultset from a vacuum?"); + return NS_OK; +} + +NS_IMETHODIMP +Vacuumer::HandleCompletion(uint16_t aReason) { + if (aReason == REASON_FINISHED) { // Update last vacuum time. int32_t now = static_cast(PR_Now() / PR_USEC_PER_SEC); MOZ_ASSERT(!mDBFilename.IsEmpty(), "Database filename cannot be empty"); @@ -178,35 +261,18 @@ Vacuumer::Complete(nsresult aStatus, nsISupports* aValue) { prefName += mDBFilename; DebugOnly rv = Preferences::SetInt(prefName.get(), now); MOZ_ASSERT(NS_SUCCEEDED(rv), "Should be able to set a preference"); - notifyCompletion(true); - return NS_OK; } -#ifdef DEBUG - nsAutoCString warnMsg; - warnMsg.AppendLiteral("Unable to vacuum database: "); - warnMsg.Append(mDBFilename); - warnMsg.AppendLiteral(" - "); - warnMsg.AppendInt(NS_ERROR_GET_CODE(aStatus)); - NS_WARNING(warnMsg.get()); -#endif + notifyCompletion(aReason == REASON_FINISHED); - if (MOZ_LOG_TEST(gStorageLog, LogLevel::Error)) { - MOZ_LOG(gStorageLog, LogLevel::Error, - ("Vacuum failed with error: %d. Database was: '%s'", aStatus, - mDBFilename.get())); - } - - notifyCompletion(false); return NS_OK; } nsresult Vacuumer::notifyCompletion(bool aSucceeded) { nsCOMPtr os = mozilla::services::GetObserverService(); if (os) { - mozilla::Unused << os->NotifyObservers( - nullptr, OBSERVER_TOPIC_VACUUM_END, - NS_ConvertUTF8toUTF16(mDBFilename).get()); + os->NotifyObservers(nullptr, OBSERVER_TOPIC_HEAVY_IO, + OBSERVER_DATA_VACUUM_END); } nsresult rv = mParticipant->OnEndVacuum(aSucceeded); @@ -287,4 +353,5 @@ VacuumManager::Observe(nsISupports* aSubject, const char* aTopic, return NS_OK; } -} // namespace mozilla::storage +} // namespace storage +} // namespace mozilla diff --git a/storage/mozIStorageAsyncConnection.idl b/storage/mozIStorageAsyncConnection.idl index 6c053850076d..00cf736dc1d8 100644 --- a/storage/mozIStorageAsyncConnection.idl +++ b/storage/mozIStorageAsyncConnection.idl @@ -193,38 +193,6 @@ interface mozIStorageAsyncConnection : nsISupports { */ void interrupt(); - /** - * Vacuum the main database plus all the attached one. - * If the database is in auto_vacuum = INCREMENTAL mode, this executes an - * incremental_vacuum, otherwise it will always execute a full vacuum. - * - * While it's possible to invoke this method directly, it's suggested, when - * possible, to use the VacuumManager instead. - * That means registering your component for the "vacuum-participant" XPCOM - * category, and implement the mozIStorageVacuumParticipant interface. - * - * @param [aCallback] Completion callback invoked once the operation is - * complete. - * @param [aUseIncremental] When set to true, this will try to convert the - * main schema to auto_vacuum = INCREMENTAL mode, if it's not set yet. - * When set to false, it will try to set auto_vacuum = NONE. - * Note a full vacuum will be executed if the auto_vacuum mode must be - * changed, otherwise an incremental vacuum will happen if the database - * is already in INCREMENTAL mode. - * @param [aSetPageSize] This can be used to change the database page_size, a - * full vacuum will be executed to persist the change. If the page - * size is already correct, or you pass 0, this will be a no-op. - * @throws If it's not possible to start the async vacuum operation, note in - * this case the callback won't be invoked. - * @note Vacuum will fail inside a transaction, or if there is an ongoing - * read statement. - */ - void asyncVacuum( - [optional] in mozIStorageCompletionCallback aCallback, - [optional] in boolean aUseIncremental, - [optional] in long aSetPageSize - ); - ////////////////////////////////////////////////////////////////////////////// //// Statement creation diff --git a/storage/mozIStorageVacuumParticipant.idl b/storage/mozIStorageVacuumParticipant.idl index 1515ac092561..a4e0f3a712a0 100644 --- a/storage/mozIStorageVacuumParticipant.idl +++ b/storage/mozIStorageVacuumParticipant.idl @@ -6,7 +6,7 @@ #include "nsISupports.idl" -interface mozIStorageAsyncConnection; +interface mozIStorageConnection; /** * This interface contains the information that the Storage service needs to @@ -19,27 +19,19 @@ interface mozIStorageAsyncConnection; interface mozIStorageVacuumParticipant : nsISupports { /** * The expected page size in bytes for the database. The vacuum manager will - * try to correct the page size by executing a full vacuum. + * try to correct the page size during idle based on this value. * * @note If the database is using the WAL journal mode, the page size won't - * be changed to the requested value. See bug 634374. + * be changed to the requested value. See bug 634374. * @note Valid page size values are powers of 2 between 512 and 65536. * The suggested value is mozIStorageConnection::defaultPageSize. */ readonly attribute long expectedDatabasePageSize; - /** - * Whether the main schema should be using auto_vacuum = INCREMENTAL. - * This will cause auto_vacuum to change to INCREMENTAL if it's not set yet. - * It is not possible to change mode of any attached databases through this, - * to do that you must open a separate connection and use asyncVacuum() on it. - */ - readonly attribute boolean useIncrementalVacuum; - /** * Connection to the database file to be vacuumed. */ - readonly attribute mozIStorageAsyncConnection databaseConnection; + readonly attribute mozIStorageConnection databaseConnection; /** * Notifies when a vacuum operation begins. Listeners should avoid using the @@ -49,9 +41,9 @@ interface mozIStorageVacuumParticipant : nsISupports { * opt-out for now, it will be retried later. Useful when participant * is running some other heavy operation that can't be interrupted. * - * @note When a vacuum operation starts or ends it will also dispatch global - * "vacuum-begin" and "vacuum-end" notifications through the observer - * service with the data argument being the database filename. + * @note When a vacuum operation starts or ends it will also dispatch a global + * "heavy-io-task" notification through the observer service with the + * data argument being either "vacuum-begin" or "vacuum-end". */ boolean onBeginVacuum(); diff --git a/storage/mozStorageConnection.cpp b/storage/mozStorageConnection.cpp index 611364585a6a..91d3b970196b 100644 --- a/storage/mozStorageConnection.cpp +++ b/storage/mozStorageConnection.cpp @@ -8,7 +8,6 @@ #include "nsIFile.h" #include "nsIFileURL.h" #include "nsIXPConnect.h" -#include "mozilla/AppShutdown.h" #include "mozilla/Telemetry.h" #include "mozilla/Mutex.h" #include "mozilla/CondVar.h" @@ -420,151 +419,6 @@ class CloseListener final : public mozIStorageCompletionCallback { NS_IMPL_ISUPPORTS(CloseListener, mozIStorageCompletionCallback) -class AsyncVacuumEvent final : public Runnable { - public: - AsyncVacuumEvent(Connection* aConnection, - mozIStorageCompletionCallback* aCallback, - bool aUseIncremental, int32_t aSetPageSize) - : Runnable("storage::AsyncVacuum"), - mConnection(aConnection), - mCallback(aCallback), - mUseIncremental(aUseIncremental), - mSetPageSize(aSetPageSize), - mStatus(NS_ERROR_UNEXPECTED) {} - - NS_IMETHOD Run() override { - // This is initially dispatched to the helper thread, then re-dispatched - // to the opener thread, where it will callback. - if (IsOnCurrentSerialEventTarget(mConnection->eventTargetOpenedOn)) { - // Send the completion event. - if (mCallback) { - mozilla::Unused << mCallback->Complete(mStatus, nullptr); - } - return NS_OK; - } - - // Ensure to invoke the callback regardless of errors. - auto guard = MakeScopeExit([&]() { - mConnection->mIsStatementOnHelperThreadInterruptible = false; - mozilla::Unused << mConnection->eventTargetOpenedOn->Dispatch( - this, NS_DISPATCH_NORMAL); - }); - - // Get list of attached databases. - nsCOMPtr stmt; - nsresult rv = mConnection->CreateStatement(MOZ_STORAGE_UNIQUIFY_QUERY_STR - "PRAGMA database_list"_ns, - getter_AddRefs(stmt)); - NS_ENSURE_SUCCESS(rv, rv); - // We must accumulate names and loop through them later, otherwise VACUUM - // will see an ongoing statement and bail out. - nsTArray schemaNames; - bool hasResult = false; - while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { - nsAutoCString name; - rv = stmt->GetUTF8String(1, name); - if (NS_SUCCEEDED(rv) && !name.EqualsLiteral("temp")) { - schemaNames.AppendElement(name); - } - } - mStatus = NS_OK; - // Mark this vacuum as an interruptible operation, so it can be interrupted - // if the connection closes during shutdown. - mConnection->mIsStatementOnHelperThreadInterruptible = true; - for (const nsCString& schemaName : schemaNames) { - rv = this->Vacuum(schemaName); - if (NS_FAILED(rv)) { - // This is sub-optimal since it's only keeping the last error reason, - // but it will do for now. - mStatus = rv; - } - } - return mStatus; - } - - nsresult Vacuum(const nsACString& aSchemaName) { - // Abort if we're in shutdown. - if (AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownConfirmed)) { - return NS_ERROR_ABORT; - } - int32_t removablePages = mConnection->RemovablePagesInFreeList(aSchemaName); - if (!removablePages) { - // There's no empty pages to remove, so skip this vacuum for now. - return NS_OK; - } - nsresult rv; - bool needsFullVacuum = true; - - if (mSetPageSize) { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA "); - query.Append(aSchemaName); - query.AppendLiteral(".page_size = "); - query.AppendInt(mSetPageSize); - nsCOMPtr stmt; - rv = mConnection->ExecuteSimpleSQL(query); - NS_ENSURE_SUCCESS(rv, rv); - } - - // Check auto_vacuum. - { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA "); - query.Append(aSchemaName); - query.AppendLiteral(".auto_vacuum"); - nsCOMPtr stmt; - rv = mConnection->CreateStatement(query, getter_AddRefs(stmt)); - NS_ENSURE_SUCCESS(rv, rv); - bool hasResult = false; - bool changeAutoVacuum = false; - if (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { - bool isIncrementalVacuum = stmt->AsInt32(0) == 2; - changeAutoVacuum = isIncrementalVacuum != mUseIncremental; - if (isIncrementalVacuum && !changeAutoVacuum) { - needsFullVacuum = false; - } - } - // Changing auto_vacuum is only supported on the main schema. - if (aSchemaName.EqualsLiteral("main") && changeAutoVacuum) { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA "); - query.Append(aSchemaName); - query.AppendLiteral(".auto_vacuum = "); - query.AppendInt(mUseIncremental ? 2 : 0); - rv = mConnection->ExecuteSimpleSQL(query); - NS_ENSURE_SUCCESS(rv, rv); - } - } - - if (needsFullVacuum) { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "VACUUM "); - query.Append(aSchemaName); - rv = mConnection->ExecuteSimpleSQL(query); - // TODO (Bug 1818039): Report failed vacuum telemetry. - NS_ENSURE_SUCCESS(rv, rv); - } else { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA "); - query.Append(aSchemaName); - query.AppendLiteral(".incremental_vacuum("); - query.AppendInt(removablePages); - query.AppendLiteral(")"); - rv = mConnection->ExecuteSimpleSQL(query); - NS_ENSURE_SUCCESS(rv, rv); - } - - return NS_OK; - } - - ~AsyncVacuumEvent() override { - NS_ReleaseOnMainThread("AsyncVacuum::mConnection", mConnection.forget()); - NS_ReleaseOnMainThread("AsyncVacuum::mCallback", mCallback.forget()); - } - - private: - RefPtr mConnection; - nsCOMPtr mCallback; - bool mUseIncremental; - int32_t mSetPageSize; - Atomic mStatus; -}; - } // namespace //////////////////////////////////////////////////////////////////////////////// @@ -576,7 +430,6 @@ Connection::Connection(Service* aService, int aFlags, : sharedAsyncExecutionMutex("Connection::sharedAsyncExecutionMutex"), sharedDBMutex("Connection::sharedDBMutex"), eventTargetOpenedOn(WrapNotNull(GetCurrentSerialEventTarget())), - mIsStatementOnHelperThreadInterruptible(false), mDBConn(nullptr), mDefaultTransactionType(mozIStorageConnection::TRANSACTION_DEFERRED), mDestroying(false), @@ -589,8 +442,7 @@ Connection::Connection(Service* aService, int aFlags, aInterruptible), mIgnoreLockingMode(aIgnoreLockingMode), mAsyncExecutionThreadShuttingDown(false), - mConnectionClosed(false), - mGrowthChunkSize(0) { + mConnectionClosed(false) { MOZ_ASSERT(!mIgnoreLockingMode || mFlags & SQLITE_OPEN_READONLY, "Can't ignore locking for a non-readonly connection!"); mStorageService->registerConnection(this); @@ -1662,17 +1514,6 @@ Connection::AsyncClose(mozIStorageCompletionCallback* aCallback) { return NS_OK; } - // If we're closing the connection during shutdown, and there is an - // interruptible statement running on the helper thread, issue a - // sqlite3_interrupt() to avoid crashing when that statement takes a long - // time (for example a vacuum). - if (AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownConfirmed) && - mInterruptible && mIsStatementOnHelperThreadInterruptible) { - MOZ_ASSERT(!isClosing(), "Must not be closing, see Interrupt()"); - DebugOnly rv2 = Interrupt(); - MOZ_ASSERT(NS_SUCCEEDED(rv2)); - } - // setClosedState nullifies our connection pointer, so we take a raw pointer // off it, to pass it through the close procedure. sqlite3* nativeConn = mDBConn; @@ -1921,36 +1762,6 @@ Connection::Interrupt() { return NS_OK; } -NS_IMETHODIMP -Connection::AsyncVacuum(mozIStorageCompletionCallback* aCallback, - bool aUseIncremental, int32_t aSetPageSize) { - NS_ENSURE_TRUE(NS_IsMainThread(), NS_ERROR_NOT_SAME_THREAD); - // Abort if we're shutting down. - if (AppShutdown::IsInOrBeyond(ShutdownPhase::AppShutdownConfirmed)) { - return NS_ERROR_ABORT; - } - // Check if AsyncClose or Close were already invoked. - if (!connectionReady()) { - return NS_ERROR_NOT_INITIALIZED; - } - nsresult rv = ensureOperationSupported(ASYNCHRONOUS); - if (NS_FAILED(rv)) { - return rv; - } - nsIEventTarget* asyncThread = getAsyncExecutionTarget(); - if (!asyncThread) { - return NS_ERROR_NOT_INITIALIZED; - } - - // Create and dispatch our vacuum event to the background thread. - nsCOMPtr vacuumEvent = - new AsyncVacuumEvent(this, aCallback, aUseIncremental, aSetPageSize); - rv = asyncThread->Dispatch(vacuumEvent, NS_DISPATCH_NORMAL); - NS_ENSURE_SUCCESS(rv, rv); - - return NS_OK; -} - NS_IMETHODIMP Connection::GetDefaultPageSize(int32_t* _defaultPageSize) { *_defaultPageSize = Service::kDefaultPageSize; @@ -2469,59 +2280,15 @@ Connection::SetGrowthIncrement(int32_t aChunkSize, return NS_ERROR_FILE_TOO_BIG; } - int srv = ::sqlite3_file_control( - mDBConn, - aDatabaseName.Length() ? nsPromiseFlatCString(aDatabaseName).get() - : nullptr, - SQLITE_FCNTL_CHUNK_SIZE, &aChunkSize); - if (srv == SQLITE_OK) { - mGrowthChunkSize = aChunkSize; - } + (void)::sqlite3_file_control(mDBConn, + aDatabaseName.Length() + ? nsPromiseFlatCString(aDatabaseName).get() + : nullptr, + SQLITE_FCNTL_CHUNK_SIZE, &aChunkSize); #endif return NS_OK; } -int32_t Connection::RemovablePagesInFreeList(const nsACString& aSchemaName) { - int32_t freeListPagesCount = 0; - if (!isConnectionReadyOnThisThread()) { - MOZ_ASSERT(false, "Database connection is not ready"); - return freeListPagesCount; - } - { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA "); - query.Append(aSchemaName); - query.AppendLiteral(".freelist_count"); - nsCOMPtr stmt; - DebugOnly rv = CreateStatement(query, getter_AddRefs(stmt)); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - bool hasResult = false; - if (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { - freeListPagesCount = stmt->AsInt32(0); - } - } - // If there's no chunk size set, any page is good to be removed. - if (mGrowthChunkSize == 0 || freeListPagesCount == 0) { - return freeListPagesCount; - } - int32_t pageSize; - { - nsAutoCString query(MOZ_STORAGE_UNIQUIFY_QUERY_STR "PRAGMA "); - query.Append(aSchemaName); - query.AppendLiteral(".page_size"); - nsCOMPtr stmt; - DebugOnly rv = CreateStatement(query, getter_AddRefs(stmt)); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - bool hasResult = false; - if (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) { - pageSize = stmt->AsInt32(0); - } else { - MOZ_ASSERT(false, "Couldn't get page_size"); - return 0; - } - } - return std::max(0, freeListPagesCount - (mGrowthChunkSize / pageSize)); -} - NS_IMETHODIMP Connection::EnableModule(const nsACString& aModuleName) { if (!connectionReady()) { diff --git a/storage/mozStorageConnection.h b/storage/mozStorageConnection.h index b511e8d2c260..68877f817f36 100644 --- a/storage/mozStorageConnection.h +++ b/storage/mozStorageConnection.h @@ -317,25 +317,6 @@ class Connection final : public mozIStorageConnection, */ void RecordQueryStatus(int srv); - /** - * Returns the number of pages in the free list that can be removed. - * - * A database may use chunked growth to reduce filesystem fragmentation, then - * Sqlite will allocate and release multiple pages in chunks. We want to - * preserve the chunked space to reduce the likelihood of fragmentation, - * releasing free pages only when there's a large amount of them. This can be - * used to decide if it's worth vacuuming the database and how many pages can - * be vacuumed in case of incremental vacuum. - * Note this returns 0, and asserts, in case of errors. - */ - int32_t RemovablePagesInFreeList(const nsACString& aSchemaName); - - /** - * Whether the statement currently running on the helper thread can be - * interrupted. - */ - Atomic mIsStatementOnHelperThreadInterruptible; - private: ~Connection(); nsresult initializeInternal(); @@ -502,11 +483,6 @@ class Connection final : public mozIStorageConnection, * sharedAsyncExecutionMutex. */ bool mConnectionClosed; - - /** - * Stores the growth increment chunk size, set through SetGrowthIncrement(). - */ - Atomic mGrowthChunkSize; }; /** diff --git a/storage/test/moz.build b/storage/test/moz.build index 7265db4d7b4c..22f42e87bdcc 100644 --- a/storage/test/moz.build +++ b/storage/test/moz.build @@ -7,7 +7,3 @@ XPCSHELL_TESTS_MANIFESTS += ["unit/xpcshell.ini"] TEST_DIRS += ["gtest"] - -TESTING_JS_MODULES += [ - "unit/VacuumParticipant.sys.mjs", -] diff --git a/storage/test/unit/VacuumParticipant.sys.mjs b/storage/test/unit/VacuumParticipant.sys.mjs index 9f1c39826ecf..d513c31b5a88 100644 --- a/storage/test/unit/VacuumParticipant.sys.mjs +++ b/storage/test/unit/VacuumParticipant.sys.mjs @@ -4,106 +4,95 @@ // This testing component is used in test_vacuum* tests. -const CAT_NAME = "vacuum-participant"; -const CONTRACT_ID = "@unit.test.com/test-vacuum-participant;1"; +/** + * Returns a new nsIFile reference for a profile database. + * @param filename for the database, excluded the .sqlite extension. + */ +function new_db_file(name) { + let file = Services.dirsvc.get("ProfD", Ci.nsIFile); + file.append(name + ".sqlite"); + return file; +} -import { MockRegistrar } from "resource://testing-common/MockRegistrar.sys.mjs"; -import { TestUtils } from "resource://testing-common/TestUtils.sys.mjs"; +/** + * Opens and returns a connection to the provided database file. + * @param nsIFile interface to the database file. + */ +function getDatabase(aFile) { + return Services.storage.openDatabase(aFile); +} -export class VacuumParticipant { - #dbConn; - #expectedPageSize = 0; - #useIncrementalVacuum = false; - #grant = false; - - /** - * Build a VacuumParticipant instance. - * Note: After creation you must await instance.promiseRegistered() to ensure - * Category Caches have been updated. - * - * @param {mozIStorageAsyncConnection} databaseConnection - * The connection to be vacuumed. - * @param {Number} [expectedPageSize] - * Used to change the database page size. - * @param {boolean} [useIncrementalVacuum] - * Whether to enable incremental vacuum on the database. - * @param {boolean} [grant] - * Whether the vacuum operation should be granted. - */ - constructor( - databaseConnection, - { expectedPageSize = 0, useIncrementalVacuum = false, grant = true } = {} - ) { - this.#dbConn = databaseConnection; - - // Register as the only participant. - this.#unregisterAllParticipants(); - this.#registerAsParticipant(); - - this.#expectedPageSize = expectedPageSize; - this.#useIncrementalVacuum = useIncrementalVacuum; - this.#grant = grant; - - this.QueryInterface = ChromeUtils.generateQI([ - "mozIStorageVacuumParticipant", - ]); - } - - promiseRegistered() { - // The category manager dispatches change notifications to the main thread, - // so we must wait one tick. - return TestUtils.waitForTick(); - } - - #registerAsParticipant() { - MockRegistrar.register(CONTRACT_ID, this); - Services.catMan.addCategoryEntry( - CAT_NAME, - "vacuumParticipant", - CONTRACT_ID, - false, - false - ); - } - - #unregisterAllParticipants() { - // First unregister other participants. - for (let { data: entry } of Services.catMan.enumerateCategory(CAT_NAME)) { - Services.catMan.deleteCategoryEntry("vacuum-participant", entry, false); - } - } - - async dispose() { - this.#unregisterAllParticipants(); - MockRegistrar.unregister(CONTRACT_ID); - await new Promise(resolve => { - this.#dbConn.asyncClose(resolve); - }); - } +export function VacuumParticipant() { + this._dbConn = getDatabase(new_db_file("testVacuum")); + Services.obs.addObserver(this, "test-options"); +} +VacuumParticipant.prototype = { get expectedDatabasePageSize() { - return this.#expectedPageSize; - } - - get useIncrementalVacuum() { - return this.#useIncrementalVacuum; - } - + return this._dbConn.defaultPageSize; + }, get databaseConnection() { - return this.#dbConn; - } + return this._dbConn; + }, - onBeginVacuum() { - if (!this.#grant) { + _grant: true, + onBeginVacuum: function TVP_onBeginVacuum() { + if (!this._grant) { + this._grant = true; return false; } Services.obs.notifyObservers(null, "test-begin-vacuum"); return true; - } - onEndVacuum(succeeded) { - Services.obs.notifyObservers( - null, - succeeded ? "test-end-vacuum-success" : "test-end-vacuum-failure" - ); - } -} + }, + onEndVacuum: function TVP_EndVacuum(aSucceeded) { + if (this._stmt) { + this._stmt.finalize(); + } + Services.obs.notifyObservers(null, "test-end-vacuum", aSucceeded); + }, + + observe: function TVP_observe(aSubject, aTopic, aData) { + if (aData == "opt-out") { + this._grant = false; + } else if (aData == "wal") { + try { + this._dbConn.close(); + } catch (e) { + // Do nothing. + } + this._dbConn = getDatabase(new_db_file("testVacuum2")); + } else if (aData == "wal-fail") { + try { + this._dbConn.close(); + } catch (e) { + // Do nothing. + } + this._dbConn = getDatabase(new_db_file("testVacuum3")); + // Use WAL journal mode. + this._dbConn.executeSimpleSQL("PRAGMA journal_mode = WAL"); + // Create a not finalized statement. + this._stmt = this._dbConn.createStatement("SELECT :test"); + this._stmt.params.test = 1; + this._stmt.executeStep(); + } else if (aData == "memory") { + try { + this._dbConn.asyncClose(); + } catch (e) { + // Do nothing. + } + this._dbConn = Services.storage.openSpecialDatabase("memory"); + } else if (aData == "dispose") { + Services.obs.removeObserver(this, "test-options"); + try { + this._dbConn.asyncClose(); + } catch (e) { + // Do nothing. + } + } + }, + + QueryInterface: ChromeUtils.generateQI([ + "mozIStorageVacuumParticipant", + "nsIObserver", + ]), +}; diff --git a/storage/test/unit/head_storage.js b/storage/test/unit/head_storage.js index 8f7d826dc68a..22d217e0c887 100644 --- a/storage/test/unit/head_storage.js +++ b/storage/test/unit/head_storage.js @@ -11,10 +11,12 @@ var { AppConstants } = ChromeUtils.importESModule( ChromeUtils.defineESModuleGetters(this, { Sqlite: "resource://gre/modules/Sqlite.sys.mjs", - TelemetryTestUtils: "resource://testing-common/TelemetryTestUtils.sys.mjs", - TestUtils: "resource://testing-common/TestUtils.sys.mjs", }); +const { TelemetryTestUtils } = ChromeUtils.importESModule( + "resource://testing-common/TelemetryTestUtils.sys.mjs" +); + const OPEN_HISTOGRAM = "SQLITE_STORE_OPEN"; const QUERY_HISTOGRAM = "SQLITE_STORE_QUERY"; diff --git a/storage/test/unit/test_vacuum.js b/storage/test/unit/test_vacuum.js index 49dae98a8ba5..4971f577e13e 100644 --- a/storage/test/unit/test_vacuum.js +++ b/storage/test/unit/test_vacuum.js @@ -2,367 +2,325 @@ * http://creativecommons.org/publicdomain/zero/1.0/ */ -// This file tests the Vacuum Manager and asyncVacuum(). +// This file tests the Vacuum Manager. -const { VacuumParticipant } = ChromeUtils.importESModule( - "resource://testing-common/VacuumParticipant.sys.mjs" +const { MockRegistrar } = ChromeUtils.importESModule( + "resource://testing-common/MockRegistrar.sys.mjs" ); +/** + * Loads a test component that will register as a vacuum-participant. + * If other participants are found they will be unregistered, to avoid conflicts + * with the test itself. + */ +function load_test_vacuum_component() { + const CATEGORY_NAME = "vacuum-participant"; + const CONTRACT_ID = "@unit.test.com/test-vacuum-participant;1"; + + MockRegistrar.registerESM( + CONTRACT_ID, + "resource://test/VacuumParticipant.sys.mjs", + "VacuumParticipant" + ); + + let { catMan } = Services; + // Temporary unregister other participants for this test. + for (let { data: entry } of catMan.enumerateCategory(CATEGORY_NAME)) { + print("Check if the found category entry (" + entry + ") is expected."); + catMan.deleteCategoryEntry("vacuum-participant", entry, false); + } + catMan.addCategoryEntry( + CATEGORY_NAME, + "vacuumParticipant", + CONTRACT_ID, + false, + false + ); + print("Check the test entry exists."); +} + /** * Sends a fake idle-daily notification to the VACUUM Manager. */ function synthesize_idle_daily() { - Cc["@mozilla.org/storage/vacuum;1"] - .getService(Ci.nsIObserver) - .observe(null, "idle-daily", null); + let vm = Cc["@mozilla.org/storage/vacuum;1"].getService(Ci.nsIObserver); + vm.observe(null, "idle-daily", null); } /** * Returns a new nsIFile reference for a profile database. * @param filename for the database, excluded the .sqlite extension. */ -function new_db_file(name = "testVacuum") { +function new_db_file(name) { let file = Services.dirsvc.get("ProfD", Ci.nsIFile); file.append(name + ".sqlite"); return file; } -function reset_vacuum_date(name = "testVacuum") { - let date = parseInt(Date.now() / 1000 - 31 * 86400); - // Set last VACUUM to a date in the past. - Services.prefs.setIntPref(`storage.vacuum.last.${name}.sqlite`, date); - return date; -} +function run_test() { + do_test_pending(); -function get_vacuum_date(name = "testVacuum") { - return Services.prefs.getIntPref(`storage.vacuum.last.${name}.sqlite`, 0); -} - -add_setup(async function() { - // turn on Cu.isInAutomation - Services.prefs.setBoolPref( - "security.turn_off_all_security_so_that_viruses_can_take_over_this_computer", - true - ); -}); - -add_task(async function test_common_vacuum() { - let last_vacuum_date = reset_vacuum_date(); - info("Test that a VACUUM correctly happens and all notifications are fired."); - let promiseTestVacuumBegin = TestUtils.topicObserved("test-begin-vacuum"); - let promiseTestVacuumEnd = TestUtils.topicObserved("test-end-vacuum-success"); - let promiseVacuumBegin = TestUtils.topicObserved("vacuum-begin"); - let promiseVacuumEnd = TestUtils.topicObserved("vacuum-end"); - - let participant = new VacuumParticipant( - Services.storage.openDatabase(new_db_file()) - ); - await participant.promiseRegistered(); - synthesize_idle_daily(); - // Wait for notifications. - await Promise.all([ - promiseTestVacuumBegin, - promiseTestVacuumEnd, - promiseVacuumBegin, - promiseVacuumEnd, - ]); - Assert.greater(get_vacuum_date(), last_vacuum_date); - await participant.dispose(); -}); - -add_task(async function test_skipped_if_recent_vacuum() { - info("Test that a VACUUM is skipped if it was run recently."); - Services.prefs.setIntPref( - "storage.vacuum.last.testVacuum.sqlite", - parseInt(Date.now() / 1000) - ); - // Wait for VACUUM skipped notification. - let promiseSkipped = TestUtils.topicObserved("vacuum-skip"); - - let participant = new VacuumParticipant( - Services.storage.openDatabase(new_db_file()) - ); - await participant.promiseRegistered(); - synthesize_idle_daily(); - - // Check that VACUUM has been skipped. - await promiseSkipped; - - await participant.dispose(); -}); - -add_task(async function test_page_size_change() { - info("Test that a VACUUM changes page_size"); - reset_vacuum_date(); - - let conn = Services.storage.openDatabase(new_db_file()); - info("Check initial page size."); + // Change initial page size. Do it immediately since it would require an + // additional vacuum op to do it later. As a bonus this makes the page size + // change test really fast since it only has to check results. + let conn = getDatabase(new_db_file("testVacuum")); + conn.executeSimpleSQL("PRAGMA page_size = 1024"); + print("Check current page size."); let stmt = conn.createStatement("PRAGMA page_size"); - Assert.ok(stmt.executeStep()); - Assert.equal(stmt.row.page_size, conn.defaultPageSize); - stmt.finalize(); - await populateFreeList(conn); - - let participant = new VacuumParticipant(conn, { expectedPageSize: 1024 }); - await participant.promiseRegistered(); - let promiseVacuumEnd = TestUtils.topicObserved("test-end-vacuum-success"); - synthesize_idle_daily(); - await promiseVacuumEnd; - - info("Check that page size was updated."); - stmt = conn.createStatement("PRAGMA page_size"); - Assert.ok(stmt.executeStep()); - Assert.equal(stmt.row.page_size, 1024); - stmt.finalize(); - - await participant.dispose(); -}); - -add_task(async function test_skipped_optout_vacuum() { - info("Test that a VACUUM is skipped if the participant wants to opt-out."); - reset_vacuum_date(); - - let participant = new VacuumParticipant( - Services.storage.openDatabase(new_db_file()), - { grant: false } - ); - await participant.promiseRegistered(); - // Wait for VACUUM skipped notification. - let promiseSkipped = TestUtils.topicObserved("vacuum-skip"); - - synthesize_idle_daily(); - - // Check that VACUUM has been skipped. - await promiseSkipped; - - await participant.dispose(); -}); - -add_task(async function test_memory_database_crash() { - info("Test that we don't crash trying to vacuum a memory database"); - reset_vacuum_date(); - - let participant = new VacuumParticipant( - Services.storage.openSpecialDatabase("memory") - ); - await participant.promiseRegistered(); - // Wait for VACUUM skipped notification. - let promiseSkipped = TestUtils.topicObserved("vacuum-skip"); - - synthesize_idle_daily(); - - // Check that VACUUM has been skipped. - await promiseSkipped; - - await participant.dispose(); -}); - -add_task(async function test_async_connection() { - info("Test we can vacuum an async connection"); - reset_vacuum_date(); - - let conn = await openAsyncDatabase(new_db_file()); - await populateFreeList(conn); - let participant = new VacuumParticipant(conn); - await participant.promiseRegistered(); - - let promiseVacuumEnd = TestUtils.topicObserved("test-end-vacuum-success"); - synthesize_idle_daily(); - await promiseVacuumEnd; - - await participant.dispose(); -}); - -add_task(async function test_change_to_incremental_vacuum() { - info("Test we can change to incremental vacuum"); - reset_vacuum_date(); - - let conn = Services.storage.openDatabase(new_db_file()); - info("Check initial vacuum."); - let stmt = conn.createStatement("PRAGMA auto_vacuum"); - Assert.ok(stmt.executeStep()); - Assert.equal(stmt.row.auto_vacuum, 0); - stmt.finalize(); - await populateFreeList(conn); - - let participant = new VacuumParticipant(conn, { useIncrementalVacuum: true }); - await participant.promiseRegistered(); - let promiseVacuumEnd = TestUtils.topicObserved("test-end-vacuum-success"); - synthesize_idle_daily(); - await promiseVacuumEnd; - - info("Check that auto_vacuum was updated."); - stmt = conn.createStatement("PRAGMA auto_vacuum"); - Assert.ok(stmt.executeStep()); - Assert.equal(stmt.row.auto_vacuum, 2); - stmt.finalize(); - - await participant.dispose(); -}); - -add_task(async function test_change_from_incremental_vacuum() { - info("Test we can change from incremental vacuum"); - reset_vacuum_date(); - - let conn = Services.storage.openDatabase(new_db_file()); - conn.executeSimpleSQL("PRAGMA auto_vacuum = 2"); - info("Check initial vacuum."); - let stmt = conn.createStatement("PRAGMA auto_vacuum"); - Assert.ok(stmt.executeStep()); - Assert.equal(stmt.row.auto_vacuum, 2); - stmt.finalize(); - await populateFreeList(conn); - - let participant = new VacuumParticipant(conn, { - useIncrementalVacuum: false, - }); - await participant.promiseRegistered(); - let promiseVacuumEnd = TestUtils.topicObserved("test-end-vacuum-success"); - synthesize_idle_daily(); - await promiseVacuumEnd; - - info("Check that auto_vacuum was updated."); - stmt = conn.createStatement("PRAGMA auto_vacuum"); - Assert.ok(stmt.executeStep()); - Assert.equal(stmt.row.auto_vacuum, 0); - stmt.finalize(); - - await participant.dispose(); -}); - -add_task(async function test_attached_vacuum() { - info("Test attached database is not a problem"); - reset_vacuum_date(); - - let conn = Services.storage.openDatabase(new_db_file()); - let conn2 = Services.storage.openDatabase(new_db_file("attached")); - - info("Attach " + conn2.databaseFile.path); - conn.executeSimpleSQL( - `ATTACH DATABASE '${conn2.databaseFile.path}' AS attached` - ); - await asyncClose(conn2); - let stmt = conn.createStatement("PRAGMA database_list"); - let schemas = []; - while (stmt.executeStep()) { - schemas.push(stmt.row.name); + try { + while (stmt.executeStep()) { + Assert.equal(stmt.row.page_size, 1024); + } + } finally { + stmt.finalize(); } - Assert.deepEqual(schemas, ["main", "attached"]); - stmt.finalize(); - await populateFreeList(conn); - await populateFreeList(conn, "attached"); + load_test_vacuum_component(); - let participant = new VacuumParticipant(conn); - await participant.promiseRegistered(); - let promiseVacuumEnd = TestUtils.topicObserved("test-end-vacuum-success"); - synthesize_idle_daily(); - await promiseVacuumEnd; - - await participant.dispose(); -}); - -add_task(async function test_vacuum_fail() { - info("Test a failed vacuum"); - reset_vacuum_date(); - - let conn = Services.storage.openDatabase(new_db_file()); - // Cannot vacuum in a transaction. - conn.beginTransaction(); - await populateFreeList(conn); - - let participant = new VacuumParticipant(conn); - await participant.promiseRegistered(); - let promiseVacuumEnd = TestUtils.topicObserved("test-end-vacuum-failure"); - synthesize_idle_daily(); - await promiseVacuumEnd; - - conn.commitTransaction(); - await participant.dispose(); -}); - -add_task(async function test_async_vacuum() { - // Since previous tests already go through most cases, this only checks - // the basics of directly calling asyncVacuum(). - info("Test synchronous connection"); - let conn = Services.storage.openDatabase(new_db_file()); - await populateFreeList(conn); - let rv = await new Promise(resolve => { - conn.asyncVacuum(status => { - resolve(status); - }); - }); - Assert.ok(Components.isSuccessCode(rv)); - await asyncClose(conn); - - info("Test asynchronous connection"); - conn = await openAsyncDatabase(new_db_file()); - await populateFreeList(conn); - rv = await new Promise(resolve => { - conn.asyncVacuum(status => { - resolve(status); - }); - }); - Assert.ok(Components.isSuccessCode(rv)); - await asyncClose(conn); -}); - -add_task(async function test_vacuum_growth() { - // Tests vacuum doesn't nullify chunked growth. - let conn = Services.storage.openDatabase(new_db_file("incremental")); - conn.executeSimpleSQL("PRAGMA auto_vacuum = INCREMENTAL"); - conn.setGrowthIncrement(2 * conn.defaultPageSize, ""); - await populateFreeList(conn); - let stmt = conn.createStatement("PRAGMA freelist_count"); - let count = 0; - Assert.ok(stmt.executeStep()); - count = stmt.row.freelist_count; - stmt.reset(); - Assert.greater(count, 2, "There's more than 2 page in freelist"); - - let rv = await new Promise(resolve => { - conn.asyncVacuum(status => { - resolve(status); - }, true); - }); - Assert.ok(Components.isSuccessCode(rv)); - - Assert.ok(stmt.executeStep()); - Assert.equal( - stmt.row.freelist_count, - 2, - "chunked growth space was preserved" - ); - stmt.reset(); - - // A full vacuuum should not be executed if there's less free pages than - // chunked growth. - rv = await new Promise(resolve => { - conn.asyncVacuum(status => { - resolve(status); - }); - }); - Assert.ok(Components.isSuccessCode(rv)); - - Assert.ok(stmt.executeStep()); - Assert.equal( - stmt.row.freelist_count, - 2, - "chunked growth space was preserved" - ); - stmt.finalize(); - - await asyncClose(conn); -}); - -async function populateFreeList(conn, schema = "main") { - await executeSimpleSQLAsync(conn, `CREATE TABLE ${schema}.test (id TEXT)`); - await executeSimpleSQLAsync( - conn, - `INSERT INTO ${schema}.test - VALUES ${Array.from({ length: 3000 }, () => Math.random()).map( - v => "('" + v + "')" - )}` - ); - await executeSimpleSQLAsync(conn, `DROP TABLE ${schema}.test`); + run_next_test(); +} + +const TESTS = [ + function test_common_vacuum() { + print( + "\n*** Test that a VACUUM correctly happens and all notifications are fired." + ); + // Wait for VACUUM begin. + let beginVacuumReceived = false; + Services.obs.addObserver(function onVacuum(aSubject, aTopic, aData) { + Services.obs.removeObserver(onVacuum, aTopic); + beginVacuumReceived = true; + }, "test-begin-vacuum"); + + // Wait for heavy IO notifications. + let heavyIOTaskBeginReceived = false; + let heavyIOTaskEndReceived = false; + Services.obs.addObserver(function onVacuum(aSubject, aTopic, aData) { + if (heavyIOTaskBeginReceived && heavyIOTaskEndReceived) { + Services.obs.removeObserver(onVacuum, aTopic); + } + + if (aData == "vacuum-begin") { + heavyIOTaskBeginReceived = true; + } else if (aData == "vacuum-end") { + heavyIOTaskEndReceived = true; + } + }, "heavy-io-task"); + + // Wait for VACUUM end. + Services.obs.addObserver(function onVacuum(aSubject, aTopic, aData) { + Services.obs.removeObserver(onVacuum, aTopic); + print("Check we received onBeginVacuum"); + Assert.ok(beginVacuumReceived); + print("Check we received heavy-io-task notifications"); + Assert.ok(heavyIOTaskBeginReceived); + Assert.ok(heavyIOTaskEndReceived); + print("Received onEndVacuum"); + run_next_test(); + }, "test-end-vacuum"); + + synthesize_idle_daily(); + }, + + function test_skipped_if_recent_vacuum() { + print("\n*** Test that a VACUUM is skipped if it was run recently."); + Services.prefs.setIntPref( + "storage.vacuum.last.testVacuum.sqlite", + parseInt(Date.now() / 1000) + ); + + // Wait for VACUUM begin. + let vacuumObserver = { + gotNotification: false, + observe: function VO_observe(aSubject, aTopic, aData) { + this.gotNotification = true; + }, + QueryInterface: ChromeUtils.generateQI(["nsIObserver"]), + }; + Services.obs.addObserver(vacuumObserver, "test-begin-vacuum"); + + // Check after a couple seconds that no VACUUM has been run. + do_timeout(2000, function() { + print("Check VACUUM did not run."); + Assert.ok(!vacuumObserver.gotNotification); + Services.obs.removeObserver(vacuumObserver, "test-begin-vacuum"); + run_next_test(); + }); + + synthesize_idle_daily(); + }, + + function test_page_size_change() { + print("\n*** Test that a VACUUM changes page_size"); + + // We did setup the database with a small page size, the previous vacuum + // should have updated it. + print("Check that page size was updated."); + let conn = getDatabase(new_db_file("testVacuum")); + let stmt = conn.createStatement("PRAGMA page_size"); + try { + while (stmt.executeStep()) { + Assert.equal(stmt.row.page_size, conn.defaultPageSize); + } + } finally { + stmt.finalize(); + } + + run_next_test(); + }, + + function test_skipped_optout_vacuum() { + print( + "\n*** Test that a VACUUM is skipped if the participant wants to opt-out." + ); + Services.obs.notifyObservers(null, "test-options", "opt-out"); + + // Wait for VACUUM begin. + let vacuumObserver = { + gotNotification: false, + observe: function VO_observe(aSubject, aTopic, aData) { + this.gotNotification = true; + }, + QueryInterface: ChromeUtils.generateQI(["nsIObserver"]), + }; + Services.obs.addObserver(vacuumObserver, "test-begin-vacuum"); + + // Check after a couple seconds that no VACUUM has been run. + do_timeout(2000, function() { + print("Check VACUUM did not run."); + Assert.ok(!vacuumObserver.gotNotification); + Services.obs.removeObserver(vacuumObserver, "test-begin-vacuum"); + run_next_test(); + }); + + synthesize_idle_daily(); + }, + + /* Changing page size on WAL is not supported till Bug 634374 is properly fixed. + function test_page_size_change_with_wal() + { + print("\n*** Test that a VACUUM changes page_size with WAL mode"); + Services.obs.notifyObservers(null, "test-options", "wal"); + + // Set a small page size. + let conn = getDatabase(new_db_file("testVacuum2")); + conn.executeSimpleSQL("PRAGMA page_size = 1024"); + let stmt = conn.createStatement("PRAGMA page_size"); + try { + while (stmt.executeStep()) { + do_check_eq(stmt.row.page_size, 1024); + } + } + finally { + stmt.finalize(); + } + + // Use WAL journal mode. + conn.executeSimpleSQL("PRAGMA journal_mode = WAL"); + stmt = conn.createStatement("PRAGMA journal_mode"); + try { + while (stmt.executeStep()) { + do_check_eq(stmt.row.journal_mode, "wal"); + } + } + finally { + stmt.finalize(); + } + + // Wait for VACUUM end. + let vacuumObserver = { + observe: function VO_observe(aSubject, aTopic, aData) { + Services.obs.removeObserver(this, aTopic); + print("Check page size has been updated."); + let stmt = conn.createStatement("PRAGMA page_size"); + try { + while (stmt.executeStep()) { + do_check_eq(stmt.row.page_size, Ci.mozIStorageConnection.DEFAULT_PAGE_SIZE); + } + } + finally { + stmt.finalize(); + } + + print("Check journal mode has been restored."); + stmt = conn.createStatement("PRAGMA journal_mode"); + try { + while (stmt.executeStep()) { + do_check_eq(stmt.row.journal_mode, "wal"); + } + } + finally { + stmt.finalize(); + } + + run_next_test(); + }, + QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver]) + } + Services.obs.addObserver(vacuumObserver, "test-end-vacuum", false); + + synthesize_idle_daily(); + }, + */ + + function test_memory_database_crash() { + print("\n*** Test that we don't crash trying to vacuum a memory database"); + Services.obs.notifyObservers(null, "test-options", "memory"); + + // Wait for VACUUM begin. + let vacuumObserver = { + gotNotification: false, + observe: function VO_observe(aSubject, aTopic, aData) { + this.gotNotification = true; + }, + QueryInterface: ChromeUtils.generateQI(["nsIObserver"]), + }; + Services.obs.addObserver(vacuumObserver, "test-begin-vacuum"); + + // Check after a couple seconds that no VACUUM has been run. + do_timeout(2000, function() { + print("Check VACUUM did not run."); + Assert.ok(!vacuumObserver.gotNotification); + Services.obs.removeObserver(vacuumObserver, "test-begin-vacuum"); + run_next_test(); + }); + + synthesize_idle_daily(); + }, + + /* Changing page size on WAL is not supported till Bug 634374 is properly fixed. + function test_wal_restore_fail() + { + print("\n*** Test that a failing WAL restoration notifies failure"); + Services.obs.notifyObservers(null, "test-options", "wal-fail"); + + // Wait for VACUUM end. + let vacuumObserver = { + observe: function VO_observe(aSubject, aTopic, aData) { + Services.obs.removeObserver(vacuumObserver, "test-end-vacuum"); + print("Check WAL restoration failed."); + do_check_false(aData); + run_next_test(); + }, + QueryInterface: ChromeUtils.generateQI([Ci.nsIObserver]) + } + Services.obs.addObserver(vacuumObserver, "test-end-vacuum", false); + + synthesize_idle_daily(); + }, + */ +]; + +function run_next_test() { + if (!TESTS.length) { + Services.obs.notifyObservers(null, "test-options", "dispose"); + do_test_finished(); + } else { + // Set last VACUUM to a date in the past. + Services.prefs.setIntPref( + "storage.vacuum.last.testVacuum.sqlite", + parseInt(Date.now() / 1000 - 31 * 86400) + ); + executeSoon(TESTS.shift()); + } } diff --git a/testing/modules/MockRegistrar.sys.mjs b/testing/modules/MockRegistrar.sys.mjs index 2241c129caf1..1262ec0af6ed 100644 --- a/testing/modules/MockRegistrar.sys.mjs +++ b/testing/modules/MockRegistrar.sys.mjs @@ -90,6 +90,13 @@ export var MockRegistrar = Object.freeze({ return cid; }, + registerESM(contractID, esmPath, symbol) { + return this.register(contractID, () => { + let exports = ChromeUtils.importESModule(esmPath); + return new exports[symbol](); + }); + }, + /** * Unregister the mock. * diff --git a/toolkit/components/places/nsNavHistory.cpp b/toolkit/components/places/nsNavHistory.cpp index e09e5db28e07..0c964a7b42db 100644 --- a/toolkit/components/places/nsNavHistory.cpp +++ b/toolkit/components/places/nsNavHistory.cpp @@ -1825,18 +1825,8 @@ nsNavHistory::SetShouldStartFrecencyRecalculation(bool aVal) { //// mozIStorageVacuumParticipant NS_IMETHODIMP -nsNavHistory::GetDatabaseConnection( - mozIStorageAsyncConnection** _DBConnection) { - NS_ENSURE_ARG_POINTER(_DBConnection); - nsCOMPtr connection = mDB->MainConn(); - connection.forget(_DBConnection); - return NS_OK; -} - -NS_IMETHODIMP -nsNavHistory::GetUseIncrementalVacuum(bool* _useIncremental) { - *_useIncremental = false; - return NS_OK; +nsNavHistory::GetDatabaseConnection(mozIStorageConnection** _DBConnection) { + return GetDBConnection(_DBConnection); } NS_IMETHODIMP diff --git a/toolkit/modules/Sqlite.sys.mjs b/toolkit/modules/Sqlite.sys.mjs index 210e63746281..5fe623d953e4 100644 --- a/toolkit/modules/Sqlite.sys.mjs +++ b/toolkit/modules/Sqlite.sys.mjs @@ -255,43 +255,6 @@ XPCOMUtils.defineLazyGetter(lazy, "Barriers", () => { return Barriers; }); -const VACUUM_CATEGORY = "vacuum-participant"; -const VACUUM_CONTRACTID = "@sqlite.module.js/vacuum-participant;"; -var registeredVacuumParticipants = new Map(); - -function registerVacuumParticipant(connectionData) { - let contractId = VACUUM_CONTRACTID + connectionData._identifier; - let factory = { - createInstance(iid) { - return connectionData.QueryInterface(iid); - }, - QueryInterface: ChromeUtils.generateQI(["nsIFactory"]), - }; - let cid = Services.uuid.generateUUID(); - Components.manager - .QueryInterface(Ci.nsIComponentRegistrar) - .registerFactory(cid, contractId, contractId, factory); - Services.catMan.addCategoryEntry( - VACUUM_CATEGORY, - contractId, - contractId, - false, - false - ); - registeredVacuumParticipants.set(contractId, { cid, factory }); -} - -function unregisterVacuumParticipant(connectionData) { - let contractId = VACUUM_CONTRACTID + connectionData._identifier; - let component = registeredVacuumParticipants.get(contractId); - if (component) { - Components.manager - .QueryInterface(Ci.nsIComponentRegistrar) - .unregisterFactory(component.cid, component.factory); - Services.catMan.deleteCategoryEntry(VACUUM_CATEGORY, contractId, false); - } -} - /** * Connection data with methods necessary for closing the connection. * @@ -393,33 +356,6 @@ function ConnectionData(connection, identifier, options = {}) { this._timeoutPromise = null; // The last timestamp when we should consider using `this._timeoutPromise`. this._timeoutPromiseExpires = 0; - - this._useIncrementalVacuum = !!options.incrementalVacuum; - if (this._useIncrementalVacuum) { - this._log.debug("Set auto_vacuum INCREMENTAL"); - this.execute("PRAGMA auto_vacuum = 2").catch(ex => { - this._log.error("Setting auto_vacuum to INCREMENTAL failed."); - console.error(ex); - }); - } - - this._expectedPageSize = options.pageSize ?? 0; - if (this._expectedPageSize) { - this._log.debug("Set page_size to " + this._expectedPageSize); - this.execute("PRAGMA page_size = " + this._expectedPageSize).catch(ex => { - this._log.error(`Setting page_size to ${this._expectedPageSize} failed.`); - console.error(ex); - }); - } - - this._vacuumOnIdle = options.vacuumOnIdle; - if (this._vacuumOnIdle) { - this._log.debug("Register as vacuum participant"); - this.QueryInterface = ChromeUtils.generateQI([ - Ci.mozIStorageVacuumParticipant, - ]); - registerVacuumParticipant(this); - } } /** @@ -435,35 +371,6 @@ function ConnectionData(connection, identifier, options = {}) { ConnectionData.byId = new Map(); ConnectionData.prototype = Object.freeze({ - get expectedDatabasePageSize() { - return this._expectedPageSize; - }, - - get useIncrementalVacuum() { - return this._useIncrementalVacuum; - }, - - /** - * This should only be used by the VacuumManager component. - * @see unsafeRawConnection for an official (but still unsafe) API. - */ - get databaseConnection() { - if (this._vacuumOnIdle) { - return this._dbConn; - } - return null; - }, - - onBeginVacuum() { - let granted = !this.transactionInProgress; - this._log.debug("Begin Vacuum - " + granted ? "granted" : "denied"); - return granted; - }, - - onEndVacuum(succeeded) { - this._log.debug("End Vacuum - " + succeeded ? "success" : "failure"); - }, - /** * Run a task, ensuring that its execution will not be interrupted by shutdown. * @@ -586,11 +493,6 @@ ConnectionData.prototype = Object.freeze({ this._log.debug("Request to close connection."); this._clearIdleShrinkTimer(); - if (this._vacuumOnIdle) { - this._log.debug("Unregister as vacuum participant"); - unregisterVacuumParticipant(this); - } - return this._barrier.wait().then(() => { if (!this._dbConn) { return undefined; @@ -901,9 +803,8 @@ ConnectionData.prototype = Object.freeze({ shrinkMemory() { this._log.debug("Shrinking memory usage."); - return this.execute("PRAGMA shrink_memory").finally(() => { - this._clearIdleShrinkTimer(); - }); + let onShrunk = this._clearIdleShrinkTimer.bind(this); + return this.execute("PRAGMA shrink_memory").then(onShrunk, onShrunk); }, discardCachedStatements() { @@ -1181,24 +1082,6 @@ ConnectionData.prototype = Object.freeze({ * USE WITH EXTREME CAUTION. This mode WILL produce incorrect results or * return "false positive" corruption errors if other connections write * to the DB at the same time. - * - * vacuumOnIdle -- (bool) Whether to register this connection to be vacuumed - * on idle by the VacuumManager component. - * If you're vacuum-ing an incremental vacuum database, ensure to also - * set incrementalVacuum to true, otherwise this will try to change it - * to full vacuum mode. - * - * incrementalVacuum -- (bool) if set to true auto_vacuum = INCREMENTAL will - * be enabled for the database. - * Changing auto vacuum of an already populated database requires a full - * VACUUM. You can evaluate to enable vacuumOnIdle for that. - * - * pageSize -- (integer) This allows to set a custom page size for the - * database. It is usually not necessary to set it, since the default - * value should be good for most consumers. - * Changing the page size of an already populated database requires a full - * VACUUM. You can evaluate to enable vacuumOnIdle for that. - * * testDelayedOpenPromise -- (promise) Used by tests to delay the open * callback handling and execute code between asyncOpen and its callback. * @@ -1280,33 +1163,6 @@ function openConnection(options) { openedOptions.defaultTransactionType = defaultTransactionType; } - if ("vacuumOnIdle" in options) { - if (typeof options.vacuumOnIdle != "boolean") { - throw new Error("Invalid vacuumOnIdle: " + options.vacuumOnIdle); - } - openedOptions.vacuumOnIdle = options.vacuumOnIdle; - } - - if ("incrementalVacuum" in options) { - if (typeof options.incrementalVacuum != "boolean") { - throw new Error( - "Invalid incrementalVacuum: " + options.incrementalVacuum - ); - } - openedOptions.incrementalVacuum = options.incrementalVacuum; - } - - if ("pageSize" in options) { - if ( - ![512, 1024, 2048, 4096, 8192, 16384, 32768, 65536].includes( - options.pageSize - ) - ) { - throw new Error("Invalid pageSize: " + options.pageSize); - } - openedOptions.pageSize = options.pageSize; - } - let identifier = getIdentifierByFileName(PathUtils.filename(path)); log.debug("Opening database: " + path + " (" + identifier + ")"); diff --git a/toolkit/modules/tests/xpcshell/test_sqlite.js b/toolkit/modules/tests/xpcshell/test_sqlite.js index 17556ca140aa..9961ace6af5c 100644 --- a/toolkit/modules/tests/xpcshell/test_sqlite.js +++ b/toolkit/modules/tests/xpcshell/test_sqlite.js @@ -1381,19 +1381,3 @@ add_task(async function test_interrupt() { "Sqlite.interrupt() should throw on a closed connection" ); }); - -add_task(async function test_pageSize() { - // Testing the possibility to set the page size on database creation. - await Assert.rejects( - getDummyDatabase("pagesize", { pageSize: 1234 }), - /Invalid pageSize/, - "Check invalid pageSize value" - ); - let c = await getDummyDatabase("pagesize", { pageSize: 8192 }); - Assert.equal( - (await c.execute("PRAGMA page_size"))[0].getResultByIndex(0), - 8192, - "Check page size was set" - ); - await c.close(); -}); diff --git a/toolkit/modules/tests/xpcshell/test_sqlite_autoVacuum.js b/toolkit/modules/tests/xpcshell/test_sqlite_autoVacuum.js deleted file mode 100644 index f3a6dced3fb8..000000000000 --- a/toolkit/modules/tests/xpcshell/test_sqlite_autoVacuum.js +++ /dev/null @@ -1,96 +0,0 @@ -"use strict"; - -const { Sqlite } = ChromeUtils.importESModule( - "resource://gre/modules/Sqlite.sys.mjs" -); -const { TestUtils } = ChromeUtils.importESModule( - "resource://testing-common/TestUtils.sys.mjs" -); - -/** - * Sends a fake idle-daily notification to the VACUUM Manager. - */ -function synthesize_idle_daily() { - Cc["@mozilla.org/storage/vacuum;1"] - .getService(Ci.nsIObserver) - .observe(null, "idle-daily", null); -} - -function unregister_vacuum_participants() { - // First unregister other participants. - for (let { data: entry } of Services.catMan.enumerateCategory( - "vacuum-participant" - )) { - Services.catMan.deleteCategoryEntry("vacuum-participant", entry, false); - } -} - -function reset_vacuum_date(dbname) { - let date = parseInt(Date.now() / 1000 - 31 * 86400); - // Set last VACUUM to a date in the past. - Services.prefs.setIntPref(`storage.vacuum.last.${dbname}`, date); - return date; -} - -function get_vacuum_date(dbname) { - return Services.prefs.getIntPref(`storage.vacuum.last.${dbname}`, 0); -} - -async function get_freelist_count(conn) { - return (await conn.execute("PRAGMA freelist_count"))[0].getResultByIndex(0); -} - -async function get_auto_vacuum(conn) { - return (await conn.execute("PRAGMA auto_vacuum"))[0].getResultByIndex(0); -} - -async function test_vacuum(options = {}) { - unregister_vacuum_participants(); - const dbName = "testVacuum.sqlite"; - const dbFile = PathUtils.join(PathUtils.profileDir, dbName); - let lastVacuumDate = reset_vacuum_date(dbName); - let conn = await Sqlite.openConnection( - Object.assign( - { - path: dbFile, - vacuumOnIdle: true, - }, - options - ) - ); - // Ensure the category manager is up-to-date. - await TestUtils.waitForTick(); - - Assert.equal( - await get_auto_vacuum(conn), - options.incrementalVacuum ? 2 : 0, - "Check auto_vacuum" - ); - - // Generate some freelist page. - await conn.execute("CREATE TABLE test (id INTEGER)"); - await conn.execute("DROP TABLE test"); - Assert.greater(await get_freelist_count(conn), 0, "Check freelist_count"); - - let promiseVacuumEnd = TestUtils.topicObserved( - "vacuum-end", - (_, d) => d == dbName - ); - synthesize_idle_daily(); - info("Await vacuum end"); - await promiseVacuumEnd; - - Assert.greater(get_vacuum_date(dbName), lastVacuumDate); - - Assert.equal(await get_freelist_count(conn), 0, "Check freelist_count"); - - await conn.close(); - await IOUtils.remove(dbFile); -} - -add_task(async function test_vacuumOnIdle() { - info("Test full vacuum"); - await test_vacuum(); - info("Test incremental vacuum"); - await test_vacuum({ incrementalVacuum: true }); -}); diff --git a/toolkit/modules/tests/xpcshell/xpcshell.ini b/toolkit/modules/tests/xpcshell/xpcshell.ini index 75e9fd0a81c1..eafb8f1e86e6 100644 --- a/toolkit/modules/tests/xpcshell/xpcshell.ini +++ b/toolkit/modules/tests/xpcshell/xpcshell.ini @@ -60,8 +60,6 @@ run-sequentially = very high failure rate in parallel [test_Services.js] [test_sqlite.js] skip-if = toolkit == 'android' -[test_sqlite_autoVacuum.js] -skip-if = toolkit == 'android' [test_sqlite_shutdown.js] [test_timer.js] [test_UpdateUtils_url.js]