Skip to content

Commit

Permalink
Bug 1791767 - Interrupt IDB idle maintenance with atomic flag. r=janv…
Browse files Browse the repository at this point in the history 
…,dom-storage-reviewers,jstutte

Previously, idle maintenance was interrupted whenever a new runnable was
posted to the executing thread. Here we replace this interrupt signal with
an explicit boolean flag in order to support task queue event targets.

Differential Revision: https://phabricator.services.mozilla.com/D165984
  • Loading branch information
@jjjalkanen
jjjalkanen committed Mar 5, 2024
1 parent 879834b commit cccef63
Showing 1 changed file with 41 additions and 40 deletions.
81 changes: 41 additions & 40 deletions dom/indexedDB/ActorsParent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1164,7 +1164,8 @@ class DatabaseConnection final : public CachingDatabaseConnection {
return CheckpointInternal(CheckpointMode::Full);
}

void DoIdleProcessing(bool aNeedsCheckpoint);
void DoIdleProcessing(bool aNeedsCheckpoint,
const Atomic<bool>& aInterrupted);

void Close();

Expand All @@ -1191,7 +1192,8 @@ class DatabaseConnection final : public CachingDatabaseConnection {
*/
Result<bool, nsresult> ReclaimFreePagesWhileIdle(
CachedStatement& aFreelistStatement, CachedStatement& aRollbackStatement,
uint32_t aFreelistCount, bool aNeedsCheckpoint);
uint32_t aFreelistCount, bool aNeedsCheckpoint,
const Atomic<bool>& aInterrupted);

Result<int64_t, nsresult> GetFileSize(const nsAString& aPath);
};
Expand Down Expand Up @@ -1590,6 +1592,7 @@ class ConnectionPool::ConnectionRunnable : public Runnable {

class ConnectionPool::IdleConnectionRunnable final : public ConnectionRunnable {
const bool mNeedsCheckpoint;
Atomic<bool> mInterrupted;

public:
IdleConnectionRunnable(DatabaseInfo& aDatabaseInfo, bool aNeedsCheckpoint)
Expand All @@ -1598,6 +1601,8 @@ class ConnectionPool::IdleConnectionRunnable final : public ConnectionRunnable {
NS_INLINE_DECL_REFCOUNTING_INHERITED(IdleConnectionRunnable,
ConnectionRunnable)

void Interrupt() { mInterrupted = true; }

private:
~IdleConnectionRunnable() override = default;

Expand Down Expand Up @@ -6887,7 +6892,8 @@ nsresult DatabaseConnection::CheckpointInternal(CheckpointMode aMode) {
return NS_OK;
}

void DatabaseConnection::DoIdleProcessing(bool aNeedsCheckpoint) {
void DatabaseConnection::DoIdleProcessing(bool aNeedsCheckpoint,
const Atomic<bool>& aInterrupted) {
AssertIsOnConnectionThread();
MOZ_ASSERT(mInReadTransaction);
MOZ_ASSERT(!mInWriteTransaction);
Expand All @@ -6912,22 +6918,23 @@ void DatabaseConnection::DoIdleProcessing(bool aNeedsCheckpoint) {
mInReadTransaction = false;
}

const bool freedSomePages = freelistCount && [this, &freelistStmt,
&rollbackStmt, freelistCount,
aNeedsCheckpoint] {
// Warn in case of an error, but do not propagate it. Just indicate we
// didn't free any pages.
QM_TRY_INSPECT(const bool& res,
ReclaimFreePagesWhileIdle(freelistStmt, rollbackStmt,
freelistCount, aNeedsCheckpoint),
false);
const bool freedSomePages =
freelistCount && [this, &freelistStmt, &rollbackStmt, freelistCount,
aNeedsCheckpoint, &aInterrupted] {
// Warn in case of an error, but do not propagate it. Just indicate we
// didn't free any pages.
QM_TRY_INSPECT(
const bool& res,
ReclaimFreePagesWhileIdle(freelistStmt, rollbackStmt, freelistCount,
aNeedsCheckpoint, aInterrupted),
false);

// Make sure we didn't leave a transaction running.
MOZ_ASSERT(!mInReadTransaction);
MOZ_ASSERT(!mInWriteTransaction);
// Make sure we didn't leave a transaction running.
MOZ_ASSERT(!mInReadTransaction);
MOZ_ASSERT(!mInWriteTransaction);

return res;
}();
return res;
}();

// Truncate the WAL if we were asked to or if we managed to free some space.
if (aNeedsCheckpoint || freedSomePages) {
Expand All @@ -6947,7 +6954,8 @@ void DatabaseConnection::DoIdleProcessing(bool aNeedsCheckpoint) {

Result<bool, nsresult> DatabaseConnection::ReclaimFreePagesWhileIdle(
CachedStatement& aFreelistStatement, CachedStatement& aRollbackStatement,
uint32_t aFreelistCount, bool aNeedsCheckpoint) {
uint32_t aFreelistCount, bool aNeedsCheckpoint,
const Atomic<bool>& aInterrupted) {
AssertIsOnConnectionThread();
MOZ_ASSERT(aFreelistStatement);
MOZ_ASSERT(aRollbackStatement);
Expand All @@ -6967,7 +6975,7 @@ Result<bool, nsresult> DatabaseConnection::ReclaimFreePagesWhileIdle(
nsIThread* currentThread = NS_GetCurrentThread();
MOZ_ASSERT(currentThread);

if (NS_HasPendingEvents(currentThread)) {
if (aInterrupted) {
return false;
}

Expand Down Expand Up @@ -6999,7 +7007,7 @@ Result<bool, nsresult> DatabaseConnection::ReclaimFreePagesWhileIdle(

mInWriteTransaction = true;

bool freedSomePages = false, interrupted = false;
bool freedSomePages = false;

const auto rollback = [&aRollbackStatement, this](const auto&) {
MOZ_ASSERT(mInWriteTransaction);
Expand All @@ -7015,14 +7023,12 @@ Result<bool, nsresult> DatabaseConnection::ReclaimFreePagesWhileIdle(
uint64_t previousFreelistCount = (uint64_t)aFreelistCount + 1;

QM_TRY(CollectWhile(
[&aFreelistCount, &previousFreelistCount, &interrupted,
currentThread]() -> Result<bool, nsresult> {
if (NS_HasPendingEvents(currentThread)) {
// Abort if something else wants to use the thread, and
// roll back this transaction. It's ok if we never make
// progress here because the idle service should
// eventually reclaim this space.
interrupted = true;
[&aFreelistCount, &previousFreelistCount,
&aInterrupted]() -> Result<bool, nsresult> {
if (aInterrupted) {
// On interrupt, abort and roll back this transaction. It's ok
// if we never make progress here because the idle service
// should eventually reclaim this space.
return false;
}
// If we were not able to free anything, we might either see
Expand All @@ -7046,9 +7052,9 @@ Result<bool, nsresult> DatabaseConnection::ReclaimFreePagesWhileIdle(

return Ok{};
})
.andThen([&commitStmt, &freedSomePages, &interrupted, &rollback,
.andThen([&commitStmt, &freedSomePages, &aInterrupted, &rollback,
this](Ok) -> Result<Ok, nsresult> {
if (interrupted) {
if (aInterrupted) {
rollback(Ok{});
freedSomePages = false;
}
Expand Down Expand Up @@ -8091,20 +8097,13 @@ bool ConnectionPool::ScheduleTransaction(TransactionInfo& aTransactionInfo,
// deliberately const to prevent the attempt to wrongly optimize the
// refcounting by passing runnable.forget() to the Dispatch method, see
// bug 1598559.
const nsCOMPtr<nsIRunnable> runnable =
new Runnable("IndexedDBDummyRunnable");

for (uint32_t index = mDatabasesPerformingIdleMaintenance.Length();
index > 0; index--) {
const auto& performingIdleMaintenanceInfo =
mDatabasesPerformingIdleMaintenance[index - 1];

const auto dbInfo = performingIdleMaintenanceInfo.mDatabaseInfo;

dbInfo->mThreadInfo.AssertValid();

MOZ_ALWAYS_SUCCEEDS(dbInfo->mThreadInfo.ThreadRef().Dispatch(
runnable, NS_DISPATCH_NORMAL));
performingIdleMaintenanceInfo.mIdleConnectionRunnable->Interrupt();
}
}

Expand Down Expand Up @@ -8476,7 +8475,8 @@ ConnectionPool::IdleConnectionRunnable::Run() {
// The connection could be null if EnsureConnection() didn't run or was not
// successful in TransactionDatabaseOperationBase::RunOnConnectionThread().
if (mDatabaseInfo.mConnection) {
mDatabaseInfo.mConnection->DoIdleProcessing(mNeedsCheckpoint);
mDatabaseInfo.mConnection->DoIdleProcessing(mNeedsCheckpoint,
mInterrupted);
}

MOZ_ALWAYS_SUCCEEDS(owningThread->Dispatch(this, NS_DISPATCH_NORMAL));
Expand Down Expand Up @@ -16968,7 +16968,8 @@ TransactionBase::CommitOp::Run() {
connection->FinishWriteTransaction();

if (mTransaction->GetMode() == IDBTransaction::Mode::Cleanup) {
connection->DoIdleProcessing(/* aNeedsCheckpoint */ true);
connection->DoIdleProcessing(/* aNeedsCheckpoint */ true,
/* aInterrupted */ Atomic<bool>(false));

connection->EnableQuotaChecks();
}
Expand Down

0 comments on commit cccef63

Please sign in to comment.