From 6b4428ffb1849f76d8fb1ce17a2fb8eb481219ca Mon Sep 17 00:00:00 2001 From: Honza Bambas Date: Mon, 6 Jan 2014 21:27:18 +0100 Subject: [PATCH] Bug 949250 - Improve CacheFileMetadata write timer, remove non-thread safe nsWeakRef, r=michal --- netwerk/cache2/CacheFile.cpp | 290 +++----------------------- netwerk/cache2/CacheFile.h | 6 +- netwerk/cache2/CacheFileIOManager.cpp | 184 +++++++++++++++- netwerk/cache2/CacheFileIOManager.h | 20 +- netwerk/cache2/CacheFileMetadata.cpp | 23 +- 5 files changed, 253 insertions(+), 270 deletions(-) diff --git a/netwerk/cache2/CacheFile.cpp b/netwerk/cache2/CacheFile.cpp index 92d3c6bc32d9..febdafc5f3e4 100644 --- a/netwerk/cache2/CacheFile.cpp +++ b/netwerk/cache2/CacheFile.cpp @@ -8,7 +8,6 @@ #include "CacheFileChunk.h" #include "CacheFileInputStream.h" #include "CacheFileOutputStream.h" -#include "nsITimer.h" #include "nsThreadUtils.h" #include "mozilla/DebugOnly.h" #include @@ -18,8 +17,6 @@ namespace mozilla { namespace net { -#define kMetadataWriteDelay 5000 - class NotifyCacheFileListenerEvent : public nsRunnable { public: NotifyCacheFileListenerEvent(CacheFileListener *aCallback, @@ -93,129 +90,6 @@ protected: nsRefPtr mChunk; }; -class MetadataWriteTimer : public nsITimerCallback -{ -public: - NS_DECL_THREADSAFE_ISUPPORTS - NS_DECL_NSITIMERCALLBACK - - MetadataWriteTimer(CacheFile *aFile); - nsresult Fire(); - nsresult Cancel(); - bool ShouldFireNew(); - -protected: - virtual ~MetadataWriteTimer(); - - nsCOMPtr mFile; - nsCOMPtr mTimer; - nsCOMPtr mTarget; - PRIntervalTime mFireTime; -}; - -NS_IMPL_ADDREF(MetadataWriteTimer) -NS_IMPL_RELEASE(MetadataWriteTimer) - -NS_INTERFACE_MAP_BEGIN(MetadataWriteTimer) - NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsITimerCallback) - NS_INTERFACE_MAP_ENTRY(nsITimerCallback) -NS_INTERFACE_MAP_END_THREADSAFE - -MetadataWriteTimer::MetadataWriteTimer(CacheFile *aFile) - : mFireTime(0) -{ - LOG(("MetadataWriteTimer::MetadataWriteTimer() [this=%p, file=%p]", - this, aFile)); - MOZ_COUNT_CTOR(MetadataWriteTimer); - - mFile = do_GetWeakReference(static_cast(aFile)); - mTarget = NS_GetCurrentThread(); -} - -MetadataWriteTimer::~MetadataWriteTimer() -{ - LOG(("MetadataWriteTimer::~MetadataWriteTimer() [this=%p]", this)); - MOZ_COUNT_DTOR(MetadataWriteTimer); - - NS_ProxyRelease(mTarget, mTimer.forget().get()); - NS_ProxyRelease(mTarget, mFile.forget().get()); -} - -NS_IMETHODIMP -MetadataWriteTimer::Notify(nsITimer *aTimer) -{ - LOG(("MetadataWriteTimer::Notify() [this=%p, timer=%p]", this, aTimer)); - - MOZ_ASSERT(aTimer == mTimer); - - nsCOMPtr supp = do_QueryReferent(mFile); - if (!supp) - return NS_OK; - - CacheFile *file = static_cast( - static_cast(supp.get())); - - CacheFileAutoLock lock(file); - - if (file->mTimer != this) - return NS_OK; - - if (file->mMemoryOnly) - return NS_OK; - - file->WriteMetadataIfNeeded(); - file->mTimer = nullptr; - - return NS_OK; -} - -nsresult -MetadataWriteTimer::Fire() -{ - LOG(("MetadataWriteTimer::Fire() [this=%p]", this)); - - nsresult rv; - -#ifdef DEBUG - bool onCurrentThread = false; - rv = mTarget->IsOnCurrentThread(&onCurrentThread); - MOZ_ASSERT(NS_SUCCEEDED(rv) && onCurrentThread); -#endif - - mTimer = do_CreateInstance("@mozilla.org/timer;1", &rv); - NS_ENSURE_SUCCESS(rv, rv); - - rv = mTimer->InitWithCallback(this, kMetadataWriteDelay, - nsITimer::TYPE_ONE_SHOT); - NS_ENSURE_SUCCESS(rv, rv); - - mFireTime = PR_IntervalNow(); - - return NS_OK; -} - -nsresult -MetadataWriteTimer::Cancel() -{ - LOG(("MetadataWriteTimer::Cancel() [this=%p]", this)); - return mTimer->Cancel(); -} - -bool -MetadataWriteTimer::ShouldFireNew() -{ - uint32_t delta = PR_IntervalToMilliseconds(PR_IntervalNow() - mFireTime); - - if (delta > kMetadataWriteDelay / 2) { - LOG(("MetadataWriteTimer::ShouldFireNew() - returning true [this=%p]", - this)); - return true; - } - - LOG(("MetadataWriteTimer::ShouldFireNew() - returning false [this=%p]", - this)); - return false; -} class DoomFileHelper : public CacheFileIOListener { @@ -273,100 +147,12 @@ private: NS_IMPL_ISUPPORTS1(DoomFileHelper, CacheFileIOListener) -// We try to write metadata when the last reference to CacheFile is released. -// We call WriteMetadataIfNeeded() under the lock from CacheFile::Release() and -// if writing fails synchronously the listener is released and lock in -// CacheFile::Release() is re-entered. This helper class ensures that the -// listener is released outside the lock in case of synchronous failure. -class MetadataListenerHelper : public CacheFileMetadataListener -{ -public: - NS_DECL_THREADSAFE_ISUPPORTS - - MetadataListenerHelper(CacheFile *aFile) - : mFile(aFile) - { - MOZ_COUNT_CTOR(MetadataListenerHelper); - mListener = static_cast(aFile); - } - - NS_IMETHOD OnMetadataRead(nsresult aResult) - { - MOZ_CRASH("MetadataListenerHelper::OnMetadataRead should not be called!"); - return NS_ERROR_UNEXPECTED; - } - - NS_IMETHOD OnMetadataWritten(nsresult aResult) - { - mFile = nullptr; - return mListener->OnMetadataWritten(aResult); - } - -private: - virtual ~MetadataListenerHelper() - { - MOZ_COUNT_DTOR(MetadataListenerHelper); - if (mFile) { - mFile->ReleaseOutsideLock(mListener.forget().get()); - } - } - - CacheFile* mFile; - nsCOMPtr mListener; -}; - -NS_IMPL_ISUPPORTS1(MetadataListenerHelper, CacheFileMetadataListener) - - NS_IMPL_ADDREF(CacheFile) -NS_IMETHODIMP_(nsrefcnt) -CacheFile::Release() -{ - NS_PRECONDITION(0 != mRefCnt, "dup release"); - nsrefcnt count = --mRefCnt; - NS_LOG_RELEASE(this, count, "CacheFile"); - - MOZ_ASSERT(count != 0, "Unexpected"); - - if (count == 1) { - bool deleteFile = false; - - // Not using CacheFileAutoLock since it hard-refers the file - // and in it's destructor reenters this method. - Lock(); - - if (mMemoryOnly) { - deleteFile = true; - } - else if (mMetadata) { - WriteMetadataIfNeeded(); - if (mWritingMetadata) { - MOZ_ASSERT(mRefCnt > 1); - } else { - if (mRefCnt == 1) - deleteFile = true; - } - } - - Unlock(); - - if (!deleteFile) { - return count; - } - - NS_LOG_RELEASE(this, 0, "CacheFile"); - delete (this); - return 0; - } - - return count; -} - +NS_IMPL_RELEASE(CacheFile) NS_INTERFACE_MAP_BEGIN(CacheFile) NS_INTERFACE_MAP_ENTRY(mozilla::net::CacheFileChunkListener) NS_INTERFACE_MAP_ENTRY(mozilla::net::CacheFileIOListener) NS_INTERFACE_MAP_ENTRY(mozilla::net::CacheFileMetadataListener) - NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, mozilla::net::CacheFileChunkListener) NS_INTERFACE_MAP_END_THREADSAFE @@ -384,16 +170,15 @@ CacheFile::CacheFile() , mOutput(nullptr) { LOG(("CacheFile::CacheFile() [this=%p]", this)); - - NS_ADDREF(this); - MOZ_COUNT_CTOR(CacheFile); } CacheFile::~CacheFile() { LOG(("CacheFile::~CacheFile() [this=%p]", this)); - MOZ_COUNT_DTOR(CacheFile); + MutexAutoLock lock(mLock); + if (!mMemoryOnly) + WriteMetadataIfNeededLocked(true); } nsresult @@ -561,7 +346,7 @@ CacheFile::OnChunkWritten(nsresult aResult, CacheFileChunk *aChunk) aChunk->mFile.forget().get())); mCachedChunks.Put(aChunk->Index(), aChunk); mChunks.Remove(aChunk->Index()); - WriteMetadataIfNeeded(); + WriteMetadataIfNeededLocked(); return NS_OK; } @@ -784,7 +569,7 @@ CacheFile::OnMetadataWritten(nsresult aResult) return NS_OK; if (IsDirty()) - WriteMetadataIfNeeded(); + WriteMetadataIfNeededLocked(); if (!mWritingMetadata) { LOG(("CacheFile::OnMetadataWritten() - Releasing file handle [this=%p]", @@ -1355,7 +1140,7 @@ CacheFile::RemoveChunk(CacheFileChunk *aChunk) mCachedChunks.Put(chunk->Index(), chunk); mChunks.Remove(chunk->Index()); if (!mMemoryOnly) - WriteMetadataIfNeeded(); + WriteMetadataIfNeededLocked(); } return NS_OK; @@ -1375,7 +1160,7 @@ CacheFile::RemoveInput(CacheFileInputStream *aInput) ReleaseOutsideLock(static_cast(aInput)); if (!mMemoryOnly) - WriteMetadataIfNeeded(); + WriteMetadataIfNeededLocked(); return NS_OK; } @@ -1399,7 +1184,7 @@ CacheFile::RemoveOutput(CacheFileOutputStream *aOutput) NotifyListenersAboutOutputRemoval(); if (!mMemoryOnly) - WriteMetadataIfNeeded(); + WriteMetadataIfNeededLocked(); // Notify close listener as the last action aOutput->NotifyCloseListener(); @@ -1531,14 +1316,29 @@ CacheFile::WriteMetadataIfNeeded() { LOG(("CacheFile::WriteMetadataIfNeeded() [this=%p]", this)); + CacheFileAutoLock lock(this); + + if (!mMemoryOnly) + WriteMetadataIfNeededLocked(); +} + +void +CacheFile::WriteMetadataIfNeededLocked(bool aFireAndForget) +{ + // When aFireAndForget is set to true, we are called from dtor. + // |this| must not be referenced after this method returns! + + LOG(("CacheFile::WriteMetadataIfNeededLocked() [this=%p]", this)); + nsresult rv; AssertOwnsLock(); MOZ_ASSERT(!mMemoryOnly); - if (mTimer) { - mTimer->Cancel(); - mTimer = nullptr; + if (!aFireAndForget) { + // if aFireAndForget is set, we are called from dtor. Write + // scheduler hard-refers CacheFile otherwise, so we cannot be here. + CacheFileIOManager::UnscheduleMetadataWrite(this); } if (NS_FAILED(mStatus)) @@ -1548,18 +1348,15 @@ CacheFile::WriteMetadataIfNeeded() mWritingMetadata || mOpeningFile) return; - LOG(("CacheFile::WriteMetadataIfNeeded() - Writing metadata [this=%p]", + LOG(("CacheFile::WriteMetadataIfNeededLocked() - Writing metadata [this=%p]", this)); - nsRefPtr mlh = new MetadataListenerHelper(this); - - rv = mMetadata->WriteMetadata(mDataSize, mlh); + rv = mMetadata->WriteMetadata(mDataSize, aFireAndForget ? nullptr : this); if (NS_SUCCEEDED(rv)) { mWritingMetadata = true; mDataIsDirty = false; - } - else { - LOG(("CacheFile::WriteMetadataIfNeeded() - Writing synchronously failed " + } else { + LOG(("CacheFile::WriteMetadataIfNeededLocked() - Writing synchronously failed " "[this=%p]", this)); // TODO: close streams with error if (NS_SUCCEEDED(mStatus)) @@ -1572,30 +1369,7 @@ CacheFile::PostWriteTimer() { LOG(("CacheFile::PostWriteTimer() [this=%p]", this)); - nsresult rv; - - AssertOwnsLock(); - - if (mTimer) { - if (mTimer->ShouldFireNew()) { - LOG(("CacheFile::PostWriteTimer() - Canceling old timer [this=%p]", - this)); - mTimer->Cancel(); - mTimer = nullptr; - } - else { - LOG(("CacheFile::PostWriteTimer() - Keeping old timer [this=%p]", this)); - return; - } - } - - mTimer = new MetadataWriteTimer(this); - - rv = mTimer->Fire(); - if (NS_FAILED(rv)) { - LOG(("CacheFile::PostWriteTimer() - Firing timer failed with error 0x%08x " - "[this=%p]", rv, this)); - } + CacheFileIOManager::ScheduleMetadataWrite(this); } PLDHashOperator diff --git a/netwerk/cache2/CacheFile.h b/netwerk/cache2/CacheFile.h index c1853908c799..4445a8c4b1b1 100644 --- a/netwerk/cache2/CacheFile.h +++ b/netwerk/cache2/CacheFile.h @@ -6,7 +6,6 @@ #define CacheFile__h__ #include "CacheFileChunk.h" -#include "nsWeakReference.h" #include "CacheFileIOManager.h" #include "CacheFileMetadata.h" #include "nsRefPtrHashtable.h" @@ -47,7 +46,6 @@ NS_DEFINE_STATIC_IID_ACCESSOR(CacheFileListener, CACHEFILELISTENER_IID) class CacheFile : public CacheFileChunkListener , public CacheFileIOListener , public CacheFileMetadataListener - , public nsSupportsWeakReference { public: NS_DECL_THREADSAFE_ISUPPORTS @@ -101,12 +99,12 @@ public: void Key(nsACString& aKey) { aKey = mKey; } private: + friend class CacheFileIOManager; friend class CacheFileChunk; friend class CacheFileInputStream; friend class CacheFileOutputStream; friend class CacheFileAutoLock; friend class MetadataWriteTimer; - friend class MetadataListenerHelper; virtual ~CacheFile(); @@ -139,6 +137,7 @@ private: bool IsDirty(); void WriteMetadataIfNeeded(); + void WriteMetadataIfNeededLocked(bool aFireAndForget = false); void PostWriteTimer(); static PLDHashOperator WriteAllCachedChunks(const uint32_t& aIdx, @@ -171,7 +170,6 @@ private: nsRefPtr mHandle; nsRefPtr mMetadata; nsCOMPtr mListener; - nsRefPtr mTimer; nsCOMPtr mDoomAfterOpenListener; nsRefPtrHashtable mChunks; diff --git a/netwerk/cache2/CacheFileIOManager.cpp b/netwerk/cache2/CacheFileIOManager.cpp index 1eabfc4d173b..0ca14f21e98b 100644 --- a/netwerk/cache2/CacheFileIOManager.cpp +++ b/netwerk/cache2/CacheFileIOManager.cpp @@ -9,6 +9,7 @@ #include "CacheHashUtils.h" #include "CacheStorageService.h" #include "nsThreadUtils.h" +#include "CacheFile.h" #include "nsIFile.h" #include "mozilla/Telemetry.h" #include "mozilla/DebugOnly.h" @@ -36,6 +37,7 @@ namespace mozilla { namespace net { #define kOpenHandlesLimit 64 +#define kMetadataWriteDelay 5000 bool CacheFileHandle::DispatchRelease() @@ -737,9 +739,55 @@ protected: }; +class MetadataWriteScheduleEvent : public nsRunnable +{ +public: + enum EMode { + SCHEDULE, + UNSCHEDULE, + SHUTDOWN + } mMode; + + nsRefPtr mFile; + nsRefPtr mIOMan; + + MetadataWriteScheduleEvent(CacheFileIOManager * aManager, + CacheFile * aFile, + EMode aMode) + : mMode(aMode) + , mFile(aFile) + , mIOMan(aManager) + { } + + virtual ~MetadataWriteScheduleEvent() { } + + NS_IMETHOD Run() + { + nsRefPtr ioMan = CacheFileIOManager::gInstance; + if (!ioMan) { + NS_WARNING("CacheFileIOManager already gone in MetadataWriteScheduleEvent::Run()"); + return NS_OK; + } + + switch (mMode) + { + case SCHEDULE: + ioMan->ScheduleMetadataWriteInternal(mFile); + break; + case UNSCHEDULE: + ioMan->UnscheduleMetadataWriteInternal(mFile); + break; + case SHUTDOWN: + ioMan->ShutdownMetadataWriteSchedulingInternal(); + break; + } + return NS_OK; + } +}; + CacheFileIOManager * CacheFileIOManager::gInstance = nullptr; -NS_IMPL_ISUPPORTS0(CacheFileIOManager) +NS_IMPL_ISUPPORTS1(CacheFileIOManager, nsITimerCallback) CacheFileIOManager::CacheFileIOManager() : mShuttingDown(false) @@ -801,6 +849,8 @@ CacheFileIOManager::Shutdown() Telemetry::AutoTimer shutdownTimer; + ShutdownMetadataWriteScheduling(); + { mozilla::Mutex lock("CacheFileIOManager::Shutdown() lock"); mozilla::CondVar condVar(lock, "CacheFileIOManager::Shutdown() condVar"); @@ -816,12 +866,12 @@ CacheFileIOManager::Shutdown() MOZ_ASSERT(gInstance->mHandles.HandleCount() == 0); MOZ_ASSERT(gInstance->mHandlesByLastUsed.Length() == 0); + if (gInstance->mIOThread) + gInstance->mIOThread->Shutdown(); + nsRefPtr ioMan; ioMan.swap(gInstance); - if (ioMan->mIOThread) - ioMan->mIOThread->Shutdown(); - return NS_OK; } @@ -947,6 +997,132 @@ CacheFileIOManager::IsShutdown() return gInstance->mShuttingDown; } +// static +nsresult +CacheFileIOManager::ScheduleMetadataWrite(CacheFile * aFile) +{ + nsRefPtr ioMan = gInstance; + NS_ENSURE_TRUE(ioMan, NS_ERROR_NOT_INITIALIZED); + + NS_ENSURE_TRUE(!ioMan->mShuttingDown, NS_ERROR_NOT_INITIALIZED); + + nsRefPtr event = new MetadataWriteScheduleEvent( + ioMan, aFile, MetadataWriteScheduleEvent::SCHEDULE); + nsCOMPtr target = ioMan->IOTarget(); + NS_ENSURE_TRUE(target, NS_ERROR_UNEXPECTED); + return target->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL); +} + +nsresult +CacheFileIOManager::ScheduleMetadataWriteInternal(CacheFile * aFile) +{ + MOZ_ASSERT(IsOnIOThreadOrCeased()); + + nsresult rv; + + if (!mMetadataWritesTimer) { + mMetadataWritesTimer = do_CreateInstance("@mozilla.org/timer;1", &rv); + NS_ENSURE_SUCCESS(rv, rv); + + rv = mMetadataWritesTimer->InitWithCallback( + this, kMetadataWriteDelay, nsITimer::TYPE_ONE_SHOT); + NS_ENSURE_SUCCESS(rv, rv); + } + + if (mScheduledMetadataWrites.IndexOf(aFile) != + mScheduledMetadataWrites.NoIndex) { + return NS_OK; + } + + mScheduledMetadataWrites.AppendElement(aFile); + + return NS_OK; +} + +// static +nsresult +CacheFileIOManager::UnscheduleMetadataWrite(CacheFile * aFile) +{ + nsRefPtr ioMan = gInstance; + NS_ENSURE_TRUE(ioMan, NS_ERROR_NOT_INITIALIZED); + + NS_ENSURE_TRUE(!ioMan->mShuttingDown, NS_ERROR_NOT_INITIALIZED); + + nsRefPtr event = new MetadataWriteScheduleEvent( + ioMan, aFile, MetadataWriteScheduleEvent::UNSCHEDULE); + nsCOMPtr target = ioMan->IOTarget(); + NS_ENSURE_TRUE(target, NS_ERROR_UNEXPECTED); + return target->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL); +} + +nsresult +CacheFileIOManager::UnscheduleMetadataWriteInternal(CacheFile * aFile) +{ + MOZ_ASSERT(IsOnIOThreadOrCeased()); + + mScheduledMetadataWrites.RemoveElement(aFile); + + if (mScheduledMetadataWrites.Length() == 0 && + mMetadataWritesTimer) { + mMetadataWritesTimer->Cancel(); + mMetadataWritesTimer = nullptr; + } + + return NS_OK; +} + +// static +nsresult +CacheFileIOManager::ShutdownMetadataWriteScheduling() +{ + nsRefPtr ioMan = gInstance; + NS_ENSURE_TRUE(ioMan, NS_ERROR_NOT_INITIALIZED); + + nsRefPtr event = new MetadataWriteScheduleEvent( + ioMan, nullptr, MetadataWriteScheduleEvent::SHUTDOWN); + nsCOMPtr target = ioMan->IOTarget(); + NS_ENSURE_TRUE(target, NS_ERROR_UNEXPECTED); + return target->Dispatch(event, nsIEventTarget::DISPATCH_NORMAL); +} + +nsresult +CacheFileIOManager::ShutdownMetadataWriteSchedulingInternal() +{ + MOZ_ASSERT(IsOnIOThreadOrCeased()); + + nsTArray > files; + files.SwapElements(mScheduledMetadataWrites); + for (uint32_t i = 0; i < files.Length(); ++i) { + CacheFile * file = files[i]; + file->WriteMetadataIfNeeded(); + } + + if (mMetadataWritesTimer) { + mMetadataWritesTimer->Cancel(); + mMetadataWritesTimer = nullptr; + } + + return NS_OK; +} + +NS_IMETHODIMP +CacheFileIOManager::Notify(nsITimer * aTimer) +{ + MOZ_ASSERT(IsOnIOThreadOrCeased()); + MOZ_ASSERT(mMetadataWritesTimer == aTimer); + + mMetadataWritesTimer = nullptr; + + nsTArray > files; + files.SwapElements(mScheduledMetadataWrites); + for (uint32_t i = 0; i < files.Length(); ++i) { + CacheFile * file = files[i]; + file->WriteMetadataIfNeeded(); + } + + return NS_OK; +} + nsresult CacheFileIOManager::OpenFile(const nsACString &aKey, uint32_t aFlags, diff --git a/netwerk/cache2/CacheFileIOManager.h b/netwerk/cache2/CacheFileIOManager.h index ea4daf9548e0..7716e3ed68e9 100644 --- a/netwerk/cache2/CacheFileIOManager.h +++ b/netwerk/cache2/CacheFileIOManager.h @@ -8,6 +8,7 @@ #include "CacheIOThread.h" #include "CacheEntriesEnumerator.h" #include "nsIEventTarget.h" +#include "nsITimer.h" #include "nsCOMPtr.h" #include "mozilla/SHA1.h" #include "nsTArray.h" @@ -137,6 +138,7 @@ class OpenFileEvent; class CloseFileEvent; class ReadEvent; class WriteEvent; +class MetadataWriteScheduleEvent; #define CACHEFILEIOLISTENER_IID \ { /* dcaf2ddc-17cf-4242-bca1-8c86936375a5 */ \ @@ -163,10 +165,11 @@ public: NS_DEFINE_STATIC_IID_ACCESSOR(CacheFileIOListener, CACHEFILEIOLISTENER_IID) -class CacheFileIOManager : public nsISupports +class CacheFileIOManager : public nsITimerCallback { public: NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_NSITIMERCALLBACK enum { OPEN = 0U, @@ -186,6 +189,15 @@ public: static bool IsOnIOThreadOrCeased(); static bool IsShutdown(); + // Make aFile's WriteMetadataIfNeeded be called automatically after + // a short interval. + static nsresult ScheduleMetadataWrite(CacheFile * aFile); + // Remove aFile from the scheduling registry array. + // WriteMetadataIfNeeded will not be automatically called. + static nsresult UnscheduleMetadataWrite(CacheFile * aFile); + // Shuts the scheduling off and flushes all pending metadata writes. + static nsresult ShutdownMetadataWriteScheduling(); + static nsresult OpenFile(const nsACString &aKey, uint32_t aFlags, CacheFileIOListener *aCallback); @@ -225,6 +237,7 @@ private: friend class DoomFileByKeyEvent; friend class ReleaseNSPRHandleEvent; friend class TruncateSeekSetEOFEvent; + friend class MetadataWriteScheduleEvent; virtual ~CacheFileIOManager(); @@ -254,6 +267,9 @@ private: nsresult OpenNSPRHandle(CacheFileHandle *aHandle, bool aCreate = false); void NSPRHandleUsed(CacheFileHandle *aHandle); + nsresult ScheduleMetadataWriteInternal(CacheFile * aFile); + nsresult UnscheduleMetadataWriteInternal(CacheFile * aFile); + nsresult ShutdownMetadataWriteSchedulingInternal(); static CacheFileIOManager *gInstance; bool mShuttingDown; @@ -262,6 +278,8 @@ private: bool mTreeCreated; CacheFileHandles mHandles; nsTArray mHandlesByLastUsed; + nsTArray > mScheduledMetadataWrites; + nsCOMPtr mMetadataWritesTimer; }; } // net diff --git a/netwerk/cache2/CacheFileMetadata.cpp b/netwerk/cache2/CacheFileMetadata.cpp index 30f4060cfc4c..f6fbf551388d 100644 --- a/netwerk/cache2/CacheFileMetadata.cpp +++ b/netwerk/cache2/CacheFileMetadata.cpp @@ -245,14 +245,31 @@ CacheFileMetadata::WriteMetadata(uint32_t aOffset, *reinterpret_cast(p) = PR_htonl(aOffset); p += sizeof(uint32_t); - mListener = aListener; - rv = CacheFileIOManager::Write(mHandle, aOffset, mWriteBuf, p - mWriteBuf, - true, this); + char * writeBuffer; + if (aListener) { + mListener = aListener; + writeBuffer = mWriteBuf; + } else { + // We are not going to pass |this| as callback to CacheFileIOManager::Write + // so we must allocate a new buffer that will be released automatically when + // write is finished. This is actually better than to let + // CacheFileMetadata::OnDataWritten do the job, since when dispatching the + // result from some reason fails during shutdown, we would unnecessarily leak + // both this object and the buffer. + writeBuffer = static_cast(moz_xmalloc(p - mWriteBuf)); + memcpy(mWriteBuf, writeBuffer, p - mWriteBuf); + } + + rv = CacheFileIOManager::Write(mHandle, aOffset, writeBuffer, p - mWriteBuf, + true, aListener ? this : nullptr); if (NS_FAILED(rv)) { LOG(("CacheFileMetadata::WriteMetadata() - CacheFileIOManager::Write() " "failed synchronously. [this=%p, rv=0x%08x]", this, rv)); mListener = nullptr; + if (writeBuffer != mWriteBuf) { + free(writeBuffer); + } free(mWriteBuf); mWriteBuf = nullptr; NS_ENSURE_SUCCESS(rv, rv);