Summary: | Protect against resource deletion during iteration in MemoryCache::pruneDeadResourcesToSize | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antti Koivisto <koivisto> | ||||
Component: | Page Loading | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | ap, beidson, jamesr, japhet, kling, koivisto, mrahaman, vangelis, webkit.review.bot | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Antti Koivisto
2012-11-05 06:05:01 PST
Created attachment 172325 [details]
patch
Comment on attachment 172325 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=172325&action=review Looks safe and mostly reasonable. r=me > Source/WebCore/loader/cache/MemoryCache.cpp:311 > + // Protect prev so it can't get deleted during destroyDecodedData(). prev -> 'previous' This appears to have changed the eviction behavior on some of our performance tests, e.g. the spike here: http://build.chromium.org/f/chromium/perf/gpu-mac-release-intel/gpu_throughput/report.html?rev=166239&graph=many_images&trace=software&history=150 Is that expected? The crashing/memory corruption case turns into a case where we interrupt the eviction in the middle, so this can affect eviction behavior. This is not ideal but it was the safest and simplest fix I could think of. Antti, This looks like it introduced a significant perf regression in Chromium that points to additional calls to WebCore::CachedResource::registerHandle and WebCore::CachedResource::unregisterHandle See: http://code.google.com/p/chromium/issues/detail?id=159555 It may have similar perf implications to other parts of WebKit as well. Can anyone please tell me if this fix is integrated in the UIWebView that we use in iPad3? I am seeing a similar crash in iPad3 & wanted to know if this patch is already used there? I tried to find in apple bug report (https://bugreport.apple.com) but could not find much details.. We do not comment about products based on WebKit. |