Bug 47736 - WebCore cache gets corrupted if revalidation request starts at an inopportune time
: WebCore cache gets corrupted if revalidation request starts at an inopportune...
Status: RESOLVED FIXED
: WebKit
Page Loading
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2010-10-15 13:30 PST by
Modified: 2010-10-15 17:42 PST (History)


Attachments
proposed fix (71.14 KB, patch)
2010-10-15 13:38 PST, Alexey Proskuryakov
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-10-15 13:30:55 PST
If a revalidation request cannot be started due to SubresourceLoader::create() returning 0, we don't correctly cancel revalidation. So, a revalidation request with an error bit set gets stuck in cache.

<rdar://problem/8429396>
------- Comment #1 From 2010-10-15 13:38:48 PST -------
Created an attachment (id=70892) [details]
proposed fix
------- Comment #2 From 2010-10-15 14:51:35 PST -------
(From update of attachment 70892 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=70892&action=review

I am finding this a little hard to review because of the logging. Can we land an initial patch that just adds the logging, and then land the bug fix separately?

> WebCore/ChangeLog:6
> +        <rdar://problem/8429396> WebCore cache gets corrupted if revalidation request starts at a wrong time

I would call this “an inopportune time” maybe. It’s not “wrong”, is it?

> WebCore/loader/loader.cpp:387
> +            if (resource->resourceToRevalidate())
> +                cache()->revalidationFailed(resource);

Is this the bug fix?
------- Comment #3 From 2010-10-15 14:59:33 PST -------
> I would call this “an inopportune time” maybe.

A word a day :-)

> > WebCore/loader/loader.cpp:387
> > +            if (resource->resourceToRevalidate())
> > +                cache()->revalidationFailed(resource);
> 
> Is this the bug fix?

Yes. I'll land it separately.
------- Comment #4 From 2010-10-15 15:14:19 PST -------
Committed <http://trac.webkit.org/changeset/69887>, and logging in <http://trac.webkit.org/changeset/69886>.
------- Comment #5 From 2010-10-15 16:48:59 PST -------
http://trac.webkit.org/changeset/69886 might have broken Leopard Intel Debug (Tests)
The following tests are not passing:
http/tests/uri/utf8-path.html
------- Comment #6 From 2010-10-15 16:49:09 PST -------
http://trac.webkit.org/changeset/69887 might have broken Leopard Intel Debug (Tests)
The following tests are not passing:
http/tests/uri/utf8-path.html
------- Comment #7 From 2010-10-15 16:57:51 PST -------
This sounds like it could be caused by the logging patch, but I don't see how exactly. I cannot reproduce this on Snow Leopard.
------- Comment #8 From 2010-10-15 17:19:49 PST -------
OK, I got a stack trace from buildbot. Will fix (not yet sure how, but I will).
------- Comment #9 From 2010-10-15 17:42:54 PST -------
This is a CRASH in FastMalloc, called from String::latin1() via CString::newUninitialized().

Committed speculative fix in <http://trac.webkit.org/changeset/69898>.