Closed Bug 232989 Opened 21 years ago Closed 21 years ago

Crash - nsAttrAndChildArray::Clear() line 527

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
major

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: bc, Assigned: sicking)

References

(
URL
)

Details

(Keywords: crash)

Attachments

(2 files, 1 obsolete file)

reduced testcases nodesetprefix07/08 and *correct* data files
2.71 KB, application/zip
Details
patch to fix
1.62 KB, patch
bryner
: review+
bryner
: superreview+
Details | Diff | Splinter Review
I get a crash in recent nightly builds but not Mozilla 1.6 when running DOM 2 Core Test Suite. I have seen a couple of different stacks. Example Stack nsAttrAndChildArray::Clear() line 527 + 10 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLDivElement::~nsHTMLDivElement() line 116 + 8 bytes nsHTMLDivElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03ea4568) line 3038 + 218 bytes nsHTMLDivElement::Release(nsHTMLDivElement * const 0x03ea4568) line 120 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLDivElement::~nsHTMLDivElement() line 116 + 8 bytes nsHTMLDivElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x041104b0) line 3038 + 218 bytes nsHTMLDivElement::Release(nsHTMLDivElement * const 0x041104b0) line 120 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLTableCellElement::~nsHTMLTableCellElement() line 132 + 8 bytes nsHTMLTableCellElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x0417c380) line 3038 + 218 bytes nsHTMLTableCellElement::Release(nsHTMLTableCellElement * const 0x0417c380) line 136 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLTableRowElement::~nsHTMLTableRowElement() line 251 + 8 bytes nsHTMLTableRowElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03fcabf0) line 3038 + 218 bytes nsHTMLTableRowElement::Release(nsHTMLTableRowElement * const 0x03fcabf0) line 255 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLTableElement::~nsHTMLTableElement() line 358 + 8 bytes nsHTMLTableElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x04011358) line 3038 + 218 bytes nsHTMLTableElement::Release(nsHTMLTableElement * const 0x04011358) line 362 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLDivElement::~nsHTMLDivElement() line 116 + 8 bytes nsHTMLDivElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03f8e0f0) line 3038 + 218 bytes nsHTMLDivElement::Release(nsHTMLDivElement * const 0x03f8e0f0) line 120 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLTableCellElement::~nsHTMLTableCellElement() line 132 + 8 bytes nsHTMLTableCellElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x042a67c8) line 3038 + 218 bytes nsHTMLTableCellElement::Release(nsHTMLTableCellElement * const 0x042a67c8) line 136 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLTableRowElement::~nsHTMLTableRowElement() line 251 + 8 bytes nsHTMLTableRowElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x040252c8) line 3038 + 218 bytes nsHTMLTableRowElement::Release(nsHTMLTableRowElement * const 0x040252c8) line 255 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLTableElement::~nsHTMLTableElement() line 358 + 8 bytes nsHTMLTableElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03ffb470) line 3038 + 218 bytes nsHTMLTableElement::Release(nsHTMLTableElement * const 0x03ffb470) line 362 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericHTMLElement::~nsGenericHTMLElement() + 18 bytes nsGenericHTMLContainerElement::~nsGenericHTMLContainerElement() + 15 bytes nsHTMLDivElement::~nsHTMLDivElement() line 116 + 8 bytes nsHTMLDivElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03f8e148) line 3038 + 218 bytes nsHTMLDivElement::Release(nsHTMLDivElement * const 0x03f8e148) line 120 + 12 bytes XPCJSRuntime::GCCallback(JSContext * 0x03d973b8, JSGCStatus JSGC_END) line 556 + 18 bytes DOMGCCallback(JSContext * 0x03d973b8, JSGCStatus JSGC_END) line 1869 + 23 bytes js_GC(JSContext * 0x03d973b8, unsigned int 0) line 1419 + 12 bytes js_ForceGC(JSContext * 0x03d973b8, unsigned int 0) line 1000 + 13 bytes JS_GC(JSContext * 0x03d973b8) line 1675 + 11 bytes nsJSContext::Notify(nsJSContext * const 0x04069750, nsITimer * 0x04145718) line 1822 + 13 bytes nsTimerImpl::Fire() line 386 nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x021f1cd8) line 616 nsAppShell::Run(nsAppShell * const 0x00a5b260) line 142 nsAppShellService::Run(nsAppShellService * const 0x00a59fa8) line 484 main1(int 1, char * * 0x002e2638, nsISupports * 0x00a34678) line 1291 + 32 bytes main(int 1, char * * 0x002e2638) line 1678 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL32! 77e814c7() Variables: - child 0x040f1a18 + nsISupports {...} + mDocument 0xdddddddd mParentPtrBits -572662307 i 2 mImpl->mBuffer[i] 0x040f1a18 - this 0x03ea4584 - mImpl 0x03ffbb30 mAttrAndChildCount 7169 mBufferSize 13 + mMappedAttrs 0x00000000 - mBuffer 0x03ffbb3c [0] 0x002ee2e8
The 527 is does |child->SetParent(nsnull);|. Which should only crash if child is null or a dangling pointer. Don't really see how any of that could happen since we keep strong references in the array. Possibly if i've messed up the refcount in the element or some such. I'll look into this when i get into the office later today
> + mDocument 0xdddddddd Whose mDocument is this? Are we calling SetParent() on a deleted node somehow?
I get a different stack when running this locally. Local Stack: nsAttrName::ReleaseInternalName() line 209 + 9 bytes nsAttrName::~nsAttrName() line 81 nsAttrAndChildArray::InternalAttr::~InternalAttr() + 26 bytes nsAttrAndChildArray::InternalAttr::`scalar deleting destructor'(unsigned int 0) + 15 bytes nsAttrAndChildArray::Clear() line 522 nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericContainerElement::~nsGenericContainerElement() + 18 bytes nsXMLElement::~nsXMLElement() line 105 + 8 bytes nsXMLElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03c8ff18) line 3038 + 218 bytes nsXMLElement::Release(nsXMLElement * const 0x03c8ff18) line 144 + 12 bytes nsAttrAndChildArray::Clear() line 528 + 12 bytes nsAttrAndChildArray::~nsAttrAndChildArray() line 77 nsGenericContainerElement::~nsGenericContainerElement() + 18 bytes nsXMLElement::~nsXMLElement() line 105 + 8 bytes nsXMLElement::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsGenericElement::Release(nsGenericElement * const 0x03c6d730) line 3038 + 218 bytes nsXMLElement::Release(nsXMLElement * const 0x03c6d730) line 144 + 12 bytes ReleaseObjects(void * 0x03c6d730, void * 0x00000000) line 152 + 18 bytes nsVoidArray::EnumerateForwards(int (void *, void *)* 0x1001ebc0 ReleaseObjects(void *, void *), void * 0x00000000) line 648 + 21 bytes nsCOMArray_base::Clear() line 160 nsCOMArray<nsIContent>::Clear() line 211 nsDocument::~nsDocument() line 547 nsXMLDocument::~nsXMLDocument() line 198 + 36 bytes nsXMLDocument::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsDocument::Release(nsDocument * const 0x0455ffb8) line 637 + 233 bytes nsXMLDocument::Release(nsXMLDocument * const 0x0455ffb8) line 211 + 12 bytes SubDocClearEntry(PLDHashTable * 0x031dbb88, PLDHashEntryHdr * 0x03bdb8cc) line 1152 + 27 bytes PL_DHashTableFinish(PLDHashTable * 0x031dbb88) line 343 + 16 bytes PL_DHashTableDestroy(PLDHashTable * 0x031dbb88) line 185 + 9 bytes nsDocument::~nsDocument() line 526 + 15 bytes nsHTMLDocument::~nsHTMLDocument() line 319 + 190 bytes nsHTMLDocument::`scalar deleting destructor'(unsigned int 1) + 15 bytes nsDocument::Release(nsDocument * const 0x02f16168) line 637 + 233 bytes nsHTMLDocument::Release(nsHTMLDocument * const 0x02f16168) line 322 + 12 bytes XPCJSRuntime::GCCallback(JSContext * 0x0419ae40, JSGCStatus JSGC_END) line 556 + 18 bytes DOMGCCallback(JSContext * 0x0419ae40, JSGCStatus JSGC_END) line 1869 + 23 bytes js_GC(JSContext * 0x0419ae40, unsigned int 0) line 1419 + 12 bytes js_ForceGC(JSContext * 0x0419ae40, unsigned int 0) line 1000 + 13 bytes JS_GC(JSContext * 0x0419ae40) line 1675 + 11 bytes nsJSContext::Notify(nsJSContext * const 0x0473a7c8, nsITimer * 0x038b31d8) line 1822 + 13 bytes nsTimerImpl::Fire() line 386 nsTimerManager::FireNextIdleTimer(nsTimerManager * const 0x021e1f80) line 616 nsAppShell::Run(nsAppShell * const 0x00a6d248) line 142 nsAppShellService::Run(nsAppShellService * const 0x00a6cf98) line 484 main1(int 1, char * * 0x002e2638, nsISupports * 0x009af410) line 1291 + 32 bytes main(int 1, char * * 0x002e2638) line 1678 + 37 bytes mainCRTStartup() line 338 + 17 bytes KERNEL3 void ReleaseInternalName() { // Since both nsINodeInfo and nsIAtom inherit nsISupports as its first // interface we can safely assume that it's first in the vtable nsISupports* name = NS_REINTERPRET_CAST(nsISupports *, mBits & ~NS_ATTRNAME_NODEINFO_BIT); -> NS_RELEASE(name); } Variables: mBits 74929337 - name 0x047754b8 + __vfptr 0xdddddddd - this 0x03db719c mBits 74929337
bz, in the first stack the mDocument belongs to child. In the second stack __vfptr belongs to name.
Yeah, definitly sounds like somewhere along the stack we're starting to delete deleted objects. My guess is that i've messed up the refcounting somewhere in Insert/Remove/Replace/AppendChild. Note that the way we did refcounting changed slightly when I started to use this class. It used to be that the elements had to NS_ADDREF and NS_RELEASE the elements in the child-array. With the new class the array handles the refcounting itself.
To see a variety of stacks pick either IFRAME or Mozilla XML Implementations and any of the Content Types and run the tests from <http://dom-ts.bclary.com/dist-dom2-core/ecmascript/level2/core/alltests.html>
I wonder if nsAttrAndChildArray::ReplaceChildAt needs to do the same dance nsCOMPtr does when setting the new child. Though I can't think of a senario where releasing a child would end up deleting the node itself. Doesn't hurt to protect against I guess, or at least assert on.
Oh, same goes for RemoveChild and Clear. But I still can't see how it could happen.
Open either nodesetprefix and either reload till you crash or start visiting other documents to crash with different stacks.
original zip had svg and not xml data.
Attachment #141051 - Attachment is obsolete: true
Attached patch patch to fix — — Splinter Review
The problem was the nsDOMAttribute::mNodeInfo is a rawpointer and nsDOMAttribute::SetPrefix doesn't release the new value or addref the old. Why this is a new crash is beyond me, it should have always crashed. Made mNodeInfo into an nsCOMPtr and used |swap| to save a cycle or two.
Assignee: general → bugmail
Status: NEW → ASSIGNED
Comment on attachment 141114 [details] [diff] [review] patch to fix r+sr=bryner
Attachment #141114 - Flags: superreview+
Attachment #141114 - Flags: review+
Attachment #141114 - Flags: superreview?(bzbarsky)
Attachment #141114 - Flags: review?(bryner)
Attachment #141114 - Flags: superreview?(bzbarsky)
Attachment #141114 - Flags: review?(bryner)
Fix checked in. Thanks goes out to Bob for finding this and providing the reduced testcase!!
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
verified fixed.
Status: RESOLVED → VERIFIED
Component: DOM: Core → DOM: Core & HTML
QA Contact: ian → general
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: