Bug 39143 - REGRESSION (r59385) crash destroying inline renderers
Summary: REGRESSION (r59385) crash destroying inline renderers
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL: http://www.766900.com/
Keywords: InRadar, Regression
: 39353 (view as bug list)
Depends on:
Reported: 2010-05-14 16:27 PDT by James Robinson
Modified: 2010-05-20 10:39 PDT (History)
4 users (show)

See Also:

patch (490 bytes, patch)
2010-05-17 18:16 PDT, Enrica Casucci
no flags Details | Formatted Diff | Diff
reduction (478 bytes, text/html)
2010-05-17 19:09 PDT, James Robinson
no flags Details
Patch (10.17 KB, patch)
2010-05-18 19:06 PDT, James Robinson
adele: review-
Details | Formatted Diff | Diff
Patch (3.39 KB, patch)
2010-05-19 11:40 PDT, Enrica Casucci
hyatt: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Robinson 2010-05-14 16:27:51 PDT
Looks very similar to https://bugs.webkit.org/show_bug.cgi?id=39042
Comment 1 James Robinson 2010-05-14 17:09:53 PDT

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000440
0x00000001017dc2d7 in WebCore::RenderInline::destroy (this=0x10a1c85c8) at /usr/local/home/jamesr/WebKit/WebCore/rendering/RenderInline.cpp:64
64	        m_continuation->destroy();
(gdb) bt
#0  0x00000001017dc2d7 in WebCore::RenderInline::destroy (this=0x10a1c85c8) at /usr/local/home/jamesr/WebKit/WebCore/rendering/RenderInline.cpp:64
#1  0x00000001017dc2e4 in WebCore::RenderInline::destroy (this=0x10a1c7838) at /usr/local/home/jamesr/WebKit/WebCore/rendering/RenderInline.cpp:64
#2  0x00000001016fd9f7 in WebCore::Node::detach (this=0x10a1c75b0) at /usr/local/home/jamesr/WebKit/WebCore/dom/Node.cpp:1226
#3  0x0000000101014ffe in WebCore::ContainerNode::detach (this=0x10a1c75b0) at /usr/local/home/jamesr/WebKit/WebCore/dom/ContainerNode.cpp:628
#4  0x0000000101237649 in WebCore::Element::detach (this=0x10a1c75b0) at /usr/local/home/jamesr/WebKit/WebCore/dom/Element.cpp:842
#5  0x0000000101014fd8 in WebCore::ContainerNode::detach (this=0x10a1c6c00) at /usr/local/home/jamesr/WebKit/WebCore/dom/ContainerNode.cpp:626
#6  0x0000000101237649 in WebCore::Element::detach (this=0x10a1c6c00) at /usr/local/home/jamesr/WebKit/WebCore/dom/Element.cpp:842
#7  0x0000000101016383 in WebCore::ContainerNode::removeChild (this=0x10a1c6860, oldChild=0x10a1c6c00, ec=@0x7fff5fbfdc24) at /usr/local/home/jamesr/WebKit/WebCore/dom/ContainerNode.cpp:377
#8  0x000000010136eb0e in WebCore::HTMLParser::handleResidualStyleCloseTagAcrossBlocks (this=0x10a1888b0, elem=0x10a1c68f0) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:1258
#9  0x000000010136f1c6 in WebCore::HTMLParser::popBlock (this=0x10a1888b0, tagName=@0x10a8cfa48, reportErrors=true) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:1445
#10 0x0000000101374683 in WebCore::HTMLParser::processCloseTag (this=0x10a1888b0, t=0x10a8cfa38) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:1024
#11 0x0000000101374003 in WebCore::HTMLParser::parseToken (this=0x10a1888b0, t=0x10a8cfa38) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLParser.cpp:258
#12 0x0000000101387347 in WebCore::HTMLTokenizer::processToken (this=0x10a8cfa00) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1941
#13 0x000000010138e8c6 in WebCore::HTMLTokenizer::parseTag (this=0x10a8cfa00, src=@0x10a8d04e0, state={static EntityShift = 4, m_bits = 0}) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1513
#14 0x000000010138f6c7 in WebCore::HTMLTokenizer::write (this=0x10a8cfa00, str=@0x7fff5fbfe190, appendData=true) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1764
#15 0x0000000101386efb in WebCore::HTMLTokenizer::timerFired (this=0x10a8cfa00) at /usr/local/home/jamesr/WebKit/WebCore/html/HTMLTokenizer.cpp:1848
#16 0x0000000101390ba3 in WebCore::Timer<WebCore::HTMLTokenizer>::fired (this=0x10a8cfc58) at Timer.h:98
#17 0x0000000101a3db7c in WebCore::ThreadTimers::sharedTimerFiredInternal (this=0x10a11dcf0) at /usr/local/home/jamesr/WebKit/WebCore/platform/ThreadTimers.cpp:112
#18 0x0000000101a3dd0b in WebCore::ThreadTimers::sharedTimerFired () at /usr/local/home/jamesr/WebKit/WebCore/platform/ThreadTimers.cpp:90
#19 0x0000000101915a46 in WebCore::timerFired () at /usr/local/home/jamesr/WebKit/WebCore/platform/mac/SharedTimerMac.mm:86
#20 0x00007fff877c4708 in __CFRunLoopRun ()
#21 0x00007fff877c28df in CFRunLoopRunSpecific ()
#22 0x00007fff87ee8ada in RunCurrentEventLoopInMode ()
#23 0x00007fff87ee88df in ReceiveNextEventCommon ()
#24 0x00007fff87ee8798 in BlockUntilNextEventMatchingListInMode ()
#25 0x00007fff86d3da4a in _DPSNextEvent ()
#26 0x00007fff86d3d399 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#27 0x000000010000b6cc in ?? ()
#28 0x00007fff86d0306f in -[NSApplication run] ()
#29 0x00007fff86cfbd8c in NSApplicationMain ()
#30 0x00000001000016f4 in ?? ()
Comment 2 Enrica Casucci 2010-05-17 18:16:12 PDT
Created attachment 56300 [details]

I have a fix, but I'm still trying to create a reduction in order to include a layout test.
Comment 3 James Robinson 2010-05-17 19:09:50 PDT
Created attachment 56307 [details]

This is a reduction from the http://www.766900.com/ page.  It's probably not completely minimal but should be useful as a starting point.  The document.write() loop is a bit unfortunate, but it seems necessary to force the </font> tag to get parsed on a separate callstack from the rest of the page.
Comment 4 mitz 2010-05-18 12:18:00 PDT
(In reply to comment #3)
> Created an attachment (id=56307) [details]
> reduction

Thanks for the reduction!

> The document.write() loop is a bit unfortunate, but it seems necessary to force the </font> tag to get parsed on a separate callstack from the rest of the page.

That’s not really necessary. Just <script> document.body.offsetTop </script> suffices, as it forces layout at that point. Then when parsing continues, the crash occurs.
Comment 5 James Robinson 2010-05-18 19:06:21 PDT
Created attachment 56440 [details]
Comment 6 James Robinson 2010-05-18 19:08:23 PDT
We're seeing this crash on more and more URLs in Chromium's reliability testing which it making it really hard to spot new regressions.  Thus, I've attached a patch to revert r59385 and add the reduction as a layout test to catch regressions.  Please consider landing this if you aren't able to fix the issue very quickly.  Thanks.
Comment 7 mitz 2010-05-18 19:19:10 PDT
Enrica is very close to fixing this.
Comment 8 Enrica Casucci 2010-05-19 09:50:42 PDT
I'll be posting my patch shortly.
Comment 9 Adele Peterson 2010-05-19 11:23:41 PDT
Comment on attachment 56440 [details]

R- since we're actively working on a real fix.
Comment 10 Enrica Casucci 2010-05-19 11:40:44 PDT
Created attachment 56505 [details]
Comment 11 Simon Fraser (smfr) 2010-05-19 11:55:00 PDT
Comment on attachment 56505 [details]

> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog	(revision 59776)
> +++ WebCore/ChangeLog	(working copy)
> @@ -1,3 +1,17 @@
> +2010-05-19  Enrica Casucci  <enrica@apple.com>
> +
> +        Reviewed by NOBODY (OOPS!).
> +
> +        REGRESSION (r59385) crash destroying inline renderers
> +        https://bugs.webkit.org/show_bug.cgi?id=39143
> +        <rdar://problem/8003662>
> +        
> +        Test: fast/inline-block/anonymous-block-crash.html
> +
> +        * rendering/RenderBlock.cpp:
> +        (WebCore::RenderBlock::layoutBlock): resetting the flag m_inlineChildren only for non anonymous blocks otherwise we incurr in
> +        a double deletion of the renderer that couses the crash.

It would be nice of the changelog explained why the crash occurs, and why not resetting the flag for anonymous blocks fixes it. i.e. dump some of your knowledge that you gained from your investigation into the changelog!
Comment 12 Dave Hyatt 2010-05-19 11:56:17 PDT
Comment on attachment 56505 [details]


This should probably be isAnonymousBlock() and not just isAnonymous().  I doubt there's any problem with display: block generated content for example.

We could leave it this way just to be safe though.  There's no harm in being more conservative about the flipping of the childrenInline switch.
Comment 13 Enrica Casucci 2010-05-19 14:54:31 PDT
Added some additional info in the ChangeLog.
Committed revision 59786.
Comment 14 mitz 2010-05-20 10:39:03 PDT
*** Bug 39353 has been marked as a duplicate of this bug. ***