Revert "[network] Improve thread-safety of cache entry handling."

This reverts commit 534e70bb48031ea0fc40294cabf44fac2055a63f.

8 files changed, 105 insertions, 141 deletions

diff --git a/system/network/cache2/CacheEntry.cpp b/system/network/cache2/CacheEntry.cpp
index d01626169..440dd542a 100644
--- a/system/network/cache2/CacheEntry.cpp
+++ b/system/network/cache2/CacheEntry.cpp
@@ -142,11 +142,8 @@ CacheEntry::Callback::~Callback()
   MOZ_COUNT_DTOR(CacheEntry::Callback);
 }
 
-// We have locks on both this and aEntry
 void CacheEntry::Callback::ExchangeEntry(CacheEntry* aEntry)
 {
-  aEntry->mLock.AssertCurrentThreadOwns();
-  mEntry->mLock.AssertCurrentThreadOwns();
   if (mEntry == aEntry)
     return;
 
@@ -211,11 +208,11 @@ CacheEntry::CacheEntry(const nsACString& aStorageID,
 , mStorageID(aStorageID)
 , mUseDisk(aUseDisk)
 , mSkipSizeCheck(aSkipSizeCheck)
-, mPinned(aPin)
 , mIsDoomed(false)
 , mSecurityInfoLoaded(false)
 , mPreventCallbacks(false)
 , mHasData(false)
+, mPinned(aPin)
 , mPinningKnown(false)
 , mState(NOTLOADED)
 , mRegistration(NEVERREGISTERED)
@@ -299,6 +296,9 @@ nsresult CacheEntry::HashingKey(nsCSubstring const& aStorageID,
 
 void CacheEntry::AsyncOpen(nsICacheEntryOpenCallback* aCallback, uint32_t aFlags)
 {
+  LOG(("CacheEntry::AsyncOpen [this=%p, state=%s, flags=%d, callback=%p]",
+    this, StateString(mState), aFlags, aCallback));
+
   bool readonly = aFlags & nsICacheStorage::OPEN_READONLY;
   bool bypassIfBusy = aFlags & nsICacheStorage::OPEN_BYPASS_IF_BUSY;
   bool truncate = aFlags & nsICacheStorage::OPEN_TRUNCATE;
@@ -306,20 +306,8 @@ void CacheEntry::AsyncOpen(nsICacheEntryOpenCallback* aCallback, uint32_t aFlags
   bool multithread = aFlags & nsICacheStorage::CHECK_MULTITHREADED;
   bool secret = aFlags & nsICacheStorage::OPEN_SECRETLY;
 
-  if (MOZ_LOG_TEST(gCache2Log, LogLevel::Debug)) {
-    MutexAutoLock lock(mLock);
-    LOG(("CacheEntry::AsyncOpen [this=%p, state=%s, flags=%d, callback=%p]",
-         this, StateString(mState), aFlags, aCallback));
-  }
-#ifdef DEBUG
-  {
-    // yes, if logging is on in DEBUG we'll take the lock twice in a row
-    MutexAutoLock lock(mLock);
-    MOZ_ASSERT(!readonly || !truncate, "Bad flags combination");
-    MOZ_ASSERT(!(truncate && mState > LOADING),
-               "Must not call truncate on already loaded entry");
-  }
-#endif
+  MOZ_ASSERT(!readonly || !truncate, "Bad flags combination");
+  MOZ_ASSERT(!(truncate && mState > LOADING), "Must not call truncate on already loaded entry");
 
   Callback callback(this, aCallback, readonly, multithread, secret);
 
@@ -474,7 +462,7 @@ NS_IMETHODIMP CacheEntry::OnFileReady(nsresult aResult, bool aIsNew)
 
   mPinned = mFile->IsPinned();;
   mPinningKnown = true;
-  LOG(("  pinning=%d", (bool)mPinned));
+  LOG(("  pinning=%d", mPinned));
 
   if (mState == READY) {
     mHasData = true;
@@ -564,7 +552,6 @@ already_AddRefed<CacheEntryHandle> CacheEntry::ReopenTruncated(bool aMemoryOnly,
 void CacheEntry::TransferCallbacks(CacheEntry & aFromEntry)
 {
   mozilla::MutexAutoLock lock(mLock);
-  aFromEntry.mLock.AssertCurrentThreadOwns();
 
   LOG(("CacheEntry::TransferCallbacks [entry=%p, from=%p]",
     this, &aFromEntry));
@@ -689,10 +676,11 @@ bool CacheEntry::InvokeCallbacks(bool aReadOnly)
 
 bool CacheEntry::InvokeCallback(Callback & aCallback)
 {
-  mLock.AssertCurrentThreadOwns();
   LOG(("CacheEntry::InvokeCallback [this=%p, state=%s, cb=%p]",
     this, StateString(mState), aCallback.mCallback.get()));
 
+  mLock.AssertCurrentThreadOwns();
+
   // When this entry is doomed we want to notify the callback any time
   if (!mIsDoomed) {
     // When we are here, the entry must be loaded from disk
@@ -806,21 +794,15 @@ bool CacheEntry::InvokeCallback(Callback & aCallback)
 
 void CacheEntry::InvokeAvailableCallback(Callback const & aCallback)
 {
+  LOG(("CacheEntry::InvokeAvailableCallback [this=%p, state=%s, cb=%p, r/o=%d, n/w=%d]",
+    this, StateString(mState), aCallback.mCallback.get(), aCallback.mReadOnly, aCallback.mNotWanted));
+
   nsresult rv;
-  uint32_t state;
-  { // Scope for lock
-    mozilla::MutexAutoLock lock(mLock);
-    state = mState;
-    LOG(
-        ("CacheEntry::InvokeAvailableCallback [this=%p, state=%s, cb=%p, "
-         "r/o=%d, "
-         "n/w=%d]",
-         this, StateString(mState), aCallback.mCallback.get(),
-         aCallback.mReadOnly, aCallback.mNotWanted));
 
-    // When we are here, the entry must be loaded from disk
-    MOZ_ASSERT(state > LOADING || mIsDoomed);
-  }
+  uint32_t const state = mState;
+
+  // When we are here, the entry must be loaded from disk
+  MOZ_ASSERT(state > LOADING || mIsDoomed);
 
   bool onAvailThread;
   rv = aCallback.OnAvailThread(&onAvailThread);
@@ -921,10 +903,11 @@ CacheEntryHandle* CacheEntry::NewWriteHandle()
 
 void CacheEntry::OnHandleClosed(CacheEntryHandle const* aHandle)
 {
-  mozilla::MutexAutoLock lock(mLock);
   LOG(("CacheEntry::OnHandleClosed [this=%p, state=%s, handle=%p]", this, StateString(mState), aHandle));
 
-  if (mIsDoomed && NS_SUCCEEDED(mFileStatus) &&
+  mozilla::MutexAutoLock lock(mLock);
+
+  if (IsDoomed() && NS_SUCCEEDED(mFileStatus) &&
       // Note: mHandlesCount is dropped before this method is called
       (mHandlesCount == 0 ||
        (mHandlesCount == 1 && mWriter && mWriter != aHandle))
@@ -1073,12 +1056,8 @@ NS_IMETHODIMP CacheEntry::GetIsForcedValid(bool *aIsForcedValid)
 {
   NS_ENSURE_ARG(aIsForcedValid);
 
-#ifdef DEBUG
-  {
-    mozilla::MutexAutoLock lock(mLock);
-    MOZ_ASSERT(mState > LOADING);
-  }
-#endif
+  MOZ_ASSERT(mState > LOADING);
+
   if (mPinned) {
     *aIsForcedValid = true;
     return NS_OK;
@@ -1442,11 +1421,12 @@ NS_IMETHODIMP CacheEntry::MetaDataReady()
 
 NS_IMETHODIMP CacheEntry::SetValid()
 {
+  LOG(("CacheEntry::SetValid [this=%p, state=%s]", this, StateString(mState)));
+
   nsCOMPtr<nsIOutputStream> outputStream;
 
-  { // Scope for lock
+  {
     mozilla::MutexAutoLock lock(mLock);
-    LOG(("CacheEntry::SetValid [this=%p, state=%s]", this, StateString(mState)));
 
     MOZ_ASSERT(mState > EMPTY);
 
@@ -1469,9 +1449,10 @@ NS_IMETHODIMP CacheEntry::SetValid()
 NS_IMETHODIMP CacheEntry::Recreate(bool aMemoryOnly,
                                    nsICacheEntry **_retval)
 {
-  mozilla::MutexAutoLock lock(mLock);
   LOG(("CacheEntry::Recreate [this=%p, state=%s]", this, StateString(mState)));
 
+  mozilla::MutexAutoLock lock(mLock);
+
   RefPtr<CacheEntryHandle> handle = ReopenTruncated(aMemoryOnly, nullptr);
   if (handle) {
     handle.forget(_retval);
@@ -1611,7 +1592,7 @@ bool CacheEntry::DeferOrBypassRemovalOnPinStatus(bool aPinned)
   mozilla::MutexAutoLock lock(mLock);
 
   if (mPinningKnown) {
-    LOG(("  pinned=%d, caller=%d", (bool)mPinned, aPinned));
+    LOG(("  pinned=%d, caller=%d", mPinned, aPinned));
     // Bypass when the pin status of this entry doesn't match the pin status
     // caller wants to remove
     return mPinned != aPinned;
@@ -1884,12 +1865,10 @@ NS_IMETHODIMP CacheOutputCloseListener::Run()
 
 // Memory reporting
 
-size_t CacheEntry::SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
+size_t CacheEntry::SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
   size_t n = 0;
 
-  MutexAutoLock lock(mLock);
-
   n += mCallbacks.ShallowSizeOfExcludingThis(mallocSizeOf);
   if (mFile) {
     n += mFile->SizeOfIncludingThis(mallocSizeOf);
@@ -1909,7 +1888,7 @@ size_t CacheEntry::SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf)
   return n;
 }
 
-size_t CacheEntry::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf)
+size_t CacheEntry::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
   return mallocSizeOf(this) + SizeOfExcludingThis(mallocSizeOf);
 }
diff --git a/system/network/cache2/CacheEntry.h b/system/network/cache2/CacheEntry.h
index 5988b74ea..4590fbbee 100644
--- a/system/network/cache2/CacheEntry.h
+++ b/system/network/cache2/CacheEntry.h
@@ -117,8 +117,8 @@ public:
   ::mozilla::Atomic<uint32_t, ::mozilla::Relaxed> mSortingExpirationTime;
 
   // Memory reporting
-  size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf);
-  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
+  size_t SizeOfExcludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
+  size_t SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const;
 
 private:
   virtual ~CacheEntry();
@@ -150,8 +150,7 @@ private:
     void ExchangeEntry(CacheEntry* aEntry);
 
     // Returns true when an entry is about to be "defer" doomed and this is
-    // a "defer" callback. The caller must hold a lock (this entry is in the
-    // caller's mCallback array)
+    // a "defer" callback.
     bool DeferDoom(bool *aDoom) const;
 
     // We are raising reference count here to take into account the pending
@@ -288,10 +287,9 @@ private:
   // When mFileStatus is read and found success it is ensured there is mFile and
   // that it is after a successful call to Init().
   ::mozilla::Atomic<nsresult, ::mozilla::ReleaseAcquire> mFileStatus;
-  // Set in constructor
-  nsCString const mURI;
-  nsCString const mEnhanceID;
-  nsCString const mStorageID;
+  nsCString mURI;
+  nsCString mEnhanceID;
+  nsCString mStorageID;
 
   // mUseDisk, mSkipSizeCheck, mIsDoomed are plain "bool", not "bool:1",
   // so as to avoid bitfield races with the byte containing
@@ -302,9 +300,7 @@ private:
   // Whether it should skip max size check.
   bool const mSkipSizeCheck;
   // Set when entry is doomed with AsyncDoom() or DoomAlreadyRemoved().
-  Atomic<bool, Relaxed> mIsDoomed;
-  // The indication of pinning this entry was open with
-  Atomic<bool, Relaxed> mPinned;
+  bool mIsDoomed;
 
   // Following flags are all synchronized with the cache entry lock.
 
@@ -319,6 +315,8 @@ private:
   // false: after load and a new file, or dropped to back to false when a writer
   //        fails to open an output stream.
   bool mHasData : 1;
+  // The indication of pinning this entry was open with
+  bool mPinned : 1;
   // Whether the pinning state of the entry is known (equals to the actual state
   // of the cache file)
   bool mPinningKnown : 1;
diff --git a/system/network/cache2/CacheFile.cpp b/system/network/cache2/CacheFile.cpp
index d265d898a..69fc3d33c 100644
--- a/system/network/cache2/CacheFile.cpp
+++ b/system/network/cache2/CacheFile.cpp
@@ -323,16 +323,6 @@ CacheFile::Init(const nsACString &aKey,
   return NS_OK;
 }
 
-void CacheFile::Key(nsACString& aKey) {
-  CacheFileAutoLock lock(this);
-  aKey = mKey;
-}
-
-bool CacheFile::IsPinned() {
-  CacheFileAutoLock lock(this);
-  return mPinned;
-}
-
 nsresult
 CacheFile::OnChunkRead(nsresult aResult, CacheFileChunk *aChunk)
 {
@@ -479,7 +469,6 @@ CacheFile::OnFileOpened(CacheFileHandle *aHandle, nsresult aResult)
     bool mAlreadyDoomed;
   } autoDoom(aHandle);
 
-  RefPtr<CacheFileMetadata> metadata;
   nsCOMPtr<CacheFileListener> listener;
   bool isNew = false;
   nsresult retval = NS_OK;
@@ -582,22 +571,20 @@ CacheFile::OnFileOpened(CacheFileHandle *aHandle, nsresult aResult)
         return NS_OK;
       }
     }
-    if (listener) {
-      lock.Unlock();
-      listener->OnFileReady(retval, isNew);
-      return NS_OK;
-    }
-
-    MOZ_ASSERT(NS_SUCCEEDED(aResult));
-    MOZ_ASSERT(!mMetadata);
-    MOZ_ASSERT(mListener);
+  }
 
-    metadata = mMetadata = new CacheFileMetadata(mHandle, mKey);
+  if (listener) {
+    listener->OnFileReady(retval, isNew);
+    return NS_OK;
   }
 
+  MOZ_ASSERT(NS_SUCCEEDED(aResult));
+  MOZ_ASSERT(!mMetadata);
+  MOZ_ASSERT(mListener);
+
   mMetadata = new CacheFileMetadata(mHandle, mKey);
 
-  rv = metadata->ReadMetadata(this);
+  rv = mMetadata->ReadMetadata(this);
   if (NS_FAILED(rv)) {
     mListener.swap(listener);
     listener->OnFileReady(rv, false);
@@ -624,42 +611,40 @@ CacheFile::OnDataRead(CacheFileHandle *aHandle, char *aBuf, nsresult aResult)
 nsresult
 CacheFile::OnMetadataRead(nsresult aResult)
 {
-  nsCOMPtr<CacheFileListener> listener;
-
-  bool isNew = false;
-  {
-    CacheFileAutoLock lock(this);
-    MOZ_ASSERT(mListener);
+  MOZ_ASSERT(mListener);
 
-    LOG(("CacheFile::OnMetadataRead() [this=%p, rv=0x%08]", this,
-         static_cast<uint32_t>(aResult)));
+  LOG(("CacheFile::OnMetadataRead() [this=%p, rv=0x%08x]", this, aResult));
 
-    if (NS_SUCCEEDED(aResult)) {
-      mPinned = mMetadata->Pinned();
-      mReady = true;
-      mDataSize = mMetadata->Offset();
-      if (mDataSize == 0 && mMetadata->ElementsSize() == 0) {
-        mMetadata->MarkDirty();
+  bool isNew = false;
+  if (NS_SUCCEEDED(aResult)) {
+    mPinned = mMetadata->Pinned();
+    mReady = true;
+    mDataSize = mMetadata->Offset();
+    if (mDataSize == 0 && mMetadata->ElementsSize() == 0) {
+      isNew = true;
+      mMetadata->MarkDirty();
+    } else {
+      const char *altData = mMetadata->GetElement(CacheFileUtils::kAltDataKey);
+      if (altData &&
+          (NS_FAILED(CacheFileUtils::ParseAlternativeDataInfo(
+            altData, &mAltDataOffset, nullptr)) ||
+          (mAltDataOffset > mDataSize))) {
+        // alt-metadata cannot be parsed or alt-data offset is invalid
+        mMetadata->InitEmptyMetadata();
+        isNew = true;
+        mAltDataOffset = -1;
+        mDataSize = 0;
       } else {
-        const char* altData =
-            mMetadata->GetElement(CacheFileUtils::kAltDataKey);
-        if (altData && (NS_FAILED(CacheFileUtils::ParseAlternativeDataInfo(
-                            altData, &mAltDataOffset, nullptr)) ||
-                        (mAltDataOffset > mDataSize))) {
-          // alt-metadata cannot be parsed or alt-data offset is invalid
-          mMetadata->InitEmptyMetadata();
-          isNew = true;
-          mAltDataOffset = -1;
-          mDataSize = 0;
-        } else {
-          PreloadChunks(0);
-        }
+        CacheFileAutoLock lock(this);
+        PreloadChunks(0);
       }
-      InitIndexEntry();
     }
-    mListener.swap(listener);
+
+    InitIndexEntry();
   }
 
+  nsCOMPtr<CacheFileListener> listener;
+  mListener.swap(listener);
   listener->OnFileReady(aResult, isNew);
   return NS_OK;
 }
@@ -1036,7 +1021,6 @@ CacheFile::Doom(CacheFileListener *aCallback)
 nsresult
 CacheFile::DoomLocked(CacheFileListener *aCallback)
 {
-  AssertOwnsLock();
   MOZ_ASSERT(mHandle || mMemoryOnly || mOpeningFile);
 
   LOG(("CacheFile::DoomLocked() [this=%p, listener=%p]", this, aCallback));
@@ -1248,7 +1232,6 @@ CacheFile::GetFetchCount(uint32_t *_retval)
 nsresult
 CacheFile::GetDiskStorageSizeInKB(uint32_t *aDiskStorageSize)
 {
-  CacheFileAutoLock lock(this);
   if (!mHandle) {
     return NS_ERROR_NOT_AVAILABLE;
   }
@@ -1273,6 +1256,30 @@ CacheFile::OnFetched()
 }
 
 void
+CacheFile::Lock()
+{
+  mLock.Lock();
+}
+
+void
+CacheFile::Unlock()
+{
+  // move the elements out of mObjsToRelease
+  // so that they can be released after we unlock
+  nsTArray<RefPtr<nsISupports>> objs;
+  objs.SwapElements(mObjsToRelease);
+
+  mLock.Unlock();
+
+}
+
+void
+CacheFile::AssertOwnsLock() const
+{
+  mLock.AssertCurrentThreadOwns();
+}
+
+void
 CacheFile::ReleaseOutsideLock(RefPtr<nsISupports> aObject)
 {
   AssertOwnsLock();
@@ -2003,7 +2010,6 @@ CacheFile::NotifyChunkListeners(uint32_t aIndex, nsresult aResult,
 bool
 CacheFile::HaveChunkListeners(uint32_t aIndex)
 {
-  AssertOwnsLock();
   ChunkListeners *listeners;
   mChunkListeners.Get(aIndex, &listeners);
   return !!listeners;
@@ -2264,7 +2270,6 @@ CacheFile::SetError(nsresult aStatus)
 nsresult
 CacheFile::InitIndexEntry()
 {
-  AssertOwnsLock();
   MOZ_ASSERT(mHandle);
 
   if (mHandle->IsDoomed())
diff --git a/system/network/cache2/CacheFile.h b/system/network/cache2/CacheFile.h
index 74bae4b7a..6d9d433b6 100644
--- a/system/network/cache2/CacheFile.h
+++ b/system/network/cache2/CacheFile.h
@@ -112,10 +112,9 @@ public:
   nsresult OnFetched();
 
   bool DataSize(int64_t* aSize);
-  void Key(nsACString& aKey);
+  void Key(nsACString& aKey) { aKey = mKey; }
   bool IsDoomed();
-  bool IsPinned();
-  // Returns true when there is a potentially unfinished write operation.
+  bool IsPinned() const { return mPinned; }
   bool IsWriteInProgress();
 
   // Memory reporting
@@ -132,16 +131,10 @@ private:
 
   virtual ~CacheFile();
 
-  void Lock() { mLock.Lock(); }
-  void Unlock() {
-    // move the elements out of mObjsToRelease
-    // so that they can be released after we unlock
-    nsTArray<RefPtr<nsISupports>> objs = std::move(mObjsToRelease);
-
-    mLock.Unlock();
-  }
-  void AssertOwnsLock() const { mLock.AssertCurrentThreadOwns(); }
-  void ReleaseOutsideLock(RefPtr<nsISupports> aObject);
+  void     Lock();
+  void     Unlock();
+  void     AssertOwnsLock() const;
+  void     ReleaseOutsideLock(RefPtr<nsISupports> aObject);
 
   enum ECallerType {
     READER    = 0,
diff --git a/system/network/cache2/CacheFileChunk.cpp b/system/network/cache2/CacheFileChunk.cpp
index 54dc9fb32..83d79f79f 100644
--- a/system/network/cache2/CacheFileChunk.cpp
+++ b/system/network/cache2/CacheFileChunk.cpp
@@ -472,7 +472,6 @@ void
 CacheFileChunk::WaitForUpdate(CacheFileChunkListener *aCallback)
 {
   AssertOwnsLock();
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   LOG(("CacheFileChunk::WaitForUpdate() [this=%p, listener=%p]",
        this, aCallback));
@@ -587,7 +586,6 @@ void
 CacheFileChunk::UpdateDataSize(uint32_t aOffset, uint32_t aLen)
 {
   AssertOwnsLock();
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   // UpdateDataSize() is called only when we've written some data to the chunk
   // and we never write data anymore once some error occurs.
diff --git a/system/network/cache2/CacheFileInputStream.cpp b/system/network/cache2/CacheFileInputStream.cpp
index 89348db09..26ca57537 100644
--- a/system/network/cache2/CacheFileInputStream.cpp
+++ b/system/network/cache2/CacheFileInputStream.cpp
@@ -348,7 +348,6 @@ NS_IMETHODIMP
 CacheFileInputStream::Seek(int32_t whence, int64_t offset)
 {
   CacheFileAutoLock lock(mFile);
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   LOG(("CacheFileInputStream::Seek() [this=%p, whence=%d, offset=%lld]",
        this, whence, offset));
@@ -396,7 +395,6 @@ NS_IMETHODIMP
 CacheFileInputStream::Tell(int64_t *_retval)
 {
   CacheFileAutoLock lock(mFile);
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   if (mClosed) {
     LOG(("CacheFileInputStream::Tell() - Stream is closed. [this=%p]", this));
diff --git a/system/network/cache2/CacheFileOutputStream.cpp b/system/network/cache2/CacheFileOutputStream.cpp
index 99e468141..a3d414b8f 100644
--- a/system/network/cache2/CacheFileOutputStream.cpp
+++ b/system/network/cache2/CacheFileOutputStream.cpp
@@ -90,7 +90,6 @@ CacheFileOutputStream::Write(const char * aBuf, uint32_t aCount,
                              uint32_t *_retval)
 {
   CacheFileAutoLock lock(mFile);
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   LOG(("CacheFileOutputStream::Write() [this=%p, count=%d]", this, aCount));
 
@@ -258,7 +257,6 @@ NS_IMETHODIMP
 CacheFileOutputStream::Seek(int32_t whence, int64_t offset)
 {
   CacheFileAutoLock lock(mFile);
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   LOG(("CacheFileOutputStream::Seek() [this=%p, whence=%d, offset=%lld]",
        this, whence, offset));
@@ -300,7 +298,6 @@ NS_IMETHODIMP
 CacheFileOutputStream::Tell(int64_t *_retval)
 {
   CacheFileAutoLock lock(mFile);
-  mFile->AssertOwnsLock();  // For thread-safety analysis
 
   if (mClosed) {
     LOG(("CacheFileOutputStream::Tell() - Stream is closed. [this=%p]", this));
@@ -374,8 +371,6 @@ void CacheFileOutputStream::NotifyCloseListener()
 void
 CacheFileOutputStream::ReleaseChunk()
 {
-  mFile->AssertOwnsLock();
-
   LOG(("CacheFileOutputStream::ReleaseChunk() [this=%p, idx=%d]",
        this, mChunk->Index()));
 
diff --git a/system/network/cache2/CacheIOThread.h b/system/network/cache2/CacheIOThread.h
index a3a5fa2bd..ea71f7df0 100644
--- a/system/network/cache2/CacheIOThread.h
+++ b/system/network/cache2/CacheIOThread.h
@@ -115,8 +115,6 @@ private:
 
   mozilla::Monitor mMonitor;
   PRThread* mThread;
-  // Only set in Init(), before the thread is started, which reads it but never
-  // writes
   UniquePtr<detail::BlockingIOWatcher> mBlockingIOWatcher;
   Atomic<nsIThread *> mXPCOMThread;
   Atomic<uint32_t, Relaxed> mLowestLevelWaiting;
@@ -130,7 +128,7 @@ private:
   // Raised when nsIEventTarget.Dispatch() is called on this thread
   Atomic<bool, Relaxed> mHasXPCOMEvents;
   // See YieldAndRerun() above
-  bool mRerunCurrentEvent;  // Only accessed on the cache thread
+  bool mRerunCurrentEvent;
   // Signal to process all pending events and then shutdown
   // Synchronized by mMonitor
   bool mShutdown;