Title: [200066] trunk/Source/WebCore
- Revision
- 200066
- Author
- [email protected]
- Date
- 2016-04-25 17:50:07 -0700 (Mon, 25 Apr 2016)
Log Message
Crash under MemoryCache::remove()
https://bugs.webkit.org/show_bug.cgi?id=157000
<rdar://problem/23344660>
Reviewed by Andreas Kling.
MemoryCache::evictResources() was caching the number of resources ('size')
in the cache for a particular sessionID, and then proceed to call
MemoryCache::remove() 'size' times using the first item in the HashMap
each time. This was unsafe because resources may be ref'ing each other
and therefore removing one may cause other resources to get removed as
well. In such case, we would call remove() too many times and crash because
we dereferenced resources.begin()->value (with the HashMap being empty).
This patch avoids the issue by copying the resources to a Vector and
ref'ing them first, before going on to remove each one from the cache.
* loader/cache/MemoryCache.cpp:
(WebCore::MemoryCache::forEachSessionResource):
(WebCore::MemoryCache::evictResources):
* loader/cache/MemoryCache.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (200065 => 200066)
--- trunk/Source/WebCore/ChangeLog 2016-04-26 00:44:52 UTC (rev 200065)
+++ trunk/Source/WebCore/ChangeLog 2016-04-26 00:50:07 UTC (rev 200066)
@@ -1,5 +1,29 @@
2016-04-25 Chris Dumez <[email protected]>
+ Crash under MemoryCache::remove()
+ https://bugs.webkit.org/show_bug.cgi?id=157000
+ <rdar://problem/23344660>
+
+ Reviewed by Andreas Kling.
+
+ MemoryCache::evictResources() was caching the number of resources ('size')
+ in the cache for a particular sessionID, and then proceed to call
+ MemoryCache::remove() 'size' times using the first item in the HashMap
+ each time. This was unsafe because resources may be ref'ing each other
+ and therefore removing one may cause other resources to get removed as
+ well. In such case, we would call remove() too many times and crash because
+ we dereferenced resources.begin()->value (with the HashMap being empty).
+
+ This patch avoids the issue by copying the resources to a Vector and
+ ref'ing them first, before going on to remove each one from the cache.
+
+ * loader/cache/MemoryCache.cpp:
+ (WebCore::MemoryCache::forEachSessionResource):
+ (WebCore::MemoryCache::evictResources):
+ * loader/cache/MemoryCache.h:
+
+2016-04-25 Chris Dumez <[email protected]>
+
Crash under WebCore::MutationObserver::deliverAllMutations()
https://bugs.webkit.org/show_bug.cgi?id=156997
<rdar://problem/16542323>
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.cpp (200065 => 200066)
--- trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2016-04-26 00:44:52 UTC (rev 200065)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.cpp 2016-04-26 00:50:07 UTC (rev 200066)
@@ -281,6 +281,19 @@
}
}
+void MemoryCache::forEachSessionResource(SessionID sessionID, const std::function<void (CachedResource&)>& function)
+{
+ auto it = m_sessionResources.find(sessionID);
+ if (it == m_sessionResources.end())
+ return;
+
+ Vector<CachedResourceHandle<CachedResource>> resourcesForSession;
+ copyValuesToVector(*it->value, resourcesForSession);
+
+ for (auto& resource : resourcesForSession)
+ function(*resource);
+}
+
void MemoryCache::destroyDecodedDataForAllImages()
{
MemoryCache::singleton().forEachResource([](CachedResource& resource) {
@@ -727,14 +740,8 @@
if (disabled())
return;
- auto it = m_sessionResources.find(sessionID);
- if (it == m_sessionResources.end())
- return;
- auto& resources = *it->value;
+ forEachSessionResource(sessionID, [this] (CachedResource& resource) { remove(resource); });
- for (int i = 0, size = resources.size(); i < size; ++i)
- remove(*resources.begin()->value);
-
ASSERT(!m_sessionResources.contains(sessionID));
}
Modified: trunk/Source/WebCore/loader/cache/MemoryCache.h (200065 => 200066)
--- trunk/Source/WebCore/loader/cache/MemoryCache.h 2016-04-26 00:44:52 UTC (rev 200065)
+++ trunk/Source/WebCore/loader/cache/MemoryCache.h 2016-04-26 00:50:07 UTC (rev 200066)
@@ -103,6 +103,7 @@
void revalidationFailed(CachedResource& revalidatingResource);
void forEachResource(const std::function<void(CachedResource&)>&);
+ void forEachSessionResource(SessionID, const std::function<void(CachedResource&)>&);
void destroyDecodedDataForAllImages();
// Sets the cache's memory capacities, in bytes. These will hold only approximately,
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
