Bug 47873

Summary: iOS: ASSERT in Cache::adjustSize running layout tests
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, ddkilzer, joepeck, koivisto, psolanki
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
[PATCH] Proposed Change
none
[PATCH] Return True and Keep Purgeable Resource in the Lists ddkilzer: review+

Description Joseph Pecoraro 2010-10-18 19:37:51 PDT
<rdar://problem/8470328> ASSERTION FAILED: delta >= 0 || ((int)m_deadSize + delta >= 0) in WebCore/loader/Cache.cpp:686 void WebCore::Cache::adjustSize(bool, int)

iOS has Cache::shouldMakeResourcePurgeableOnEviction returning true, and is seeing
the following ASSERT running LayoutTests:

    Thread 0 Crashed:  Dispatch queue: com.apple.main-thread
    0   WebCore    0x01122c09 WebCore::Cache::adjustSize(bool, int) + 201 (Cache.cpp:686)
    1   WebCore    0x01123c1a WebCore::Cache::makeResourcePurgeable(WebCore::CachedResource*) + 168 (Cache.cpp:436)
    2   WebCore    0x01123fc4 WebCore::Cache::pruneDeadResources() + 922 (Cache.cpp:384)
    3   WebCore    0x0112961b WebCore::Cache::prune() + 85 (Cache.h:142)

The ASSERT is:

    ASSERT(delta >= 0 || ((int)m_deadSize + delta >= 0));

It appears as though resources are removing themselves (via adjustSize) multiple times
when pruning dead resources. The "makeResourcePurgeable" performs an adjustSize
on an already purgeable resource.

   (gdb) b WebCore::Cache::adjustSize
   Breakpoint 1 at 0x1336717: file WebCore/loader/Cache.cpp, line 684.

   (gdb) commands
   Type commands for when breakpoint 1 is hit, one per line.
   End with a line saying just "end".
   > bt 10
   > c
   > end

   Breakpoint 1, WebCore::Cache::adjustSize (this=0x881a000, live=false, delta=-45470) at WebCore/loader/Cache.cpp:684
   684	    if (live) {
   #0  WebCore::Cache::adjustSize (this=0x881a000, live=false, delta=-45470) at WebCore/loader/Cache.cpp:684
   #1  0x013377f6 in WebCore::Cache::makeResourcePurgeable (this=0x881a000, resource=0x103bd800) at WebCore/loader/Cache.cpp:435
   #2  0x01337ba0 in WebCore::Cache::pruneDeadResources (this=0x881a000) at WebCore/loader/Cache.cpp:385
   #3  0x0133d207 in WebCore::Cache::prune (this=0x881a000) at Cache.h:141
   #4  0x01338092 in WebCore::Cache::setCapacities (this=0x881a000, minDeadBytes=0, maxDeadBytes=0, totalBytes=4194304) at WebCore/loader/Cache.cpp:418
   #5  0x0573b754 in +[WebView(WebFileInternal) _setCacheModel:] (self=0x57ea814, _cmd=0x57582e0, cacheModel=0) at WebKit/mac/WebView/WebView.mm:6537
   #6  0x0573ae23 in +[WebView(WebFileInternal) _preferencesChangedNotification:] (self=0x57ea814, _cmd=0x57904cb, notification=0x11852240) at WebKit/mac/WebView/WebView.mm:6584
   #7  0x007286c1 in _nsnote_callback ()
   #8  0x00302f99 in __CFXNotificationPost_old ()
   #9  0x0028233a in _CFXNotificationPostNotification ()

   ... a little later ... without it showing up in between ...

   Breakpoint 1, WebCore::Cache::adjustSize (this=0x881a000, live=false, delta=-45470) at WebCore/loader/Cache.cpp:684
   684	    if (live) {
   #0  WebCore::Cache::adjustSize (this=0x881a000, live=false, delta=-45470) at WebCore/loader/Cache.cpp:684
   #1  0x013377f6 in WebCore::Cache::makeResourcePurgeable (this=0x881a000, resource=0x103bd800) at WebCore/loader/Cache.cpp:435
   #2  0x01337ba0 in WebCore::Cache::pruneDeadResources (this=0x881a000) at WebCore/loader/Cache.cpp:385
   #3  0x0133d207 in WebCore::Cache::prune (this=0x881a000) at Cache.h:141
   #4  0x01345c0f in WebCore::CachedResource::removeClient (this=0x118710b0, client=0x11872bf4) at WebCore/loader/CachedResource.cpp:210
   #5  0x01d142a5 in WebCore::ScriptElementData::execute (this=0x11872bf4, cachedScript=0x118710b0) at WebCore/dom/ScriptElement.cpp:207
   #6  0x014d7505 in WebCore::Document::executeScriptSoonTimerFired (this=0x834ec00, timer=0x834f21c) at WebCore/dom/Document.cpp:4966
   #7  0x014f72e9 in WebCore::Timer<WebCore::Document>::fired (this=0x834f21c) at Timer.h:102
   #8  0x01e988c6 in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x902ea50) at WebCore/platform/ThreadTimers.cpp:126
   #9  0x01e9897d in WebCore::ThreadTimers::sharedTimerFired () at WebCore/platform/ThreadTimers.cpp:101

   Program received signal EXC_BAD_ACCESS, Could not access memory.
   Reason: KERN_INVALID_ADDRESS at address: 0xbbadbeef
   0x013367cd in WebCore::Cache::adjustSize (this=0x881a000, live=false, delta=-45470) at WebCore/loader/Cache.cpp:688
   688	        ASSERT(delta >= 0 || ((int)m_deadSize + delta >= 0));
Comment 1 Joseph Pecoraro 2010-10-18 19:46:14 PDT
Created attachment 71120 [details]
[PATCH] Proposed Change

I'll be rebasing and re-running test to verify.

If desired I may be able to make a test for this:

  - Cache a resource
  - Loop and continually prune the cache enough that it would likely underflow (m_deadSize is unsigned and underflowing triggers the ASSERT)
  - Have the test pass if there was no assertion.

Another approach might be to add a method to Cache to verify its integrity.
This might be worth doing, so let me know!
Comment 2 Joseph Pecoraro 2010-10-19 09:36:17 PDT
Actually, I think I can (and should) return "true" here. That way it would not
get evicted the second pass through the pruneDeadResources list and instead
stay in the lists with its "purgeable" but "not purged" state. It would eventually
get evicted if it ever "wasPurged()" in an code block inside of pruning.
Comment 3 Joseph Pecoraro 2010-10-19 10:54:31 PDT
Created attachment 71185 [details]
[PATCH] Return True and Keep Purgeable Resource in the Lists
Comment 4 David Kilzer (:ddkilzer) 2010-10-19 12:39:23 PDT
Comment on attachment 71185 [details]
[PATCH] Return True and Keep Purgeable Resource in the Lists

r=me
Comment 5 Joseph Pecoraro 2010-10-19 13:35:37 PDT
Thanks!

Committed r70077
	M	WebCore/ChangeLog
	M	WebCore/loader/Cache.cpp
r70077 = 4fb7c2de4c7e231e9a2ade392063bffe48002ab7
http://trac.webkit.org/changeset/70077