summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMoonchild <moonchild@palemoon.org>2022-03-28 12:32:04 +0000
committerMoonchild <moonchild@palemoon.org>2022-04-03 01:15:47 +0200
commit9c429abfb9fe38316901c44357648cb0f5512ebb (patch)
tree6dac44432527662a93ab13b675c36c5b896b2851
parent5a5bd92efa0616efe393833fca1ba2de1d0f3093 (diff)
downloaduxp-RB_29.4.5.1-UXP.tar.gz
Revert "[network] Improve thread-safety of cache entry handling."RB_29.4.5.1-UXP
This reverts commit 0d26601e1a668f95cf8e52d12cd6e13d51835e0f.
-rw-r--r--netwerk/cache2/CacheFile.cpp115
-rw-r--r--netwerk/cache2/CacheFile.h19
-rw-r--r--netwerk/cache2/CacheFileChunk.cpp2
-rw-r--r--netwerk/cache2/CacheFileInputStream.cpp2
-rw-r--r--netwerk/cache2/CacheFileOutputStream.cpp5
-rw-r--r--netwerk/cache2/CacheIOThread.h4
6 files changed, 67 insertions, 80 deletions
diff --git a/netwerk/cache2/CacheFile.cpp b/netwerk/cache2/CacheFile.cpp
index d265d898a8..69fc3d33c7 100644
--- a/netwerk/cache2/CacheFile.cpp
+++ b/netwerk/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/netwerk/cache2/CacheFile.h b/netwerk/cache2/CacheFile.h
index 74bae4b7a3..6d9d433b69 100644
--- a/netwerk/cache2/CacheFile.h
+++ b/netwerk/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/netwerk/cache2/CacheFileChunk.cpp b/netwerk/cache2/CacheFileChunk.cpp
index 54dc9fb32f..83d79f79fc 100644
--- a/netwerk/cache2/CacheFileChunk.cpp
+++ b/netwerk/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/netwerk/cache2/CacheFileInputStream.cpp b/netwerk/cache2/CacheFileInputStream.cpp
index 89348db094..26ca575370 100644
--- a/netwerk/cache2/CacheFileInputStream.cpp
+++ b/netwerk/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/netwerk/cache2/CacheFileOutputStream.cpp b/netwerk/cache2/CacheFileOutputStream.cpp
index 99e468141b..a3d414b8fa 100644
--- a/netwerk/cache2/CacheFileOutputStream.cpp
+++ b/netwerk/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/netwerk/cache2/CacheIOThread.h b/netwerk/cache2/CacheIOThread.h
index a3a5fa2bdb..ea71f7df0a 100644
--- a/netwerk/cache2/CacheIOThread.h
+++ b/netwerk/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;