From 80df1b7aa40ff488753ae5be7ef52ae73953845d Mon Sep 17 00:00:00 2001 From: Michal Novotny Date: Fri, 14 Dec 2012 18:03:01 +0100 Subject: [PATCH] Bug 808997 - Explicitly close all output and input streams of all active cache entries during shutdown, r=hurley --- netwerk/base/src/nsStreamListenerTee.cpp | 5 +- netwerk/cache/Makefile.in | 1 - netwerk/cache/nsCacheEntry.cpp | 40 +-- netwerk/cache/nsCacheEntry.h | 11 +- netwerk/cache/nsCacheEntryDescriptor.cpp | 265 ++++++++++++++++--- netwerk/cache/nsCacheEntryDescriptor.h | 80 ++++-- netwerk/cache/nsCacheService.cpp | 149 +++++++---- netwerk/cache/nsCacheService.h | 5 +- netwerk/cache/nsDiskCacheStreams.cpp | 39 +-- netwerk/cache/nsDiskCacheStreams.h | 3 +- netwerk/cache/nsIDiskCacheStreamInternal.idl | 15 -- netwerk/test/unit/test_doomentry.js | 2 +- toolkit/components/telemetry/Histograms.json | 40 ++- 13 files changed, 436 insertions(+), 219 deletions(-) delete mode 100644 netwerk/cache/nsIDiskCacheStreamInternal.idl diff --git a/netwerk/base/src/nsStreamListenerTee.cpp b/netwerk/base/src/nsStreamListenerTee.cpp index d18eb45ef19..51d7b47d16b 100644 --- a/netwerk/base/src/nsStreamListenerTee.cpp +++ b/netwerk/base/src/nsStreamListenerTee.cpp @@ -40,7 +40,10 @@ nsStreamListenerTee::OnStopRequest(nsIRequest *request, if (mEventTarget) { nsIOutputStream *sink = nullptr; mSink.swap(sink); - NS_ProxyRelease(mEventTarget, sink); + if (NS_FAILED(NS_ProxyRelease(mEventTarget, sink))) { + NS_WARNING("Releasing sink on the current thread!"); + NS_RELEASE(sink); + } } else { mSink = 0; diff --git a/netwerk/cache/Makefile.in b/netwerk/cache/Makefile.in index 09850c2d969..15713108ee5 100644 --- a/netwerk/cache/Makefile.in +++ b/netwerk/cache/Makefile.in @@ -26,7 +26,6 @@ XPIDLSRCS = \ nsICacheService.idl \ nsICacheSession.idl \ nsICacheVisitor.idl \ - nsIDiskCacheStreamInternal.idl \ $(NULL) EXPORTS = \ diff --git a/netwerk/cache/nsCacheEntry.cpp b/netwerk/cache/nsCacheEntry.cpp index 8073d8dbc57..3e1494c98eb 100644 --- a/netwerk/cache/nsCacheEntry.cpp +++ b/netwerk/cache/nsCacheEntry.cpp @@ -211,20 +211,15 @@ nsCacheEntry::RemoveRequest(nsCacheRequest * request) bool -nsCacheEntry::RemoveDescriptor(nsCacheEntryDescriptor * descriptor) +nsCacheEntry::RemoveDescriptor(nsCacheEntryDescriptor * descriptor, + bool * doomEntry) { NS_ASSERTION(descriptor->CacheEntry() == this, "### Wrong cache entry!!"); - nsresult rv = descriptor->CloseOutput(); - if (rv == NS_BASE_STREAM_WOULD_BLOCK) - return true; - descriptor->ClearCacheEntry(); + *doomEntry = descriptor->ClearCacheEntry(); + PR_REMOVE_AND_INIT_LINK(descriptor); - // Doom entry if something bad happens while closing. See bug #673543 - if (NS_FAILED(rv)) - nsCacheService::DoomEntry(this); - if (!PR_CLIST_IS_EMPTY(&mDescriptorQ)) return true; // stay active if we still have open descriptors @@ -236,7 +231,7 @@ nsCacheEntry::RemoveDescriptor(nsCacheEntryDescriptor * descriptor) void -nsCacheEntry::DetachDescriptors(void) +nsCacheEntry::DetachDescriptors() { nsCacheEntryDescriptor * descriptor = (nsCacheEntryDescriptor *)PR_LIST_HEAD(&mDescriptorQ); @@ -245,14 +240,6 @@ nsCacheEntry::DetachDescriptors(void) nsCacheEntryDescriptor * nextDescriptor = (nsCacheEntryDescriptor *)PR_NEXT_LINK(descriptor); - // Doom entry if something bad happens while closing. See bug #673543 - // Errors are handled different from RemoveDescriptor because this - // method is only called from ClearDoomList (in which case the entry is - // doomed anyway) and ClearActiveEntries (in which case we are shutting - // down and really want to get rid of the entry immediately) - if (NS_FAILED(descriptor->CloseOutput())) - nsCacheService::DoomEntry(this); - descriptor->ClearCacheEntry(); PR_REMOVE_AND_INIT_LINK(descriptor); descriptor = nextDescriptor; @@ -260,6 +247,23 @@ nsCacheEntry::DetachDescriptors(void) } +void +nsCacheEntry::GetDescriptors( + nsTArray > &outDescriptors) +{ + nsCacheEntryDescriptor * descriptor = + (nsCacheEntryDescriptor *)PR_LIST_HEAD(&mDescriptorQ); + + while (descriptor != &mDescriptorQ) { + nsCacheEntryDescriptor * nextDescriptor = + (nsCacheEntryDescriptor *)PR_NEXT_LINK(descriptor); + + outDescriptors.AppendElement(descriptor); + descriptor = nextDescriptor; + } +} + + /****************************************************************************** * nsCacheEntryInfo - for implementing about:cache *****************************************************************************/ diff --git a/netwerk/cache/nsCacheEntry.h b/netwerk/cache/nsCacheEntry.h index 852b42bf45b..a76f3322557 100644 --- a/netwerk/cache/nsCacheEntry.h +++ b/netwerk/cache/nsCacheEntry.h @@ -191,16 +191,17 @@ public: nsCacheAccessMode accessGranted, nsICacheEntryDescriptor ** result); - // nsresult Open(nsCacheRequest *request, nsICacheEntryDescriptor ** result); - // nsresult AsyncOpen(nsCacheRequest *request); bool RemoveRequest( nsCacheRequest * request); - bool RemoveDescriptor( nsCacheEntryDescriptor * descriptor); - + bool RemoveDescriptor( nsCacheEntryDescriptor * descriptor, + bool * doomEntry); + + void GetDescriptors(nsTArray > &outDescriptors); + private: friend class nsCacheEntryHashTable; friend class nsCacheService; - void DetachDescriptors(void); + void DetachDescriptors(); // internal methods void MarkDoomed() { mFlags |= eDoomedMask; } diff --git a/netwerk/cache/nsCacheEntryDescriptor.cpp b/netwerk/cache/nsCacheEntryDescriptor.cpp index 792d549ba08..cb2a8670fdf 100644 --- a/netwerk/cache/nsCacheEntryDescriptor.cpp +++ b/netwerk/cache/nsCacheEntryDescriptor.cpp @@ -77,10 +77,11 @@ nsCacheEntryDescriptor::nsCacheEntryDescriptor(nsCacheEntry * entry, nsCacheAccessMode accessGranted) : mCacheEntry(entry), mAccessGranted(accessGranted), - mOutput(nullptr), + mOutputWrapper(nullptr), mLock("nsCacheEntryDescriptor.mLock"), mAsyncDoomPending(false), - mDoomedOnClose(false) + mDoomedOnClose(false), + mClosingDescriptor(false) { PR_INIT_CLIST(this); NS_ADDREF(nsCacheService::GlobalInstance()); // ensure it lives for the lifetime of the descriptor @@ -97,6 +98,10 @@ nsCacheEntryDescriptor::~nsCacheEntryDescriptor() if (mCacheEntry) Close(); + NS_ASSERTION(mInputWrappers.Count() == 0, + "We have still some input wrapper!"); + NS_ASSERTION(!mOutputWrapper, "We have still an output wrapper!"); + nsCacheService * service = nsCacheService::GlobalInstance(); NS_RELEASE(service); } @@ -309,25 +314,31 @@ nsCacheEntryDescriptor::OpenInputStream(uint32_t offset, nsIInputStream ** resul { NS_ENSURE_ARG_POINTER(result); + nsInputStreamWrapper* cacheInput = nullptr; { nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHEENTRYDESCRIPTOR_OPENINPUTSTREAM)); if (!mCacheEntry) return NS_ERROR_NOT_AVAILABLE; if (!mCacheEntry->IsStreamData()) return NS_ERROR_CACHE_DATA_IS_NOT_STREAM; + // Don't open any new stream when closing descriptor or clearing entries + if (mClosingDescriptor || nsCacheService::GetClearingEntries()) + return NS_ERROR_NOT_AVAILABLE; + // ensure valid permissions if (!(mAccessGranted & nsICache::ACCESS_READ)) return NS_ERROR_CACHE_READ_ACCESS_DENIED; - } - nsInputStreamWrapper* cacheInput = nullptr; - const char *val; - val = mCacheEntry->GetMetaDataElement("uncompressed-len"); - if (val) { - cacheInput = new nsDecompressInputStreamWrapper(this, offset); - } else { - cacheInput = new nsInputStreamWrapper(this, offset); + const char *val; + val = mCacheEntry->GetMetaDataElement("uncompressed-len"); + if (val) { + cacheInput = new nsDecompressInputStreamWrapper(this, offset); + } else { + cacheInput = new nsInputStreamWrapper(this, offset); + } + if (!cacheInput) return NS_ERROR_OUT_OF_MEMORY; + + mInputWrappers.AppendElement(cacheInput); } - if (!cacheInput) return NS_ERROR_OUT_OF_MEMORY; NS_ADDREF(*result = cacheInput); return NS_OK; @@ -338,30 +349,36 @@ nsCacheEntryDescriptor::OpenOutputStream(uint32_t offset, nsIOutputStream ** res { NS_ENSURE_ARG_POINTER(result); + nsOutputStreamWrapper* cacheOutput = nullptr; { nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHEENTRYDESCRIPTOR_OPENOUTPUTSTREAM)); if (!mCacheEntry) return NS_ERROR_NOT_AVAILABLE; if (!mCacheEntry->IsStreamData()) return NS_ERROR_CACHE_DATA_IS_NOT_STREAM; + // Don't open any new stream when closing descriptor or clearing entries + if (mClosingDescriptor || nsCacheService::GetClearingEntries()) + return NS_ERROR_NOT_AVAILABLE; + // ensure valid permissions if (!(mAccessGranted & nsICache::ACCESS_WRITE)) return NS_ERROR_CACHE_WRITE_ACCESS_DENIED; - } - nsOutputStreamWrapper* cacheOutput = nullptr; - int32_t compressionLevel = nsCacheService::CacheCompressionLevel(); - const char *val; - val = mCacheEntry->GetMetaDataElement("uncompressed-len"); - if ((compressionLevel > 0) && val) { - cacheOutput = new nsCompressOutputStreamWrapper(this, offset); - } else { - // clear compression flag when compression disabled - see bug #715198 - if (val) { - mCacheEntry->SetMetaDataElement("uncompressed-len", nullptr); + int32_t compressionLevel = nsCacheService::CacheCompressionLevel(); + const char *val; + val = mCacheEntry->GetMetaDataElement("uncompressed-len"); + if ((compressionLevel > 0) && val) { + cacheOutput = new nsCompressOutputStreamWrapper(this, offset); + } else { + // clear compression flag when compression disabled - see bug 715198 + if (val) { + mCacheEntry->SetMetaDataElement("uncompressed-len", nullptr); + } + cacheOutput = new nsOutputStreamWrapper(this, offset); } - cacheOutput = new nsOutputStreamWrapper(this, offset); + if (!cacheOutput) return NS_ERROR_OUT_OF_MEMORY; + + mOutputWrapper = cacheOutput; } - if (!cacheOutput) return NS_ERROR_OUT_OF_MEMORY; NS_ADDREF(*result = cacheOutput); return NS_OK; @@ -537,8 +554,38 @@ nsCacheEntryDescriptor::MarkValid() NS_IMETHODIMP nsCacheEntryDescriptor::Close() { + nsRefPtr outputWrapper; + nsTArray > inputWrappers; + + { + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHEENTRYDESCRIPTOR_CLOSE)); + if (!mCacheEntry) return NS_ERROR_NOT_AVAILABLE; + + // Make sure no other stream can be opened + mClosingDescriptor = true; + outputWrapper = mOutputWrapper; + for (int32_t i = 0 ; i < mInputWrappers.Count() ; i++) + inputWrappers.AppendElement(static_cast( + mInputWrappers[i])); + } + + // Call Close() on the streams outside the lock since it might need to call + // methods that grab the cache service lock, e.g. compressed output stream + // when it finalizes the entry + if (outputWrapper) { + if (NS_FAILED(outputWrapper->Close())) { + NS_WARNING("Dooming entry because Close() failed!!!"); + Doom(); + } + outputWrapper = nullptr; + } + + for (uint32_t i = 0 ; i < inputWrappers.Length() ; i++) + inputWrappers[i]->Close(); + + inputWrappers.Clear(); + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHEENTRYDESCRIPTOR_CLOSE)); - if (!mCacheEntry) return NS_ERROR_NOT_AVAILABLE; // XXX perhaps closing descriptors should clear/sever transports @@ -613,6 +660,9 @@ nsInputStreamWrapper::LazyInit() { nsCacheServiceAutoLock lock(LOCK_TELEM(NSINPUTSTREAMWRAPPER_LAZYINIT)); + if (!mDescriptor) + return NS_ERROR_NOT_AVAILABLE; + nsCacheAccessMode mode; nsresult rv = mDescriptor->GetAccessGranted(&mode); if (NS_FAILED(rv)) return rv; @@ -636,18 +686,53 @@ nsInputStreamWrapper::LazyInit() return NS_OK; } +void nsCacheEntryDescriptor:: +nsInputStreamWrapper::CloseInternal() +{ + mLock.AssertCurrentThreadOwns(); + nsCacheServiceAutoLock lock(LOCK_TELEM(NSINPUTSTREAMWRAPPER_CLOSEINTERNAL)); + + if (mDescriptor) { + NS_ASSERTION(mDescriptor->mInputWrappers.IndexOf(this) != -1, + "Wrapper not found in array!"); + mDescriptor->mInputWrappers.RemoveElement(this); + nsCacheService::ReleaseObject_Locked(mDescriptor); + mDescriptor = nullptr; + } + mInitialized = false; + mInput = nullptr; +} + nsresult nsCacheEntryDescriptor:: nsInputStreamWrapper::Close() { - nsresult rv = EnsureInit(); - if (NS_FAILED(rv)) return rv; + mozilla::MutexAutoLock lock(mLock); - return mInput->Close(); + return Close_Locked(); +} + +nsresult nsCacheEntryDescriptor:: +nsInputStreamWrapper::Close_Locked() +{ + nsresult rv = EnsureInit(); + if (NS_SUCCEEDED(rv)) { + rv = mInput->Close(); + } else { + NS_ASSERTION(!mInput, + "Shouldn't have mInput when EnsureInit() failed"); + } + + // Call CloseInternal() even when EnsureInit() failed, e.g. in case we are + // closing streams with nsCacheService::CloseAllStream() + CloseInternal(); + return rv; } nsresult nsCacheEntryDescriptor:: nsInputStreamWrapper::Available(uint64_t *avail) { + mozilla::MutexAutoLock lock(mLock); + nsresult rv = EnsureInit(); if (NS_FAILED(rv)) return rv; @@ -656,6 +741,14 @@ nsInputStreamWrapper::Available(uint64_t *avail) nsresult nsCacheEntryDescriptor:: nsInputStreamWrapper::Read(char *buf, uint32_t count, uint32_t *countRead) +{ + mozilla::MutexAutoLock lock(mLock); + + return Read_Locked(buf, count, countRead); +} + +nsresult nsCacheEntryDescriptor:: +nsInputStreamWrapper::Read_Locked(char *buf, uint32_t count, uint32_t *countRead) { nsresult rv = EnsureInit(); if (NS_SUCCEEDED(rv)) @@ -697,6 +790,8 @@ nsDecompressInputStreamWrapper::Read(char * buf, uint32_t count, uint32_t *countRead) { + mozilla::MutexAutoLock lock(mLock); + int zerr = Z_OK; nsresult rv = NS_OK; @@ -737,9 +832,9 @@ nsDecompressInputStreamWrapper::Read(char * buf, mZstream.avail_out > 0 && count > 0) { if (mZstream.avail_in == 0) { - rv = nsInputStreamWrapper::Read((char*)mReadBuffer, - mReadBufferLen, - &mZstream.avail_in); + rv = nsInputStreamWrapper::Read_Locked((char*)mReadBuffer, + mReadBufferLen, + &mZstream.avail_in); if (NS_FAILED(rv) || !mZstream.avail_in) { break; } @@ -774,18 +869,29 @@ nsDecompressInputStreamWrapper::Read(char * buf, nsresult nsCacheEntryDescriptor:: nsDecompressInputStreamWrapper::Close() { + mozilla::MutexAutoLock lock(mLock); + + if (!mDescriptor) + return NS_ERROR_NOT_AVAILABLE; + EndZstream(); if (mReadBuffer) { nsMemory::Free(mReadBuffer); mReadBuffer = 0; mReadBufferLen = 0; } - return nsInputStreamWrapper::Close(); + return nsInputStreamWrapper::Close_Locked(); } nsresult nsCacheEntryDescriptor:: nsDecompressInputStreamWrapper::InitZstream() { + if (!mDescriptor) + return NS_ERROR_NOT_AVAILABLE; + + if (mStreamEnded) + return NS_ERROR_FAILURE; + // Initialize zlib inflate stream mZstream.zalloc = Z_NULL; mZstream.zfree = Z_NULL; @@ -806,6 +912,7 @@ nsDecompressInputStreamWrapper::EndZstream() { if (mStreamInitialized && !mStreamEnded) { inflateEnd(&mZstream); + mStreamInitialized = false; mStreamEnded = true; } return NS_OK; @@ -826,6 +933,9 @@ nsOutputStreamWrapper::LazyInit() { nsCacheServiceAutoLock lock(LOCK_TELEM(NSOUTPUTSTREAMWRAPPER_LAZYINIT)); + if (!mDescriptor) + return NS_ERROR_NOT_AVAILABLE; + nsCacheAccessMode mode; nsresult rv = mDescriptor->GetAccessGranted(&mode); if (NS_FAILED(rv)) return rv; @@ -857,12 +967,15 @@ nsOutputStreamWrapper::LazyInit() // If anything above failed, clean up internal state and get out of here // (see bug #654926)... if (NS_FAILED(rv)) { - mDescriptor->InternalCleanup(stream); + nsCacheService::ReleaseObject_Locked(stream.forget().get()); + mDescriptor->mOutputWrapper = nullptr; + nsCacheService::ReleaseObject_Locked(mDescriptor); + mDescriptor = nullptr; + mInitialized = false; return rv; } - // ... otherwise, set members and mark initialized - mDescriptor->mOutput = mOutput = stream; + mOutput = stream; mInitialized = true; return NS_OK; } @@ -874,18 +987,52 @@ nsOutputStreamWrapper::OnWrite(uint32_t count) return mDescriptor->RequestDataSizeChange((int32_t)count); } +void nsCacheEntryDescriptor:: +nsOutputStreamWrapper::CloseInternal() +{ + mLock.AssertCurrentThreadOwns(); + nsCacheServiceAutoLock lock(LOCK_TELEM(NSOUTPUTSTREAMWRAPPER_CLOSEINTERNAL)); + + if (mDescriptor) { + mDescriptor->mOutputWrapper = nullptr; + nsCacheService::ReleaseObject_Locked(mDescriptor); + mDescriptor = nullptr; + } + mInitialized = false; + mOutput = nullptr; +} + + NS_IMETHODIMP nsCacheEntryDescriptor:: nsOutputStreamWrapper::Close() { - nsresult rv = EnsureInit(); - if (NS_FAILED(rv)) return rv; + mozilla::MutexAutoLock lock(mLock); - return mOutput->Close(); + return Close_Locked(); +} + +nsresult nsCacheEntryDescriptor:: +nsOutputStreamWrapper::Close_Locked() +{ + nsresult rv = EnsureInit(); + if (NS_SUCCEEDED(rv)) { + rv = mOutput->Close(); + } else { + NS_ASSERTION(!mOutput, + "Shouldn't have mOutput when EnsureInit() failed"); + } + + // Call CloseInternal() even when EnsureInit() failed, e.g. in case we are + // closing streams with nsCacheService::CloseAllStream() + CloseInternal(); + return rv; } NS_IMETHODIMP nsCacheEntryDescriptor:: nsOutputStreamWrapper::Flush() { + mozilla::MutexAutoLock lock(mLock); + nsresult rv = EnsureInit(); if (NS_FAILED(rv)) return rv; @@ -896,6 +1043,15 @@ NS_IMETHODIMP nsCacheEntryDescriptor:: nsOutputStreamWrapper::Write(const char * buf, uint32_t count, uint32_t * result) +{ + mozilla::MutexAutoLock lock(mLock); + return Write_Locked(buf, count, result); +} + +nsresult nsCacheEntryDescriptor:: +nsOutputStreamWrapper::Write_Locked(const char * buf, + uint32_t count, + uint32_t * result) { nsresult rv = EnsureInit(); if (NS_FAILED(rv)) return rv; @@ -945,6 +1101,8 @@ nsCompressOutputStreamWrapper::Write(const char * buf, uint32_t count, uint32_t * result) { + mozilla::MutexAutoLock lock(mLock); + int zerr = Z_OK; nsresult rv = NS_OK; @@ -977,6 +1135,7 @@ nsCompressOutputStreamWrapper::Write(const char * buf, zerr = deflate(&mZstream, Z_NO_FLUSH); if (zerr == Z_STREAM_ERROR) { deflateEnd(&mZstream); + mStreamEnded = true; mStreamInitialized = false; return NS_ERROR_FAILURE; } @@ -988,6 +1147,7 @@ nsCompressOutputStreamWrapper::Write(const char * buf, rv = WriteBuffer(); if (NS_FAILED(rv)) { deflateEnd(&mZstream); + mStreamEnded = true; mStreamInitialized = false; return rv; } @@ -1001,7 +1161,13 @@ nsCompressOutputStreamWrapper::Write(const char * buf, NS_IMETHODIMP nsCacheEntryDescriptor:: nsCompressOutputStreamWrapper::Close() { - nsresult rv = NS_OK; + mozilla::MutexAutoLock lock(mLock); + + if (!mDescriptor) + return NS_ERROR_NOT_AVAILABLE; + + nsresult retval = NS_OK; + nsresult rv; int zerr = 0; if (mStreamInitialized) { @@ -1009,9 +1175,14 @@ nsCompressOutputStreamWrapper::Close() do { zerr = deflate(&mZstream, Z_FINISH); rv = WriteBuffer(); + if (NS_FAILED(rv)) + retval = rv; } while (zerr == Z_OK && rv == NS_OK); deflateEnd(&mZstream); + mStreamInitialized = false; } + // Do not allow to initialize stream after calling Close(). + mStreamEnded = true; if (mDescriptor->CacheEntry()) { nsAutoCString uncompressedLenStr; @@ -1027,6 +1198,8 @@ nsCompressOutputStreamWrapper::Close() uncompressedLenStr.AppendInt(mUncompressedCount); rv = mDescriptor->SetMetaDataElement("uncompressed-len", uncompressedLenStr.get()); + if (NS_FAILED(rv)) + retval = rv; } if (mWriteBuffer) { @@ -1035,12 +1208,22 @@ nsCompressOutputStreamWrapper::Close() mWriteBufferLen = 0; } - return nsOutputStreamWrapper::Close(); + rv = nsOutputStreamWrapper::Close_Locked(); + if (NS_FAILED(rv)) + retval = rv; + + return retval; } nsresult nsCacheEntryDescriptor:: nsCompressOutputStreamWrapper::InitZstream() { + if (!mDescriptor) + return NS_ERROR_NOT_AVAILABLE; + + if (mStreamEnded) + return NS_ERROR_FAILURE; + // Determine compression level: Aggressive compression // may impact performance on mobile devices, while a // lower compression level still provides substantial @@ -1068,7 +1251,7 @@ nsCompressOutputStreamWrapper::WriteBuffer() { uint32_t bytesToWrite = mWriteBufferLen - mZstream.avail_out; uint32_t result = 0; - nsresult rv = nsCacheEntryDescriptor::nsOutputStreamWrapper::Write( + nsresult rv = nsCacheEntryDescriptor::nsOutputStreamWrapper::Write_Locked( (const char *)mWriteBuffer, bytesToWrite, &result); mZstream.next_out = mWriteBuffer; mZstream.avail_out = mWriteBufferLen; diff --git a/netwerk/cache/nsCacheEntryDescriptor.h b/netwerk/cache/nsCacheEntryDescriptor.h index 0f301566f1f..9eb907e1082 100644 --- a/netwerk/cache/nsCacheEntryDescriptor.h +++ b/netwerk/cache/nsCacheEntryDescriptor.h @@ -13,9 +13,9 @@ #include "nsIInputStream.h" #include "nsIOutputStream.h" #include "nsCacheService.h" -#include "nsIDiskCacheStreamInternal.h" #include "zlib.h" #include "mozilla/Mutex.h" +#include "nsVoidArray.h" /****************************************************************************** * nsCacheEntryDescriptor @@ -30,6 +30,7 @@ public: NS_DECL_NSICACHEENTRYINFO friend class nsAsyncDoomEvent; + friend class nsCacheService; nsCacheEntryDescriptor(nsCacheEntry * entry, nsCacheAccessMode mode); virtual ~nsCacheEntryDescriptor(); @@ -43,8 +44,12 @@ public: * methods callbacks for nsCacheService */ nsCacheEntry * CacheEntry(void) { return mCacheEntry; } - void ClearCacheEntry(void) + bool ClearCacheEntry(void) { + NS_ASSERTION(mInputWrappers.Count() == 0, "Bad state"); + NS_ASSERTION(!mOutputWrapper, "Bad state"); + + bool doomEntry = false; bool asyncDoomPending; { mozilla::MutexAutoLock lock(mLock); @@ -52,33 +57,15 @@ public: } if (asyncDoomPending && mCacheEntry) { - nsCacheService::gService->DoomEntry_Internal(mCacheEntry, true); + doomEntry = true; mDoomedOnClose = true; } mCacheEntry = nullptr; - } - nsresult CloseOutput(void) - { - nsresult rv = InternalCleanup(mOutput); - mOutput = nullptr; - return rv; + return doomEntry; } private: - nsresult InternalCleanup(nsIOutputStream *stream) - { - if (stream) { - nsCOMPtr tmp (do_QueryInterface(stream)); - if (tmp) - return tmp->CloseInternal(); - else - return stream->Close(); - } - return NS_OK; - } - - /************************************************************************* * input stream wrapper class - * @@ -86,11 +73,14 @@ private: * doesn't need any references to the stream wrapper. *************************************************************************/ class nsInputStreamWrapper : public nsIInputStream { + friend class nsCacheEntryDescriptor; + private: nsCacheEntryDescriptor * mDescriptor; nsCOMPtr mInput; uint32_t mStartOffset; bool mInitialized; + mozilla::Mutex mLock; public: NS_DECL_ISUPPORTS NS_DECL_NSIINPUTSTREAM @@ -99,19 +89,33 @@ private: : mDescriptor(desc) , mStartOffset(off) , mInitialized(false) + , mLock("nsInputStreamWrapper.mLock") { NS_ADDREF(mDescriptor); } virtual ~nsInputStreamWrapper() { - NS_RELEASE(mDescriptor); + nsCOMPtr desc; + { + nsCacheServiceAutoLock lock(LOCK_TELEM( + NSINPUTSTREAMWRAPPER_DESTRUCTOR)); + desc.swap(mDescriptor); + if (desc) { + NS_ASSERTION(desc->mInputWrappers.IndexOf(this) != -1, + "Wrapper not found in array!"); + desc->mInputWrappers.RemoveElement(this); + } + } + } private: nsresult LazyInit(); nsresult EnsureInit() { return mInitialized ? NS_OK : LazyInit(); } + nsresult Read_Locked(char *buf, uint32_t count, uint32_t *countRead); + nsresult Close_Locked(); + void CloseInternal(); }; - friend class nsInputStreamWrapper; class nsDecompressInputStreamWrapper : public nsInputStreamWrapper { @@ -152,11 +156,14 @@ private: * doesn't need any references to the stream wrapper. *************************************************************************/ class nsOutputStreamWrapper : public nsIOutputStream { + friend class nsCacheEntryDescriptor; + protected: nsCacheEntryDescriptor * mDescriptor; nsCOMPtr mOutput; uint32_t mStartOffset; bool mInitialized; + mozilla::Mutex mLock; public: NS_DECL_ISUPPORTS NS_DECL_NSIOUTPUTSTREAM @@ -165,6 +172,7 @@ private: : mDescriptor(desc) , mStartOffset(off) , mInitialized(false) + , mLock("nsOutputStreamWrapper.mLock") { NS_ADDREF(mDescriptor); // owning ref } @@ -172,19 +180,29 @@ private: { // XXX _HACK_ the storage stream needs this! Close(); + nsCOMPtr desc; { - nsCacheServiceAutoLock lock(LOCK_TELEM(NSOUTPUTSTREAMWRAPPER_CLOSE)); - mDescriptor->mOutput = nullptr; + nsCacheServiceAutoLock lock(LOCK_TELEM( + NSOUTPUTSTREAMWRAPPER_DESTRUCTOR)); + desc.swap(mDescriptor); + if (desc) { + desc->mOutputWrapper = nullptr; + } + mOutput = nullptr; } - NS_RELEASE(mDescriptor); } private: nsresult LazyInit(); nsresult EnsureInit() { return mInitialized ? NS_OK : LazyInit(); } nsresult OnWrite(uint32_t count); + nsresult Write_Locked(const char * buf, + uint32_t count, + uint32_t * result); + nsresult Close_Locked(); + void CloseInternal(); }; - friend class nsOutputStreamWrapper; + class nsCompressOutputStreamWrapper : public nsOutputStreamWrapper { private: @@ -192,6 +210,7 @@ private: uint32_t mWriteBufferLen; z_stream mZstream; bool mStreamInitialized; + bool mStreamEnded; uint32_t mUncompressedCount; public: NS_DECL_ISUPPORTS @@ -202,6 +221,7 @@ private: , mWriteBuffer(0) , mWriteBufferLen(0) , mStreamInitialized(false) + , mStreamEnded(false) , mUncompressedCount(0) { } @@ -222,10 +242,12 @@ private: */ nsCacheEntry * mCacheEntry; // we are a child of the entry nsCacheAccessMode mAccessGranted; - nsIOutputStream * mOutput; + nsVoidArray mInputWrappers; + nsOutputStreamWrapper * mOutputWrapper; mozilla::Mutex mLock; bool mAsyncDoomPending; bool mDoomedOnClose; + bool mClosingDescriptor; }; diff --git a/netwerk/cache/nsCacheService.cpp b/netwerk/cache/nsCacheService.cpp index 5b1f915a9d8..a80236d0fb5 100644 --- a/netwerk/cache/nsCacheService.cpp +++ b/netwerk/cache/nsCacheService.cpp @@ -1197,7 +1197,7 @@ nsCacheService::ShutdownCustomCacheDeviceEnum(const nsAString& aProfileDir, void nsCacheService::Shutdown() { - // Thie method must be called on the main thread because mCacheIOThread must + // This method must be called on the main thread because mCacheIOThread must // only be modified on the main thread. if (!NS_IsMainThread()) { NS_RUNTIMEABORT("nsCacheService::Shutdown called off the main thread"); @@ -1210,17 +1210,26 @@ nsCacheService::Shutdown() nsCOMPtr parentDir; { - nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_SHUTDOWN)); - NS_ASSERTION(mInitialized, - "can't shutdown nsCacheService unless it has been initialized."); + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_SHUTDOWN)); + NS_ASSERTION(mInitialized, + "can't shutdown nsCacheService unless it has been initialized."); + if (!mInitialized) + return; - if (mInitialized) { + mClearingEntries = true; + DoomActiveEntries(nullptr); + } + + CloseAllStreams(); + + { + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_SHUTDOWN)); + NS_ASSERTION(mInitialized, "Bad state"); mInitialized = false; // Clear entries ClearDoomList(); - ClearActiveEntries(); if (mSmartSizeTimer) { mSmartSizeTimer->Cancel(); @@ -1261,9 +1270,9 @@ nsCacheService::Shutdown() LogCacheStatistics(); #endif + mClearingEntries = false; mCacheIOThread.swap(cacheIOThread); } - } // lock if (cacheIOThread) cacheIOThread->Shutdown(); @@ -2336,10 +2345,15 @@ nsCacheService::OnProfileShutdown(bool cleanse) // a reference to it. Ignore this call. return; } - nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_ONPROFILESHUTDOWN)); - gService->mClearingEntries = true; + { + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_ONPROFILESHUTDOWN)); + gService->mClearingEntries = true; + gService->DoomActiveEntries(nullptr); + } - gService->DoomActiveEntries(nullptr); + gService->CloseAllStreams(); + + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_ONPROFILESHUTDOWN)); gService->ClearDoomList(); // Make sure to wait for any pending cache-operations before @@ -2534,13 +2548,19 @@ void nsCacheService::CloseDescriptor(nsCacheEntryDescriptor * descriptor) { // ask entry to remove descriptor - nsCacheEntry * entry = descriptor->CacheEntry(); - bool stillActive = entry->RemoveDescriptor(descriptor); + nsCacheEntry * entry = descriptor->CacheEntry(); + bool doomEntry; + bool stillActive = entry->RemoveDescriptor(descriptor, &doomEntry); if (!entry->IsValid()) { gService->ProcessPendingRequests(entry); } + if (doomEntry) { + gService->DoomEntry_Internal(entry, true); + return; + } + if (!stillActive) { gService->DeactivateEntry(entry); } @@ -2844,22 +2864,6 @@ nsCacheService::ProcessPendingRequests(nsCacheEntry * entry) return NS_OK; } - -void -nsCacheService::ClearPendingRequests(nsCacheEntry * entry) -{ - nsCacheRequest * request = (nsCacheRequest *)PR_LIST_HEAD(&entry->mRequestQ); - - while (request != &entry->mRequestQ) { - nsCacheRequest * next = (nsCacheRequest *)PR_NEXT_LINK(request); - - // XXX we're just dropping these on the floor for now...definitely wrong. - PR_REMOVE_AND_INIT_LINK(request); - delete request; - request = next; - } -} - bool nsCacheService::IsDoomListEmpty() { @@ -2874,37 +2878,13 @@ nsCacheService::ClearDoomList() while (entry != &mDoomedEntries) { nsCacheEntry * next = (nsCacheEntry *)PR_NEXT_LINK(entry); - - entry->DetachDescriptors(); - DeactivateEntry(entry); - entry = next; - } -} - -void -nsCacheService::ClearActiveEntries() -{ - nsVoidArray entries; - - // We can't detach descriptors while enumerating hash table since calling - // entry->DetachDescriptors() could involve dooming the entry which tries - // to remove the entry from the hash table. - mActiveEntries.VisitEntries(GetActiveEntries, &entries); - - for (int32_t i = 0 ; i < entries.Count() ; i++) { - nsCacheEntry * entry = static_cast(entries.ElementAt(i)); - NS_ASSERTION(entry, "### active entry = nullptr!"); - // only called from Shutdown() so we don't worry about pending requests - gService->ClearPendingRequests(entry); entry->DetachDescriptors(); - gService->DeactivateEntry(entry); + DeactivateEntry(entry); + entry = next; } - - mActiveEntries.Shutdown(); } - PLDHashOperator nsCacheService::GetActiveEntries(PLDHashTable * table, PLDHashEntryHdr * hdr, @@ -2957,6 +2937,67 @@ nsCacheService::RemoveActiveEntry(PLDHashTable * table, } +void +nsCacheService::CloseAllStreams() +{ + nsTArray > inputs; + nsTArray > outputs; + + { + nsCacheServiceAutoLock lock(LOCK_TELEM(NSCACHESERVICE_CLOSEALLSTREAMS)); + + nsVoidArray entries; + +#if DEBUG + // make sure there is no active entry + mActiveEntries.VisitEntries(GetActiveEntries, &entries); + NS_ASSERTION(entries.Count() == 0, "Bad state"); +#endif + + // Get doomed entries + nsCacheEntry * entry = (nsCacheEntry *)PR_LIST_HEAD(&mDoomedEntries); + while (entry != &mDoomedEntries) { + nsCacheEntry * next = (nsCacheEntry *)PR_NEXT_LINK(entry); + entries.AppendElement(entry); + entry = next; + } + + // Iterate through all entries and collect input and output streams + for (int32_t i = 0 ; i < entries.Count() ; i++) { + entry = static_cast(entries.ElementAt(i)); + + nsTArray > descs; + entry->GetDescriptors(descs); + + for (uint32_t j = 0 ; j < descs.Length() ; j++) { + if (descs[j]->mOutputWrapper) + outputs.AppendElement(descs[j]->mOutputWrapper); + + for (int32_t k = 0 ; k < descs[j]->mInputWrappers.Count() ; k++) + inputs.AppendElement(static_cast< + nsCacheEntryDescriptor::nsInputStreamWrapper *>( + descs[j]->mInputWrappers[k])); + } + } + } + + uint32_t i; + for (i = 0 ; i < inputs.Length() ; i++) + inputs[i]->Close(); + + for (i = 0 ; i < outputs.Length() ; i++) + outputs[i]->Close(); +} + + +bool +nsCacheService::GetClearingEntries() +{ + AssertOwnsLock(); + return gService->mClearingEntries; +} + + #if defined(PR_LOGGING) void nsCacheService::LogCacheStatistics() diff --git a/netwerk/cache/nsCacheService.h b/netwerk/cache/nsCacheService.h index 6ba6d18c837..d05b62fe26d 100644 --- a/netwerk/cache/nsCacheService.h +++ b/netwerk/cache/nsCacheService.h @@ -121,6 +121,8 @@ public: static int32_t CacheCompressionLevel(); + static bool GetClearingEntries(); + /** * Methods called by any cache classes */ @@ -284,10 +286,9 @@ private: nsresult ProcessPendingRequests(nsCacheEntry * entry); - void ClearPendingRequests(nsCacheEntry * entry); void ClearDoomList(void); - void ClearActiveEntries(void); void DoomActiveEntries(DoomCheckFn check); + void CloseAllStreams(); static PLDHashOperator GetActiveEntries(PLDHashTable * table, diff --git a/netwerk/cache/nsDiskCacheStreams.cpp b/netwerk/cache/nsDiskCacheStreams.cpp index 55dd4270fe0..e8f70668a52 100644 --- a/netwerk/cache/nsDiskCacheStreams.cpp +++ b/netwerk/cache/nsDiskCacheStreams.cpp @@ -11,7 +11,6 @@ #include "nsDiskCacheStreams.h" #include "nsCacheService.h" #include "mozilla/FileUtils.h" -#include "nsIDiskCacheStreamInternal.h" #include "nsThreadUtils.h" #include "mozilla/Telemetry.h" #include "mozilla/TimeStamp.h" @@ -184,7 +183,6 @@ nsDiskCacheInputStream::IsNonBlocking(bool * nonBlocking) * nsDiskCacheOutputStream *****************************************************************************/ class nsDiskCacheOutputStream : public nsIOutputStream - , public nsIDiskCacheStreamInternal { public: nsDiskCacheOutputStream( nsDiskCacheStreamIO * parent); @@ -192,7 +190,6 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSIOUTPUTSTREAM - NS_DECL_NSIDISKCACHESTREAMINTERNAL void ReleaseStreamIO() { NS_IF_RELEASE(mStreamIO); } @@ -202,9 +199,8 @@ private: }; -NS_IMPL_THREADSAFE_ISUPPORTS2(nsDiskCacheOutputStream, - nsIOutputStream, - nsIDiskCacheStreamInternal) +NS_IMPL_THREADSAFE_ISUPPORTS1(nsDiskCacheOutputStream, + nsIOutputStream) nsDiskCacheOutputStream::nsDiskCacheOutputStream( nsDiskCacheStreamIO * parent) : mStreamIO(parent) @@ -244,29 +240,6 @@ nsDiskCacheOutputStream::Close() return rv; } -NS_IMETHODIMP -nsDiskCacheOutputStream::CloseInternal() -{ - nsresult rv = NS_OK; - mozilla::TimeStamp start = mozilla::TimeStamp::Now(); - - if (!mClosed) { - mClosed = true; - // tell parent streamIO we are closing - rv = mStreamIO->CloseOutputStreamInternal(this); - } - - mozilla::Telemetry::ID id; - if (NS_IsMainThread()) - id = mozilla::Telemetry::NETWORK_DISK_CACHE_OUTPUT_STREAM_CLOSE_INTERNAL_MAIN_THREAD; - else - id = mozilla::Telemetry::NETWORK_DISK_CACHE_OUTPUT_STREAM_CLOSE_INTERNAL; - - mozilla::Telemetry::AccumulateTimeDelta(id, start); - - return rv; -} - NS_IMETHODIMP nsDiskCacheOutputStream::Flush() { @@ -465,20 +438,14 @@ nsresult nsDiskCacheStreamIO::CloseOutputStream(nsDiskCacheOutputStream * outputStream) { nsCacheServiceAutoLock lock(LOCK_TELEM(NSDISKCACHESTREAMIO_CLOSEOUTPUTSTREAM)); // grab service lock - return CloseOutputStreamInternal(outputStream); -} -nsresult -nsDiskCacheStreamIO::CloseOutputStreamInternal( - nsDiskCacheOutputStream * outputStream) -{ nsresult rv; if (outputStream != mOutStream) { NS_WARNING("mismatched output streams"); return NS_ERROR_UNEXPECTED; } - + // output stream is closing if (!mBinding) { // if we're severed, just clear member variables NS_ASSERTION(!mBufDirty, "oops"); diff --git a/netwerk/cache/nsDiskCacheStreams.h b/netwerk/cache/nsDiskCacheStreams.h index 821134f5256..3edf79903f8 100644 --- a/netwerk/cache/nsDiskCacheStreams.h +++ b/netwerk/cache/nsDiskCacheStreams.h @@ -32,8 +32,7 @@ public: nsresult GetOutputStream(uint32_t offset, nsIOutputStream ** outputStream); nsresult CloseOutputStream(nsDiskCacheOutputStream * outputStream); - nsresult CloseOutputStreamInternal(nsDiskCacheOutputStream * outputStream); - + nsresult Write( const char * buffer, uint32_t count, uint32_t * bytesWritten); diff --git a/netwerk/cache/nsIDiskCacheStreamInternal.idl b/netwerk/cache/nsIDiskCacheStreamInternal.idl deleted file mode 100644 index 18caa2c505f..00000000000 --- a/netwerk/cache/nsIDiskCacheStreamInternal.idl +++ /dev/null @@ -1,15 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -#include "nsISupports.idl" - -[scriptable, uuid(61ff88f7-516e-4924-93af-42e7c412d18b)] -interface nsIDiskCacheStreamInternal : nsISupports -{ - /** - * We use this method internally to close nsDiskCacheOutputStream under - * the cache service lock. - */ - void closeInternal(); -}; diff --git a/netwerk/test/unit/test_doomentry.js b/netwerk/test/unit/test_doomentry.js index 67991fc778d..68668c91bb8 100644 --- a/netwerk/test/unit/test_doomentry.js +++ b/netwerk/test/unit/test_doomentry.js @@ -138,8 +138,8 @@ function check_doom3(status) // entry was doomed but writing should still succeed var data = "testdata"; write_and_check(gOstream, data, data.length); - gEntry.close(); gOstream.close(); + gEntry.close(); // dooming the same entry again should fail new DoomEntry("testentry", check_doom4); } diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index 79207505128..a66ba9d7be4 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -1000,11 +1000,17 @@ "n_buckets": 50, "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSOUTPUTSTREAMWRAPPER_LAZYINIT" }, - "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSOUTPUTSTREAMWRAPPER_CLOSE": { + "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSOUTPUTSTREAMWRAPPER_CLOSEINTERNAL": { "kind": "exponential", "high": "10 * 1000", "n_buckets": 50, - "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSOUTPUTSTREAMWRAPPER_CLOSE" + "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSOUTPUTSTREAMWRAPPER_CLOSEINTERNAL" + }, + "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSOUTPUTSTREAMWRAPPER_DESTRUCTOR": { + "kind": "exponential", + "high": "10 * 1000", + "n_buckets": 50, + "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSOUTPUTSTREAMWRAPPER_DESTRUCTOR" }, "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSINPUTSTREAMWRAPPER_LAZYINIT": { "kind": "exponential", @@ -1012,6 +1018,18 @@ "n_buckets": 50, "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSINPUTSTREAMWRAPPER_LAZYINIT" }, + "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSINPUTSTREAMWRAPPER_CLOSEINTERNAL": { + "kind": "exponential", + "high": "10 * 1000", + "n_buckets": 50, + "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSINPUTSTREAMWRAPPER_CLOSEINTERNAL" + }, + "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSINPUTSTREAMWRAPPER_DESTRUCTOR": { + "kind": "exponential", + "high": "10 * 1000", + "n_buckets": 50, + "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSINPUTSTREAMWRAPPER_DESTRUCTOR" + }, "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSEVICTDISKCACHEENTRIESEVENT_RUN": { "kind": "exponential", "high": "10 * 1000", @@ -1150,6 +1168,12 @@ "n_buckets": 50, "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSCACHESERVICE_DISKDEVICEHEAPSIZE" }, + "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSCACHESERVICE_CLOSEALLSTREAMS": { + "kind": "exponential", + "high": "10 * 1000", + "n_buckets": 50, + "description": "Time spent waiting on the cache service lock (ms) on the main thread in NSCACHESERVICE_CLOSEALLSTREAMS" + }, "CACHE_SERVICE_LOCK_WAIT_MAINTHREAD_NSCACHEENTRYDESCRIPTOR_DOOM": { "kind": "exponential", "high": "10 * 1000", @@ -1698,18 +1722,6 @@ "n_buckets": 10, "description": "Time spent in nsDiskCacheOutputStream::Close() on the main thread (ms)" }, - "NETWORK_DISK_CACHE_OUTPUT_STREAM_CLOSE_INTERNAL": { - "kind": "exponential", - "high": "10000", - "n_buckets": 10, - "description": "Time spent in nsDiskCacheOutputStream::CloseInternal() on non-main thread (ms)" - }, - "NETWORK_DISK_CACHE_OUTPUT_STREAM_CLOSE_INTERNAL_MAIN_THREAD": { - "kind": "exponential", - "high": "10000", - "n_buckets": 10, - "description": "Time spent in nsDiskCacheOutputStream::CloseInternal on the main thread (ms)" - }, "IDLE_NOTIFY_BACK_MS": { "kind": "exponential", "high": "5000",