Add more release assertions in the MemoryCache to help identify crash…

…es root cause https://bugs.webkit.org/show_bug.cgi?id=259199 Reviewed by Ryosuke Niwa. Add more release assertions in the MemoryCache to help identify crashes root cause. We're seeing quite a few crashes in MemoryCache code. * Source/WebCore/loader/cache/MemoryCache.cpp: (WebCore::MemoryCache::singleton): (WebCore::MemoryCache::sessionResourceMap const): (WebCore::MemoryCache::ensureSessionResourceMap): (WebCore::MemoryCache::add): (WebCore::MemoryCache::revalidationSucceeded): (WebCore::MemoryCache::revalidationFailed): (WebCore::MemoryCache::resourceForRequest): (WebCore::MemoryCache::pruneLiveResources): (WebCore::MemoryCache::forEachResource): (WebCore::MemoryCache::forEachSessionResource): (WebCore::MemoryCache::destroyDecodedDataForAllImages): (WebCore::MemoryCache::pruneLiveResourcesToSize): (WebCore::MemoryCache::pruneDeadResources): (WebCore::MemoryCache::pruneDeadResourcesToSize): (WebCore::MemoryCache::remove): (WebCore::MemoryCache::lruListFor): (WebCore::MemoryCache::removeFromLRUList): (WebCore::MemoryCache::insertInLRUList): (WebCore::MemoryCache::resourceAccessed): (WebCore::MemoryCache::inLiveDecodedResourcesList const): (WebCore::MemoryCache::removeResourcesWithOrigin): (WebCore::MemoryCache::removeResourcesWithOrigins): (WebCore::MemoryCache::getOriginsWithCache): (WebCore::MemoryCache::originsWithCache const): (WebCore::MemoryCache::removeFromLiveDecodedResourcesList): (WebCore::MemoryCache::insertInLiveDecodedResourcesList): (WebCore::MemoryCache::addToLiveResourcesSize): (WebCore::MemoryCache::removeFromLiveResourcesSize): (WebCore::MemoryCache::adjustSize): (WebCore::MemoryCache::setDisabled): (WebCore::MemoryCache::evictResources): (WebCore::MemoryCache::prune): (WebCore::MemoryCache::pruneSoon): Canonical link: https://commits.webkit.org/266050@main
WebKit · Jul 13, 2023 · 59c4d69 · 59c4d69
1 parent 00c3f53
commit 59c4d69
Showing 1 changed file with 48 additions and 7 deletions.
diff --git a/Source/WebCore/loader/cache/MemoryCache.cpp b/Source/WebCore/loader/cache/MemoryCache.cpp
@@ -55,7 +55,7 @@ static const float cTargetPrunePercentage = .95f; // Percentage of capacity towa
 
 MemoryCache& MemoryCache::singleton()
 {
-    ASSERT(isMainThread());
+    RELEASE_ASSERT(isMainThread());
     static NeverDestroyed<MemoryCache> memoryCache;
     return memoryCache;
 }
@@ -78,13 +78,17 @@ MemoryCache::MemoryCache()
 
 auto MemoryCache::sessionResourceMap(PAL::SessionID sessionID) const -> CachedResourceMap*
 {
-    ASSERT(sessionID.isValid());
+    RELEASE_ASSERT(sessionID.isValid());
+    RELEASE_ASSERT(isMainThread());
+    RELEASE_ASSERT(m_sessionResources.isValidKey(sessionID));
     return m_sessionResources.get(sessionID);
 }
 
 auto MemoryCache::ensureSessionResourceMap(PAL::SessionID sessionID) -> CachedResourceMap&
 {
-    ASSERT(sessionID.isValid());
+    RELEASE_ASSERT(sessionID.isValid());
+    RELEASE_ASSERT(isMainThread());
+    RELEASE_ASSERT(m_sessionResources.isValidKey(sessionID));
     auto& map = m_sessionResources.add(sessionID, nullptr).iterator->value;
     if (!map)
         map = makeUnique<CachedResourceMap>();
@@ -112,6 +116,8 @@ URL MemoryCache::removeFragmentIdentifierIfNeeded(const URL& originalURL)
 
 bool MemoryCache::add(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
+
     if (disabled())
         return false;
 
@@ -133,6 +139,7 @@ bool MemoryCache::add(CachedResource& resource)
 
 void MemoryCache::revalidationSucceeded(CachedResource& revalidatingResource, const ResourceResponse& response)
 {
+    RELEASE_ASSERT(isMainThread());
     ASSERT(response.source() == ResourceResponse::Source::MemoryCacheAfterValidation);
     ASSERT(revalidatingResource.resourceToRevalidate());
     CachedResource& resource = *revalidatingResource.resourceToRevalidate();
@@ -168,14 +175,15 @@ void MemoryCache::revalidationSucceeded(CachedResource& revalidatingResource, co
 
 void MemoryCache::revalidationFailed(CachedResource& revalidatingResource)
 {
-    ASSERT(isMainThread());
+    RELEASE_ASSERT(isMainThread());
     LOG(ResourceLoading, "Revalidation failed for %p", &revalidatingResource);
     ASSERT(revalidatingResource.resourceToRevalidate());
     revalidatingResource.clearResourceToRevalidate();
 }
 
 CachedResource* MemoryCache::resourceForRequest(const ResourceRequest& request, PAL::SessionID sessionID)
 {
+    RELEASE_ASSERT(isMainThread());
     // FIXME: Change all clients to make sure HTTP(s) URLs have no fragment identifiers before calling here.
     // CachedResourceLoader is now doing this. Add an assertion once all other clients are doing it too.
     auto* resources = sessionResourceMap(sessionID);
@@ -210,6 +218,7 @@ unsigned MemoryCache::liveCapacity() const
 
 void MemoryCache::pruneLiveResources(bool shouldDestroyDecodedDataForAllLiveResources)
 {
+    RELEASE_ASSERT(isMainThread());
     unsigned capacity = shouldDestroyDecodedDataForAllLiveResources ? 0 : liveCapacity();
     if (capacity && m_liveSize <= capacity)
         return;
@@ -221,6 +230,7 @@ void MemoryCache::pruneLiveResources(bool shouldDestroyDecodedDataForAllLiveReso
 
 void MemoryCache::forEachResource(const Function<void(CachedResource&)>& function)
 {
+    RELEASE_ASSERT(isMainThread());
     Vector<WeakPtr<CachedResource>> allResources;
     for (auto& lruList : m_allResources) {
         allResources.reserveCapacity(allResources.size() + lruList->computeSize());
@@ -235,6 +245,8 @@ void MemoryCache::forEachResource(const Function<void(CachedResource&)>& functio
 
 void MemoryCache::forEachSessionResource(PAL::SessionID sessionID, const Function<void(CachedResource&)>& function)
 {
+    RELEASE_ASSERT(isMainThread());
+    RELEASE_ASSERT(m_sessionResources.isValidKey(sessionID));
     auto it = m_sessionResources.find(sessionID);
     if (it == m_sessionResources.end())
         return;
@@ -245,6 +257,7 @@ void MemoryCache::forEachSessionResource(PAL::SessionID sessionID, const Functio
 
 void MemoryCache::destroyDecodedDataForAllImages()
 {
+    RELEASE_ASSERT(isMainThread());
     forEachResource([](CachedResource& resource) {
         if (auto cachedImage = dynamicDowncast<CachedImage>(resource)) {
             if (RefPtr image = cachedImage->image())
@@ -255,6 +268,7 @@ void MemoryCache::destroyDecodedDataForAllImages()
 
 void MemoryCache::pruneLiveResourcesToSize(unsigned targetSize, bool shouldDestroyDecodedDataForAllLiveResources)
 {
+    RELEASE_ASSERT(isMainThread());
     if (m_inPruneResources)
         return;
 
@@ -310,6 +324,7 @@ void MemoryCache::pruneLiveResourcesToSize(unsigned targetSize, bool shouldDestr
 void MemoryCache::pruneDeadResources()
 {
     LOG(ResourceLoading, "MemoryCache::pruneDeadResources");
+    RELEASE_ASSERT(isMainThread());
 
     unsigned capacity = deadCapacity();
     if (capacity && m_deadSize <= capacity)
@@ -321,6 +336,7 @@ void MemoryCache::pruneDeadResources()
 
 void MemoryCache::pruneDeadResourcesToSize(unsigned targetSize)
 {
+    RELEASE_ASSERT(isMainThread());
     if (m_inPruneResources)
         return;
 
@@ -403,7 +419,7 @@ void MemoryCache::setCapacities(unsigned minDeadBytes, unsigned maxDeadBytes, un
 
 void MemoryCache::remove(CachedResource& resource)
 {
-    ASSERT(isMainThread());
+    RELEASE_ASSERT(isMainThread());
     LOG(ResourceLoading, "Evicting resource %p for '%.255s' from cache", &resource, resource.url().string().latin1().data());
     // The resource may have already been removed by someone other than our caller,
     // who needed a fresh copy for a reload. See <http://bugs.webkit.org/show_bug.cgi?id=12479#c6>.
@@ -418,25 +434,29 @@ void MemoryCache::remove(CachedResource& resource)
             resource.setInCache(false);
 
             // If the resource map is now empty, remove it from m_sessionResources.
-            if (resources->isEmpty())
+            if (resources->isEmpty()) {
+                RELEASE_ASSERT(m_sessionResources.isValidKey(resource.sessionID()));
                 m_sessionResources.remove(resource.sessionID());
+            }
 
             // Remove from the appropriate LRU list.
             removeFromLRUList(resource);
             removeFromLiveDecodedResourcesList(resource);
             adjustSize(resource.hasClients(), -static_cast<long long>(resource.size()));
         } else {
-            ASSERT(resources->get(key) != &resource);
+            RELEASE_ASSERT(resources->get(key) != &resource);
             LOG(ResourceLoading, "  resource %p is not in cache", &resource);
         }
     }
+    RELEASE_ASSERT(!resource.inCache());
 
     if (resource.canDelete())
         resource.deleteThis();
 }
 
 auto MemoryCache::lruListFor(CachedResource& resource) -> LRUList&
 {
+    RELEASE_ASSERT(isMainThread());
     unsigned accessCount = std::max(resource.accessCount(), 1U);
     unsigned queueIndex = WTF::fastLog2(resource.size() / accessCount);
 #if ASSERT_ENABLED
@@ -451,6 +471,7 @@ auto MemoryCache::lruListFor(CachedResource& resource) -> LRUList&
 
 void MemoryCache::removeFromLRUList(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     // If we've never been accessed, then we're brand new and not in any list.
     if (!resource.accessCount())
         return;
@@ -470,6 +491,7 @@ void MemoryCache::removeFromLRUList(CachedResource& resource)
 
 void MemoryCache::insertInLRUList(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     ASSERT(resource.inCache());
     ASSERT(resource.accessCount() > 0);
 
@@ -479,6 +501,7 @@ void MemoryCache::insertInLRUList(CachedResource& resource)
 
 void MemoryCache::resourceAccessed(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     ASSERT(resource.inCache());
 
     // Need to make sure to remove before we increase the access count, since
@@ -498,11 +521,13 @@ void MemoryCache::resourceAccessed(CachedResource& resource)
 
 bool MemoryCache::inLiveDecodedResourcesList(CachedResource& resource) const
 {
+    RELEASE_ASSERT(isMainThread());
     return m_liveDecodedResources.contains(resource);
 }
 
 void MemoryCache::removeResourcesWithOrigin(const SecurityOrigin& origin, const String& cachePartition)
 {
+    RELEASE_ASSERT(isMainThread());
     Vector<CachedResource*> resourcesWithOrigin;
     for (auto& resources : m_sessionResources.values()) {
         for (auto& keyValue : *resources) {
@@ -524,18 +549,21 @@ void MemoryCache::removeResourcesWithOrigin(const SecurityOrigin& origin, const
 
 void MemoryCache::removeResourcesWithOrigin(const SecurityOrigin& origin)
 {
+    RELEASE_ASSERT(isMainThread());
     String originPartition = ResourceRequest::partitionName(origin.host());
     removeResourcesWithOrigin(origin, originPartition);
 }
 
 void MemoryCache::removeResourcesWithOrigin(const ClientOrigin& origin)
 {
+    RELEASE_ASSERT(isMainThread());
     auto cachePartition = origin.topOrigin == origin.clientOrigin ? emptyString() : ResourceRequest::partitionName(origin.topOrigin.host());
     removeResourcesWithOrigin(origin.clientOrigin.securityOrigin(), cachePartition);
 }
 
 void MemoryCache::removeResourcesWithOrigins(PAL::SessionID sessionID, const HashSet<RefPtr<SecurityOrigin>>& origins)
 {
+    RELEASE_ASSERT(isMainThread());
     auto* resourceMap = sessionResourceMap(sessionID);
     if (!resourceMap)
         return;
@@ -563,6 +591,7 @@ void MemoryCache::removeResourcesWithOrigins(PAL::SessionID sessionID, const Has
 
 void MemoryCache::getOriginsWithCache(SecurityOriginSet& origins)
 {
+    RELEASE_ASSERT(isMainThread());
     for (auto& resources : m_sessionResources.values()) {
         for (auto& keyValue : *resources) {
             auto& resource = *keyValue.value;
@@ -577,6 +606,8 @@ void MemoryCache::getOriginsWithCache(SecurityOriginSet& origins)
 
 HashSet<RefPtr<SecurityOrigin>> MemoryCache::originsWithCache(PAL::SessionID sessionID) const
 {
+    RELEASE_ASSERT(isMainThread());
+
     HashSet<RefPtr<SecurityOrigin>> origins;
 
     auto it = m_sessionResources.find(sessionID);
@@ -596,30 +627,35 @@ HashSet<RefPtr<SecurityOrigin>> MemoryCache::originsWithCache(PAL::SessionID ses
 
 void MemoryCache::removeFromLiveDecodedResourcesList(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     m_liveDecodedResources.remove(resource);
 }
 
 void MemoryCache::insertInLiveDecodedResourcesList(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     // Make sure we aren't in the list already.
     ASSERT(!m_liveDecodedResources.contains(resource));
     m_liveDecodedResources.add(resource);
 }
 
 void MemoryCache::addToLiveResourcesSize(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     m_liveSize += resource.size();
     m_deadSize -= resource.size();
 }
 
 void MemoryCache::removeFromLiveResourcesSize(CachedResource& resource)
 {
+    RELEASE_ASSERT(isMainThread());
     m_liveSize -= resource.size();
     m_deadSize += resource.size();
 }
 
 void MemoryCache::adjustSize(bool live, long long delta)
 {
+    RELEASE_ASSERT(isMainThread());
     if (live) {
         ASSERT(delta >= 0 || (static_cast<long long>(m_liveSize) + delta >= 0));
         m_liveSize += delta;
@@ -688,6 +724,7 @@ MemoryCache::Statistics MemoryCache::getStatistics()
 
 void MemoryCache::setDisabled(bool disabled)
 {
+    RELEASE_ASSERT(isMainThread());
     m_disabled = disabled;
     if (!m_disabled)
         return;
@@ -701,6 +738,7 @@ void MemoryCache::setDisabled(bool disabled)
 
 void MemoryCache::evictResources()
 {
+    RELEASE_ASSERT(isMainThread());
     if (disabled())
         return;
 
@@ -710,6 +748,7 @@ void MemoryCache::evictResources()
 
 void MemoryCache::evictResources(PAL::SessionID sessionID)
 {
+    RELEASE_ASSERT(isMainThread());
     if (disabled())
         return;
 
@@ -725,6 +764,7 @@ bool MemoryCache::needsPruning() const
 
 void MemoryCache::prune()
 {
+    RELEASE_ASSERT(isMainThread());
     if (!needsPruning())
         return;
 
@@ -734,6 +774,7 @@ void MemoryCache::prune()
 
 void MemoryCache::pruneSoon()
 {
+    RELEASE_ASSERT(isMainThread());
     if (m_pruneTimer.isActive())
         return;
     if (!needsPruning())