WebCore cache gets corrupted if revalidation request starts at an inopportune time

• Component: Page Loading
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

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>

[Alexey Proskuryakov]

Created attachment 70892 [details]
proposed fix

[Darin Adler]

Comment on attachment 70892 [details]
proposed fix

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?

[Alexey Proskuryakov]

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

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/69887>, and logging in <http://trac.webkit.org/changeset/69886>.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

OK, I got a stack trace from buildbot. Will fix (not yet sure how, but I will).

[Alexey Proskuryakov]

This is a CRASH in FastMalloc, called from String::latin1() via CString::newUninitialized().

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