Bugzilla@Mozilla – Bug 433758
Crash [@ nsContentList::Item] with null this
Last modified: 2008-09-23 21:33:03 PDT
Summon comment box
reproduced on fedora 6 32bit but not centos5 64bit. #5 0x00002aaab3a3eae0 in nsContentList::Item (this=0x0, aIndex=0, aDoFlush=1) at /work/mozilla/builds/1.9.0/mozilla/content/base/src/nsContentList.cpp:378 #6 0x00002aaab3bbb4d7 in nsHTMLDocument::GetDocumentAllResult ( this=0x21253660, aID=@0x7fff98372c10, aResult=0x7fff98372c40) at /work/mozilla/builds/1.9.0/mozilla/content/html/document/src/nsHTMLDocument.cpp:3892 #7 0x00002aaab3d21360 in nsHTMLDocumentSH::DocumentAllGetProperty ( cx=0x20c4ae10, obj=0x216ae340, id=560133188, vp=0x7fff98372d20) at /work/mozilla/builds/1.9.0/mozilla/dom/src/base/nsDOMClassInfo.cpp:8010 #8 0x00002aaab3d21786 in nsHTMLDocumentSH::DocumentAllNewResolve ( cx=0x20c4ae10, obj=0x216ae340, id=560133188, flags=1, objp=0x7fff98372de0) at /work/mozilla/builds/1.9.0/mozilla/dom/src/base/nsDOMClassInfo.cpp:8098 #9 0x00002aaaaadc8c4c in js_LookupPropertyWithFlags (cx=0x20c4ae10, obj=0x216ae340, id=560133188, flags=1, objp=0x7fff98372ee0, propp=0x7fff98372ed8) at /work/mozilla/builds/1.9.0/mozilla/js/src/jsobj.c:3334
(In reply to comment #0) > reproduced on fedora 6 32bit but not centos5 64bit. ignore that. found on centos5 32 and 64bit. /me has too many windows open.
that's the optimizer confusing the debugger nsHTMLDocument::GetDocumentAllResult(const nsAString& aID, nsISupports** aResult) if (!entry->mDocAllList) { entry->mDocAllList = new nsContentList(root, DocAllResultMatch, nsnull, nsnull, PR_TRUE, id); NS_ENSURE_TRUE(entry->mDocAllList, NS_ERROR_OUT_OF_MEMORY); nsIContent* cont = entry->mDocAllList->Item(1, PR_TRUE); if (cont) { return NS_OK; you are here: NS_IF_ADDREF(*aResult = entry->mDocAllList->Item(0, PR_TRUE));
Created attachment 321944 [details] [review] Keep strong ref contentList->Item(..., PR_TRUE) flushes, so better to keep contentList alive, even when hashtable entry goes away. Would be great to find some small testcase, but at least on Linux I couldn't reproduce the crash with the patch, but without patch crash happens ~50% of time when loading the url.
Olli, do you still want this on the branch? If so, does the current patch apply and can we get a test for it?
Oh yes, we want this. I'll try to write some testcase tomorrow.
Reproduced on Windows (as expected from the patch), reliable crasher in a debug build (not always at the same spot, depending on what random junk was in the content list object). Could not reproduce on the 1.8 branch, is this due to trunk deCOMtamination or similar changes?
The code was added in Bug 259332, so 1.9 only.
Jonas, can you think of any reasonable small test case for this?
Comment on attachment 321944 [details] [review] Keep strong ref The patch does apply cleanly to 1.9.0.x
Hmm.. not really sure exactly what needs to happen in order to trigger the badness here. What *probably* was going wrong here wasn't that the list was going away, but rather that the hash table changed and so the |entry| variable was stale. These steps might do it: 1. Call getElementById some 100 times with different IDs. That will make us make the hash fully live. 2. document.write a bunch of markup which contains lots of elements with ids. Probably in the order of 64 such elements. Let at least 1 element have the id "foo". 3. Call document.all.foo The tricky part is finding the right number of elements in 2 I think. You want enough number that it forces the hash to resize, but you also don't want us to force a flush during the document.write, though flushing might never happen during document.write. If flushing is messy during document.write you could end the current script and put normal markup in the doc instead, before step 3.
Comment on attachment 321944 [details] [review] Keep strong ref Approved for 1.9.0.2. Please land in CVS. a=ss It'd be good to get a test for this landed as well, if possible.
Forgot to add fixed1.9.0.2 CVS Log: 3.787 Olli.Pettay%helsinki.fi 2008-08-20 08:24 Bug 433758, r+sr=jst, a=ss
hi, did we ever get a testcase landed for this? Without one, i am having a hard time verifying this fix on 1.9.0.2 builds. Thanks.
Unfortunately I haven't figured out the testcase yet. Does the URL still work as a test?
The url crashes 3.01 on OS X for me. 3.02 does not crash. I reproduced this multiple times. Verified for 1.9.0.2 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.2) Gecko/2008090212 Firefox/3.0.2.