Bug 3560 – page with use of first-letter crashes reproducibly in RenderObject::renderArena()

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Jens Ayton]

When page is opened, it initially displays correctly. The application is unresponsive. After a couple of 
seconds, it crashes. This is known to occur in Safari 1.2, 1.3 and 2.0, and in Xyle scope 1.0.1, and has 
been observed on a number of machines, and is consistent.

This has been reported before via Safari's bug report button, but there doesn't appear to be a 
corresponding report in Bugzilla.

The maintainer of the page recently bought a Powerbook. I'll try to poke him into adjusting it and trying 
to isolate the crash.

Backtrace and stuff:

Exception:  EXC_BAD_ACCESS (0x0001)
Codes:      KERN_INVALID_ADDRESS (0x0001) at 0x7c4002aa

Thread 0 Crashed:
0   com.apple.WebCore        	0x962f81e4 khtml::RenderObject::renderArena() const + 0x18
1   com.apple.WebCore        	0x9645bb64 khtml::appendRunsForObject(int, int, khtml::
RenderObject*, khtml::BidiState&) + 0x210
2   com.apple.WebCore        	0x9645bc38 khtml::appendRun(khtml::BidiState&) + 0x80
3   com.apple.WebCore        	0x9645cdc4 khtml::RenderBlock::bidiReorderLine(khtml::BidiIterator 
const&, khtml::BidiIterator const&, khtml::BidiState&) + 0x9a8
4   com.apple.WebCore        	0x962f13c4 khtml::RenderBlock::layoutInlineChildren(bool) + 0x7f0
5   com.apple.WebCore        	0x962f56f8 khtml::RenderBlock::layoutBlock(bool) + 0x2b4
6   com.apple.WebCore        	0x962ee918 khtml::RenderBlock::layoutBlockChildren(bool) + 0x2b8
7   com.apple.WebCore        	0x962f5710 khtml::RenderBlock::layoutBlock(bool) + 0x2cc
8   com.apple.WebCore        	0x962ee918 khtml::RenderBlock::layoutBlockChildren(bool) + 0x2b8
9   com.apple.WebCore        	0x962f5710 khtml::RenderBlock::layoutBlock(bool) + 0x2cc
10  com.apple.WebCore        	0x962ee918 khtml::RenderBlock::layoutBlockChildren(bool) + 0x2b8
11  com.apple.WebCore        	0x962f5710 khtml::RenderBlock::layoutBlock(bool) + 0x2cc
12  com.apple.WebCore        	0x962ee918 khtml::RenderBlock::layoutBlockChildren(bool) + 0x2b8
13  com.apple.WebCore        	0x962f5710 khtml::RenderBlock::layoutBlock(bool) + 0x2cc
14  com.apple.WebCore        	0x962ee918 khtml::RenderBlock::layoutBlockChildren(bool) + 0x2b8
15  com.apple.WebCore        	0x962f5710 khtml::RenderBlock::layoutBlock(bool) + 0x2cc
16  com.apple.WebCore        	0x9631291c khtml::RenderCanvas::layout() + 0xfc
17  com.apple.WebCore        	0x9631a234 KHTMLView::layout() + 0x318
18  com.apple.WebCore        	0x9647bd30 DOM::DocumentImpl::implicitClose() + 0x310
19  com.apple.WebCore        	0x9632fff8 KHTMLPart::checkEmitLoadEvent() + 0x22c
20  com.apple.WebCore        	0x963171d4 KHTMLPart::checkCompleted() + 0xe4
21  com.apple.WebCore        	0x96303084 KWQSignal::call(khtml::DocLoader*, khtml::CachedObject*) 
const + 0x90
22  com.apple.WebCore        	0x9645a45c khtml::Loader::slotFinished(KIO::Job*, NSData*) + 0x1ac
23  com.apple.WebCore        	0x964950d8 KWQSignal::call(KIO::Job*, NSData*) const + 0x90
24  com.apple.WebCore        	0x9649aa1c -[KWQResourceLoader finishJobAndHandle:] + 0x3c
25  com.apple.WebKit         	0x9605cb78 -[WebSubresourceClient didFinishLoading] + 0x48
26  com.apple.Foundation     	0x90a6dea4 -[NSURLConnection(NSURLConnectionInternal) 
_sendDidFinishLoadingCallback] + 0x4c
27  com.apple.Foundation     	0x90a3d354 -[NSURLConnection(NSURLConnectionInternal) 
_sendCallbacks] + 0x1f8
28  com.apple.Foundation     	0x90a279e4 -[NSArray makeObjectsPerformSelector:withObject:] + 
0x108
29  com.apple.Foundation     	0x90a51b7c _sendCallbacks + 0xd4
30  com.apple.CoreFoundation 	0x901c3c88 __CFRunLoopDoSources0 + 0x1fc
31  com.apple.CoreFoundation 	0x901c1540 __CFRunLoopRun + 0x1b0
32  com.apple.CoreFoundation 	0x901c5e6c CFRunLoopRunSpecific + 0x148
33  com.apple.HIToolbox      	0x92885f60 RunCurrentEventLoopInMode + 0xac
34  com.apple.HIToolbox      	0x9288c6c8 ReceiveNextEventCommon + 0x17c
35  com.apple.HIToolbox      	0x928ae6a0 BlockUntilNextEventMatchingListInMode + 0x60
36  com.apple.AppKit         	0x92e826dc _DPSNextEvent + 0x180
37  com.apple.AppKit         	0x92e9915c -[NSApplication nextEventMatchingMask:untilDate:inMode:
dequeue:] + 0x74
38  com.apple.Safari         	0x0000bd24 0x1000 + 0xad24
39  com.apple.AppKit         	0x92ead4dc -[NSApplication run] + 0x21c
40  com.apple.AppKit         	0x92f69bec NSApplicationMain + 0x1d0
41  com.apple.Safari         	0x00007fc4 0x1000 + 0x6fc4
42  com.apple.Safari         	0x000546e8 0x1000 + 0x536e8

[Jens Ayton]

Clarification: the page in question is that specified in the "URL" field of the report, i.e. http://
hol.istic.net/

[Nicholas 'Aquarion' Avenell]

The page with the example on it is mine. I'm going to leave that precise page
alone until I can reproduce the exact bug on a static page (Downloading to
desktop apparently stops the thing from crashing) for the purposes of debugging.
If move the broken page (Pretty likely, as Jens mentioned I'm now on a
powerbook, and my site crashing it is annoying ;-) I'll note here where it's gone.

[Chris Petersen]

I'm getting a crash when I load the page in both TOT Webkit and Safari 2.0 (v412).

[Chris Petersen]

Setting to P1 since this is a crash

[Chris Petersen]

Attaching test case of problem.

[Chris Petersen]

Created attachment 2411 [details]
Reduced test case from hol.istic.net

Opening this test case in Safari 2.0 or TOT Webkit results in a crash

[Chris Petersen]

The test case uses a script  that affects several H3 elements in the BODY. The crash occurs when the 
first-letter pseudo selector is applied to one of these heading :

<h3 id="versionsTitle">versionsTitle</h3>.

 When the script attempts to manipulating this element , a crash occurs.

function versionsInit(){
	thing = document.getElementById("revisionsList");
	content = thing.innerHTML;
	
	if (content == ""){
		 document.getElementById("revisions").style.display = "none";
	} else {
		header = document.getElementById("versionsTitle");
		header.onclick = toggleVersions;
		header.content = header.innerHTML
			thing.style.display = "none";
			header.innerHTML = "+ "+header.content;
		}
	}

If you remove this first-letter pseudo selector, the crash no longer occurs.

[Alice Liu]

unable to reproduce

[Nicholas 'Aquarion' Avenell]

Okay, From here (Safari 2.0.2 (416.12)) Clicking on the reduced test case crashes it out still (It no longer 
applies to Hol.istic.net itself since I Saw The Light, bought a Powerbook and started to care a lot more 
about being able to visit my own website in Safari) 

[Darin Adler]

After a little debugging, the problem is obvious, but the solution is not yet obvious. The 
RenderTextFragment created for the first letter points to the same text node as the RenderTextFragment 
for the remaining text. But the text node only points to the remaining text renderer. Thus the text node's 
detach method only destroys the remaining text RenderTextFragment, orphaning a RenderTextFragment 
that now points to a non-existent node.

We have to change something so that both renderers get destroyed. Maybe this can be special code in 
RenderTextFragment that knows about the other RenderTextFragment so that destroying one will destroy 
both.

[Alice Liu]

<rdar://problem/4330356>

[Alice Liu]

*** Bug 4269 has been marked as a duplicate of this bug. ***

[Darin Adler]

Created attachment 6892 [details]
no test case yet, but would like a first pass of review anyway

(In reply to comment #13)
> Created an attachment (id=6892) [edit]

Wrong patch or wrong bug?

[Darin Adler]

Comment on attachment 6892 [details]
no test case yet, but would like a first pass of review anyway

Oops. Attached this patch to the wrong bug!

Created attachment 7012 [details]
Proposed fix, no test and change log yet

I didn't bother with anything more generic than the first-letter case as it seems to be unique. Do you think firstLetter() belongs in a subclass (RenderTextFragmentRemaining?)?

Please r- since there's no test and change log.

[Darin Adler]

Comment on attachment 7012 [details]
Proposed fix, no test and change log yet

Looks like a good fix to me.

Created attachment 7024 [details]
Same patch, with layout test and change log

[Beth Dakin]

I committed this patch.

http://build.webkit.org/results/post-commit-leaks-powerpc-mac-os-x/598/DumpRenderTree-leaks.txt
This patched causes RenderTextFragments to leak. Investigating...