Bug 2056841: Eliminate race on OriginInfo changes by holding mutex continuously. r=dom-storage-reviewers,hsingh

To avoid deadlocks, the DirtyTrackingAutoLock had to be unlocked for IO
thread operations. This allowed the weak reference it contained to
expire when the related in-memory OriginInfo was concurrently removed or
replaced, causing MaybeUpdateSize and GroupInfo failures.

Differential Revision: https://phabricator.services.mozilla.com/D314958

dom/quota/ActorsParent.cpp

@@ -6052,7 +6052,26 @@ void QuotaManager::FlushDirtyOriginInfos() {
   const uint32_t maxOriginsToSaveInOneBatch =
       StaticPrefs::dom_quotaManager_maxOriginsToSaveInOneBatch();
 
-  MutexAutoLock lock(mQuotaMutex);
+  // Use TryLock to avoid deadlocking with DirtyTrackingAutoLock, which holds
+  // mQuotaMutex while dispatching FlagOriginInfoAsDirtyOnDisk to the IO thread
+  // and blocking on a CondVar. If the IO thread tried to acquire mQuotaMutex
+  // while a DTAL holder was waiting for its IO dispatch, both would block.
+  //
+  // FlushDirtyOriginInfos is the only IO-thread mQuotaMutex user that can run
+  // concurrently with a DTAL holder (it fires on a timer). Other IO-thread
+  // mQuotaMutex users are safe because all DTAL call sites are normal quota
+  // operations that require temporary storage to be initialized:
+  //  - ClearOrigins, CleanupTemporaryStorage DEBUG checkers: only called
+  //    during initialization, before clients can use temporary storage.
+  //  - UnloadQuota, RemoveQuota: only called during shutdown, after all
+  //    client operations have completed.
+  //
+  // Dirty origins are retained in the queue and will be flushed on the next
+  // timer cycle.
+  MutexAutoTryLock lock(mQuotaMutex);
+  if (!lock) {
+    return;
+  }
 
   const auto now = TimeStamp::Now();
   uint32_t flushed = 0;
@@ -7681,7 +7700,7 @@ RefPtr<BoolPromise> QuotaManager::InitializeAllTemporaryOrigins() {
 nsresult QuotaManager::FlagOriginInfoAsDirtyOnDisk(
     DirtyTrackingAutoLock& aProofOfLock,
     const OriginStateMetadata& aStateMetadata) {
-  AssertNotCurrentThreadOwnsQuotaMutex();
+  AssertCurrentThreadOwnsQuotaMutex();
 
   struct SharedState {
     mozilla::Mutex mMutex;
@@ -8911,7 +8930,10 @@ void QuotaManager::CleanupTemporaryStorage() {
     // Verify that origins being cleared meet our criteria:
     // non-persisted, zero usage, and outside cutoff window
     auto checker = [&self = *this, cutoffTime](const auto& doomedOriginInfo) {
-      MutexAutoLock lock(self.mQuotaMutex);
+      MutexAutoTryLock lock(self.mQuotaMutex);
+      if (!lock) {
+        return;
+      }
       MOZ_ASSERT(!doomedOriginInfo->LockedPersisted());
       MOZ_ASSERT(doomedOriginInfo->LockedUsage() == 0);
       MOZ_ASSERT(doomedOriginInfo->LockedAccessTime() < cutoffTime);
@@ -8933,7 +8955,10 @@ void QuotaManager::CleanupTemporaryStorage() {
 
 #ifdef DEBUG
   auto nonPersistedChecker = [&self = *this](const auto& doomedOriginInfo) {
-    MutexAutoLock lock(self.mQuotaMutex);
+    MutexAutoTryLock lock(self.mQuotaMutex);
+    if (!lock) {
+      return;
+    }
     MOZ_ASSERT(!doomedOriginInfo->LockedPersisted());
   };
 #else

dom/quota/CanonicalQuotaObject.cpp

@@ -229,6 +229,15 @@ bool CanonicalQuotaObject::LockedMaybeUpdateSize(
   // We unlocked and relocked several times so we need to recompute all the
   // essential variables and recheck the group limit.
 
+  if (!originInfo->mGroupInfo) {
+    // During the pause, the origin got removed. We can skip updating the
+    // values of the in-memory representation because the origin is no
+    // longer in memory.
+    DirtyTrackingAutoLock::PauseLock pauseLock(aProofOfLock);
+    quotaManager->FinalizeOriginEviction(std::move(locks));
+    return false;
+  }
+
   QM_ASSERT_NO_UNDERFLOW(aSize, mSize);
   const uint64_t increase = aSize - mSize;
 

dom/quota/DirtyTrackingAutoLock.cpp

@@ -47,15 +47,10 @@ void DirtyTrackingAutoLock::EagerMarkAsDirty() {
 
   auto stateMetadata = mOriginInfo->LockedFlattenToOriginStateMetadata();
   stateMetadata.mDirty = true;
-  // The lock was acquired to set the dirty flag and timestamp above.
-  // Release it for the disk write that persists the flag, then re-acquire
-  // in RAII manner so the caller can proceed with the actual metadata
-  // modifications.
-  PauseLock pausedLock(*this);
 
   auto* quotaManager = QuotaManager::Get();
   MOZ_ASSERT(quotaManager);
-  quotaManager->AssertNotCurrentThreadOwnsQuotaMutex();
+  quotaManager->AssertCurrentThreadOwnsQuotaMutex();
 
   quotaManager->FlagOriginInfoAsDirtyOnDisk(*this, stateMetadata);
 }

dom/quota/DirtyTrackingAutoLock.h

@@ -25,13 +25,17 @@ class OriginInfo;
 // on destruction. In addition, on construction it eagerly writes the dirty
 // flag to the storage database so that a crash mid-operation leaves the
 // origin marked as needing a full metadata resync on next startup.
+//
+// The mutex is held throughout the entire lifetime of this object, including
+// during the eager disk write. FlushDirtyOriginInfos uses TryLock to avoid
+// deadlocking with this path.
 class MOZ_RAII MOZ_SCOPED_CAPABILITY MOZ_CAPABILITY("dirty_tracking_autolock")
     DirtyTrackingAutoLock {
  public:
   // RAII helper that temporarily releases the quota mutex held by a
-  // DirtyTrackingAutoLock, allowing file I/O to proceed without holding
-  // the lock. The mutex is re-acquired when the PauseLock is destroyed.
-  // Analogous to MutexAutoUnlock.
+  // DirtyTrackingAutoLock. Used by the eviction path in LockedMaybeUpdateSize
+  // to release the lock during blocking I/O. The mutex is re-acquired when
+  // the PauseLock is destroyed.
   class MOZ_RAII MOZ_SCOPED_CAPABILITY PauseLock {
    public:
     explicit PauseLock(DirtyTrackingAutoLock& aAutoLock)