|Summary:||ASSERTION FAILURE: resource->inCache() in WebCore::Cache::remove|
|Product:||WebKit||Reporter:||Matt Lilek <dev+webkit>|
|Component:||Page Loading||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||ap, mitz, timothy|
|OS:||OS X 10.4|
Description Matt Lilek 2007-01-30 09:31:48 PST
I've only seen this assertion failure on Colloquy's web plugin, but it's producable by reloading that page a few times. ASSERTION FAILED: resource->inCache() (/Users/matt/Code/WebKit/WebCore/loader/Cache.cpp:163 void WebCore::Cache::remove(WebCore::CachedResource*)) Thread 0 Crashed: 0 com.apple.WebCore 0x011545c0 WebCore::Cache::remove(WebCore::CachedResource*) + 104 (Cache.cpp:163) 1 com.apple.WebCore 0x01159c4c WebCore::Loader::didFail(WebCore::SubresourceLoader*, WebCore::ResourceError const&) + 448 (loader.cpp:135) 2 com.apple.WebCore 0x014f3bb4 WebCore::SubresourceLoader::didFail(WebCore::ResourceError const&) + 212 (SubresourceLoader.cpp:186) 3 com.apple.WebCore 0x014f1e98 WebCore::ResourceLoader::didFail(WebCore::ResourceHandle*, WebCore::ResourceError const&) + 68 4 com.apple.WebCore 0x014c7fdc -[WebCoreResourceHandleAsDelegate connection:didFailWithError:] + 164 (ResourceHandleMac.mm:368) 5 com.apple.Foundation 0x929ba110 -[NSURLConnection(NSURLConnectionInternal) _sendDidFailCallback] + 100 6 com.apple.Foundation 0x9298fab8 -[NSURLConnection(NSURLConnectionInternal) _sendCallbacks] + 556 7 com.apple.Foundation 0x9298f810 _sendCallbacks + 156
Comment 2 Matt Lilek 2007-01-30 11:51:50 PST
D'oh! I totally missed that.
Comment 5 Matt Lilek 2007-02-07 22:17:01 PST
Created attachment 13044 [details] Reduction 1. Load the testcase. 2. From Safari's Debug menu, open the Caches window and tick the "Disable WebCore Caches" checkbox. 3. Click ASSERT! 4. WebKit prints the above assert.
Comment 6 mitz 2007-02-16 15:01:10 PST
Created attachment 13206 [details] Simpler reduction Just hit Command-R to trigger the ASSERT. When you reload, the first Iframe creates a cached resource for the image and it begins to load. Then the second Iframe needs the same URL, but since it's a reload it wants a fresh copy, so it checks if there is a cached resource, and seeing that there is one (put there by the first Iframe), it removes it from the cache. Now the loader learns that the loading failed (since the file doesn't exist) so it wants to remove the resource from the cache, and that's when the assert is hit (because the second frame has already removed it).
Comment 7 mitz 2007-02-16 15:38:59 PST
The reason this doesn't happen with, say, two images with the same src in the same document is that a DocLoader maintains a list of reloaded URLs, so it will not issue multiple requests to load the same URL. I can think of two ways to address this bug: 1) Say that it's okay that separate documents (which can be in different iframes in the same parent document or in two different windows) reload separately, remove the ASSERT, and in the case that the resource is not in the cache skip straight to the canDelete() check. This will have the side effect of actually doing two loads of the same resource in parallel instead of having the second frame/window join in on the reload that's in progress. 2) Manage the "reloaded URLs" or "reloading URLs" list/status in the Cache. Clients will need to tell the cache that they want a resource reloaded, and the cache will see if a reload of the URL is in progress. If so, it will hand back the reloading resource. Otherwise, it will remove an existing resource (if any) and create a new one. The part I'm unclear about here is when a resource should transition from "reloading" to "loaded" status.
Comment 8 mitz 2007-02-16 15:40:58 PST
(In reply to comment #7) > This will have the side effect of > actually doing two loads of the same resource in parallel instead of having the > second frame/window join in on the reload that's in progress. That's how WebKit behaves currently, of course.
Comment 9 mitz 2007-02-17 02:41:29 PST
Created attachment 13210 [details] Replace the ASSERT with a check and allow simultaneous reloads Includes change log and a layout test (for debug builds only). Implements option 1) from comment #7.
Comment 10 Maciej Stachowiak 2007-02-17 08:23:59 PST
Comment on attachment 13210 [details] Replace the ASSERT with a check and allow simultaneous reloads r=me
Comment 11 Adele Peterson 2007-02-17 11:14:02 PST
Committed revision 19683.