diff --git a/dom/cache/Context.cpp b/dom/cache/Context.cpp index 503beefc9647..9b4d30061ca5 100644 --- a/dom/cache/Context.cpp +++ b/dom/cache/Context.cpp @@ -732,6 +732,7 @@ Context::Dispatch(nsIEventTarget* aTarget, Action* aAction) MOZ_ASSERT(aTarget); MOZ_ASSERT(aAction); + MOZ_ASSERT(mState != STATE_CONTEXT_CANCELED); if (mState == STATE_CONTEXT_CANCELED) { return; } else if (mState == STATE_CONTEXT_INIT) { @@ -766,11 +767,18 @@ Context::CancelAll() AllowToClose(); } +bool +Context::IsCanceled() const +{ + NS_ASSERT_OWNINGTHREAD(Context); + return mState == STATE_CONTEXT_CANCELED; +} + void Context::Invalidate() { NS_ASSERT_OWNINGTHREAD(Context); - mManager->Invalidate(); + mManager->NoteClosing(); CancelAll(); } diff --git a/dom/cache/Context.h b/dom/cache/Context.h index bbfba99d94f3..f2abf02e9538 100644 --- a/dom/cache/Context.h +++ b/dom/cache/Context.h @@ -126,6 +126,9 @@ public: // Only callable from the thread that created the Context. void CancelAll(); + // True if CancelAll() has been called. + bool IsCanceled() const; + // Like CancelAll(), but also marks the Manager as "invalid". void Invalidate(); diff --git a/dom/cache/Manager.cpp b/dom/cache/Manager.cpp index 6c434042e905..5dcc1c3d7cfb 100644 --- a/dom/cache/Manager.cpp +++ b/dom/cache/Manager.cpp @@ -157,6 +157,7 @@ public: if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } ref = new Manager(aManagerId, ioThread); + ref->Init(); MOZ_ASSERT(!sFactory->mManagerList.Contains(ref)); sFactory->mManagerList.AppendElement(ref); @@ -181,7 +182,7 @@ public: // If there is an invalid Manager finishing up and a new Manager // is created for the same origin, then the new Manager will // be blocked until QuotaManager finishes clearing the origin. - if (manager->IsValid() && *manager->mManagerId == *aManagerId) { + if (!manager->IsClosing() && *manager->mManagerId == *aManagerId) { return manager.forget(); } } @@ -1292,11 +1293,15 @@ public: if (!mManager->SetCacheIdOrphanedIfRefed(mCacheId)) { // no outstanding references, delete immediately - nsRefPtr context = mManager->CurrentContext(); - context->CancelForCacheId(mCacheId); - nsRefPtr action = - new DeleteOrphanedCacheAction(mManager, mCacheId); - context->Dispatch(mManager->mIOThread, action); + nsRefPtr context = mManager->mContext; + + // TODO: note that we need to check this cache for staleness on startup (bug 1110446) + if (!context->IsCanceled()) { + context->CancelForCacheId(mCacheId); + nsRefPtr action = + new DeleteOrphanedCacheAction(mManager, mCacheId); + context->Dispatch(mManager->mIOThread, action); + } } } @@ -1443,32 +1448,31 @@ Manager::RemoveContext(Context* aContext) MOZ_ASSERT(mContext == aContext); // Whether the Context destruction was triggered from the Manager going - // idle or the underlying storage being invalidated, we should be invalid - // when the context is destroyed. - MOZ_ASSERT(!mValid); + // idle or the underlying storage being invalidated, we should know we + // are closing before the Conext is destroyed. + MOZ_ASSERT(mClosing); mContext = nullptr; - // If we're trying to shutdown, then note that we're done. This is the - // delayed case from Manager::Shutdown(). - if (mShuttingDown) { - Factory::Remove(this); - } + // Once the context is gone, we can immediately remove ourself from the + // Factory list. We don't need to block shutdown by staying in the list + // any more. + Factory::Remove(this); } void -Manager::Invalidate() +Manager::NoteClosing() { NS_ASSERT_OWNINGTHREAD(Manager); - // QuotaManager can trigger this more than once. - mValid = false; + // This can be called more than once legitimately through different paths. + mClosing = true; } bool -Manager::IsValid() const +Manager::IsClosing() const { NS_ASSERT_OWNINGTHREAD(Manager); - return mValid; + return mClosing; } void @@ -1500,8 +1504,8 @@ Manager::ReleaseCacheId(CacheId aCacheId) bool orphaned = mCacheIdRefs[i].mOrphaned; mCacheIdRefs.RemoveElementAt(i); // TODO: note that we need to check this cache for staleness on startup (bug 1110446) - if (orphaned && !mShuttingDown && mValid) { - nsRefPtr context = CurrentContext(); + nsRefPtr context = mContext; + if (orphaned && context && !context->IsCanceled()) { context->CancelForCacheId(aCacheId); nsRefPtr action = new DeleteOrphanedCacheAction(this, aCacheId); @@ -1544,9 +1548,9 @@ Manager::ReleaseBodyId(const nsID& aBodyId) bool orphaned = mBodyIdRefs[i].mOrphaned; mBodyIdRefs.RemoveElementAt(i); // TODO: note that we need to check this body for staleness on startup (bug 1110446) - if (orphaned && !mShuttingDown && mValid) { + nsRefPtr context = mContext; + if (orphaned && context && !context->IsCanceled()) { nsRefPtr action = new DeleteOrphanedBodyAction(aBodyId); - nsRefPtr context = CurrentContext(); context->Dispatch(mIOThread, action); } } @@ -1589,12 +1593,14 @@ Manager::ExecuteCacheOp(Listener* aListener, CacheId aCacheId, MOZ_ASSERT(aOpArgs.type() != CacheOpArgs::TCacheAddAllArgs); MOZ_ASSERT(aOpArgs.type() != CacheOpArgs::TCachePutAllArgs); - if (mShuttingDown || !mValid) { + if (mClosing) { aListener->OnOpComplete(NS_ERROR_FAILURE, void_t()); return; } - nsRefPtr context = CurrentContext(); + nsRefPtr context = mContext; + MOZ_ASSERT(!context->IsCanceled()); + nsRefPtr streamList = new StreamList(this, context); ListenerId listenerId = SaveListener(aListener); @@ -1631,12 +1637,14 @@ Manager::ExecuteStorageOp(Listener* aListener, Namespace aNamespace, NS_ASSERT_OWNINGTHREAD(Manager); MOZ_ASSERT(aListener); - if (mShuttingDown || !mValid) { + if (mClosing) { aListener->OnOpComplete(NS_ERROR_FAILURE, void_t()); return; } - nsRefPtr context = CurrentContext(); + nsRefPtr context = mContext; + MOZ_ASSERT(!context->IsCanceled()); + nsRefPtr streamList = new StreamList(this, context); ListenerId listenerId = SaveListener(aListener); @@ -1678,12 +1686,14 @@ Manager::ExecutePutAll(Listener* aListener, CacheId aCacheId, NS_ASSERT_OWNINGTHREAD(Manager); MOZ_ASSERT(aListener); - if (mShuttingDown || !mValid) { + if (mClosing) { aListener->OnOpComplete(NS_ERROR_FAILURE, CachePutAllResult()); return; } - nsRefPtr context = CurrentContext(); + nsRefPtr context = mContext; + MOZ_ASSERT(!context->IsCanceled()); + ListenerId listenerId = SaveListener(aListener); nsRefPtr action = new CachePutAllAction(this, listenerId, aCacheId, @@ -1698,7 +1708,7 @@ Manager::Manager(ManagerId* aManagerId, nsIThread* aIOThread) , mIOThread(aIOThread) , mContext(nullptr) , mShuttingDown(false) - , mValid(true) + , mClosing(false) { MOZ_ASSERT(mManagerId); MOZ_ASSERT(mIOThread); @@ -1707,8 +1717,8 @@ Manager::Manager(ManagerId* aManagerId, nsIThread* aIOThread) Manager::~Manager() { NS_ASSERT_OWNINGTHREAD(Manager); + MOZ_ASSERT(mClosing); MOZ_ASSERT(!mContext); - Shutdown(); nsCOMPtr ioThread; mIOThread.swap(ioThread); @@ -1720,6 +1730,19 @@ Manager::~Manager() MOZ_ALWAYS_TRUE(NS_SUCCEEDED(NS_DispatchToMainThread(runnable))); } +void +Manager::Init() +{ + NS_ASSERT_OWNINGTHREAD(Manager); + + // Create the context immediately. Since there can at most be one Context + // per Manager now, this lets us cleanly call Factory::Remove() once the + // Context goes away. + nsRefPtr setupAction = new SetupAction(); + nsRefPtr ref = Context::Create(this, setupAction); + mContext = ref; +} + void Manager::Shutdown() { @@ -1731,37 +1754,20 @@ Manager::Shutdown() if (mShuttingDown) { return; } - - // Set a flag to prevent any new requests from coming in and creating - // a new Context. We must ensure all Contexts and IO operations are - // complete before shutdown proceeds. mShuttingDown = true; - // If there is a context, then we must wait for it to complete. Cancel and - // only note that we are done after its cleaned up. + // Note that we are closing to prevent any new requests from coming in and + // creating a new Context. We must ensure all Contexts and IO operations are + // complete before shutdown proceeds. + NoteClosing(); + + // If there is a context, then cancel and only note that we are done after + // its cleaned up. if (mContext) { nsRefPtr context = mContext; context->CancelAll(); return; } - - // Otherwise, note that we are complete immediately - Factory::Remove(this); -} - -already_AddRefed -Manager::CurrentContext() -{ - NS_ASSERT_OWNINGTHREAD(Manager); - nsRefPtr ref = mContext; - if (!ref) { - MOZ_ASSERT(!mShuttingDown); - MOZ_ASSERT(mValid); - nsRefPtr setupAction = new SetupAction(); - ref = Context::Create(this, setupAction); - mContext = ref; - } - return ref.forget(); } Manager::ListenerId @@ -1846,9 +1852,10 @@ Manager::NoteOrphanedBodyIdList(const nsTArray& aDeletedBodyIdList) } } - if (!deleteNowList.IsEmpty()) { + // TODO: note that we need to check these bodies for staleness on startup (bug 1110446) + nsRefPtr context = mContext; + if (!deleteNowList.IsEmpty() && context && !context->IsCanceled()) { nsRefPtr action = new DeleteOrphanedBodyAction(deleteNowList); - nsRefPtr context = CurrentContext(); context->Dispatch(mIOThread, action); } } @@ -1863,16 +1870,17 @@ Manager::MaybeAllowContextToClose() // Cache state information to complete before doing this. Once we allow // the Context to close we may not reliably get notified of storage // invalidation. - if (mContext && mListeners.IsEmpty() - && mCacheIdRefs.IsEmpty() - && mBodyIdRefs.IsEmpty()) { + nsRefPtr context = mContext; + if (context && mListeners.IsEmpty() + && mCacheIdRefs.IsEmpty() + && mBodyIdRefs.IsEmpty()) { // Mark this Manager as invalid so that it won't get used again. We don't // want to start any new operations once we allow the Context to close since // it may race with the underlying storage getting invalidated. - mValid = false; + NoteClosing(); - mContext->AllowToClose(); + context->AllowToClose(); } } diff --git a/dom/cache/Manager.h b/dom/cache/Manager.h index b1a7ac374774..b5a3ca85a2fd 100644 --- a/dom/cache/Manager.h +++ b/dom/cache/Manager.h @@ -133,8 +133,8 @@ public: // Marks the Manager "invalid". Once the Context completes no new operations // will be permitted with this Manager. New actors will get a new Manager. - void Invalidate(); - bool IsValid() const; + void NoteClosing(); + bool IsClosing() const; // If an actor represents a long term reference to a cache or body stream, // then they must call AddRefCacheId() or AddRefBodyId(). This will @@ -185,6 +185,7 @@ private: Manager(ManagerId* aManagerId, nsIThread* aIOThread); ~Manager(); + void Init(); void Shutdown(); already_AddRefed CurrentContext(); @@ -248,7 +249,7 @@ private: nsTArray mStreamLists; bool mShuttingDown; - bool mValid; + bool mClosing; struct CacheIdRefCounter {