[webkit-dev] Early deletion of DocumentLoader instances

[Raphael Kubo da Costa]

Darin Adler <darin at apple.com> writes:

> On May 23, 2011, at 1:34 PM, Raphael Kubo da Costa wrote:
>
>> While working on the EFL port, I've noticed that sometimes a
>> FrameLoader's DocumentLoader ends up being deleted too early
>> (FrameLoader::setDocumentLoader causes the current DocumentLoader to
>> be
>> deref'ed and freed), in the sense that later on
>> Document::explicitClose tries to access this DocumentLoader instance
>> that has already been freed, causing a crash.
>
> This looks like a basic design problem. The document has a pointer to
> the document loader, and keeps that pointer even after the document
> loader has been destroyed. That is a broken design.
>
> Also, there is a Document::setDocumentLoader function, but nobody ever
> calls it.

I could submit a patch to remove it, if that's desirable.

> What we need are some test cases showing problems caused by this
> mistake that we can use as regression tests;

Hmm, I wonder how to write a test for this. I've experienced this crash
while writing some DRT code for the EFL port. In short, I reuse the same
Frame and Frame::loader() to load each layout test page, and the code
will occasionally crash when some page's contents are delivered. Should
I try to create a manual-test for WebCore?

> then we should fix it by making some better relationship between the
> Document and DocumentLoader that guarantees we won’t have a dangling
> pointer. Either reference counting to keep the object alive, or code
> to zero out the pointer at some point before the object is deleted.

r80507 added a check for m_frame before using it (Document also has a
raw Frame pointer). Perhaps the same should be done here?

-- 
Raphael Kubo da Costa
ProFUSION embedded systems
http://profusion.mobi