[Webkit-unassigned] [Bug 15123] Self-replicating code makes Safari hang and eventually crash

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=15123





--- Comment #29 from Adam Barth <abarth at webkit.org>  2011-01-25 21:29:25 PST ---
(From update of attachment 80078)
View in context: https://bugs.webkit.org/attachment.cgi?id=80078&action=review

> Source/WebCore/dom/Document.cpp:105
> +#include "NestingLevelIncrementer.h"

Technically we shouldn't be including headers from the parser directory in other parts of the project.  Ideally we'd move this class to WebCore/platform or to WTF, but I'm not going to make you do that in this patch.  It would be nice to do as a follow-up patch though.

> Source/WebCore/dom/Document.cpp:2256
> +    NestingLevelIncrementer nestingLevelIncrementer(m_writeRecursionDepth);

I worry that the parser can destroy the document and then we'll be twiddling unallocated memory in this object's destructor.  If you look inside the parser, you'll see that it refs the document in some cases.  Some of the comments there or the svn blame for when those refs were added should give you some ideas for how to write a test to see this issue.

> Source/WebCore/dom/Document.cpp:2259
> +    m_writeRecursionIsTooDeep = (m_writeRecursionDepth > 1) && m_writeRecursionIsTooDeep;
> +    m_writeRecursionIsTooDeep = (m_writeRecursionDepth > cMaxWriteRecursionDepth) || m_writeRecursionIsTooDeep;

It's slightly strange that after we panic to the top of the stack m_writeRecursionIsTooDeep is still true even though there's not recursing on the stack.  m_writeRecursionIsTooDeep only gets cleared when write is finally called again with a clean stack.  I'm not sure I see a clean way of clearing the variable though...

> Source/WebCore/ChangeLog:7
> +        Self-replicating code makes Safari hang and eventually crash
> +        https://bugs.webkit.org/show_bug.cgi?id=15123
> +

It would be nice to explain more about why we're making this change.  For example, I would mention that we're trying to match Firefox, which is where these values and the "panic to the top of the stack" behavior come from.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.