Bug 12479 – ASSERTION FAILURE: resource->inCache() in WebCore::Cache::remove

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Matt Lilek]

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

Heh, just when I close bug 11380 :)

[Matt Lilek]

D'oh! I totally missed that.

[Mark Rowe (bdash)]

<rdar://problem/4971219>

Bug 12652 has steps to reproduce.

[Matt Lilek]

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.

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).

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.

(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.

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.

[Maciej Stachowiak]

Comment on attachment 13210 [details]
Replace the ASSERT with a check and allow simultaneous reloads

r=me

[Adele Peterson]

Committed revision 19683.