Bug 54959 – MemoryCache::revalidationSucceeded accesses possibly freed object

• Status: UNCONFIRMED
• Component: Page Loading

[Fabrizio]

In MemoryCache::revalidationSucceeded(), revalidatingResource is blindly accessed after evict() has been called on it:
=======================================
evict(revalidatingResource);
	
ASSERT(!m_resources.get(resource->url()));
m_resources.set(resource->url(), resource);
resource->setInCache(true);
resource->updateResponseAfterRevalidation(response);
insertInLRUList(resource);
int delta = resource->size();
if (resource->decodedSize() && resource->hasClients())
    insertInLiveDecodedResourcesList(resource);
if (delta)
    adjustSize(resource->hasClients(), delta);
	   
revalidatingResource->switchClientsToRevalidatedResource();
=======================================



An inspection of MemoryCache::evict(CachedResource* resource) shows that it may
delete the resource:
=======================================
if (resource->canDelete())
  delete resource;
=======================================

Therefore, if evict() has been called on revalidatingResource, then revalidatingResource must not be accessed without checking, since it may have been deleted.

[Fabrizio]

Created attachment 83316 [details]
don't access revalidatingResource without a check, since it may have been freed

[Fabrizio]

Created attachment 83317 [details]
don't access revalidatingResource without a check, since it may have been freed

[WebKit Review Bot]

Attachment 83317 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/loader/cache/MemoryCache.cpp:121:  Missing space before ( in if(  [whitespace/parens] [5]
Source/WebCore/loader/cache/MemoryCache.cpp:122:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/MemoryCache.cpp:123:  Tab found; better to use spaces  [whitespace/tab] [1]
Source/WebCore/loader/cache/MemoryCache.cpp:124:  Tab found; better to use spaces  [whitespace/tab] [1]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Fabrizio]

Created attachment 83318 [details]
fix style errors

[Alexey Proskuryakov]

Comment on attachment 83318 [details]
fix style errors

Even if evict() destroys revalidatingResource, the local variable value doesn't change, so this patch has no effect.

Please describe the problem in more detail. Is this something that you observed in practice?