Bug 134165

Summary:	konqueror crashes when sending DOM events
Product:	[Applications] konqueror	Reporter:	Jiri Palecek <jpalecek>
Component:	khtml ecma	Assignee:	Konqueror Developers <konq-bugs>
Status:	RESOLVED FIXED
Severity:	crash	CC:	frank78ac, maksim
Priority:	NOR
Version:	unspecified
Target Milestone:	---
Platform:	Debian testing
OS:	Linux
Latest Commit:		Version Fixed In:
Attachments:	Backtrace from the crash

Description Jiri Palecek 2006-09-16 14:00:59 UTC

Version:            (using KDE KDE 3.5.4)
Installed from:    Debian testing/unstable Packages
Compiler:          gcc-4.1 
OS:                Linux

Hello,

konqueror crashes when sending DOM mutation events. I
wasn't able to make a smaller testcase, but to test it,
go to 

http://www.ms.mff.cuni.cz/~palej3am/crash/crash.html

and in the combobox (SELECT element), select "Body".

Regards
    Jiri Palecek

Comment 1 Jiri Palecek 2006-09-16 14:02:29 UTC

Created attachment 17791 [details]
Backtrace from the crash

Comment 2 Maksim Orlovich 2006-09-16 16:19:27 UTC

==6022==    at 0x78FF0E7: khtml::DocPtr<DOM::DocumentImpl>::get() const (shared.h:151)
==6022==    by 0x796CE79: DOM::NodeBaseImpl::dispatchChildInsertedEvents(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1562)
==6022==    by 0x796D3B2: DOM::NodeBaseImpl::appendChild(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1291)
==6022==    by 0x79692B9: DOM::NodeBaseImpl::cloneChildNodes(DOM::NodeImpl*) (dom_nodeimpl.cpp:1406)
==6022==    by 0x7972F0C: DOM::ElementImpl::finishCloneNode(DOM::ElementImpl*, bool) (dom_elementimpl.cpp:451)
==6022==    by 0x797306E: DOM::ElementImpl::cloneNode(bool) (dom_elementimpl.cpp:436)
==6022==    by 0x7B027AE: DOM::Node::cloneNode(bool) (dom_node.cpp:318)
==6022==    by 0x7A8128B: KJS::DOMNodeProtoFunc::tryCall(KJS::ExecState*, KJS::Object&, KJS::List const&) (kjs_dom.cpp:557)
==6022==    by 0x7A72283: KJS::DOMFunction::call(KJS::ExecState*, KJS::Object&, KJS::List const&) (kjs_binding.cpp:114)
==6022==    by 0x7C2B2DD: KJS::Object::call(KJS::ExecState*, KJS::Object&, KJS::List const&) (object.cpp:73)
==6022==    by 0x7BFF659: KJS::FunctionCallNode::evaluate(KJS::ExecState*) const (nodes.cpp:870)
==6022==    by 0x7BFC563: KJS::AssignNode::evaluate(KJS::ExecState*) const (nodes.cpp:1562)
==6022==  Address 0x75EB194 is 12 bytes inside a block of size 60 free'd
==6022==    at 0x401EBFA: operator delete(void*) (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==6022==    by 0x799A014: DOM::HTMLGenericElementImpl::~HTMLGenericElementImpl() (html_elementimpl.cpp:676)
==6022==    by 0x7964514: khtml::TreeShared<DOM::NodeImpl>::removedLastRef() (shared.h:57)
==6022==    by 0x78FF667: khtml::TreeShared<DOM::NodeImpl>::deref() (shared.h:63)
==6022==    by 0x79818FF: DOM::MutationEventImpl::~MutationEventImpl() (dom2_eventsimpl.cpp:921)
==6022==    by 0x78FFA38: khtml::Shared<DOM::EventImpl>::deref() (shared.h:39)
==6022==    by 0x796CE64: DOM::NodeBaseImpl::dispatchChildInsertedEvents(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1556)
==6022==    by 0x796D3B2: DOM::NodeBaseImpl::appendChild(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1291)
==6022==    by 0x79692B9: DOM::NodeBaseImpl::cloneChildNodes(DOM::NodeImpl*) (dom_nodeimpl.cpp:1406)
==6022==    by 0x7972F0C: DOM::ElementImpl::finishCloneNode(DOM::ElementImpl*, bool) (dom_elementimpl.cpp:451)
==6022==    by 0x797306E: DOM::ElementImpl::cloneNode(bool) (dom_elementimpl.cpp:436)
==6022==    by 0x7B027AE: DOM::Node::cloneNode(bool) (dom_node.cpp:318)

Comment 3 Maksim Orlovich 2006-09-16 16:41:39 UTC

Woops, that was from experimental tree, this one is more accurate:

==27139== Invalid read of size 4
==27139==    at 0x78FD855: DOM::NodeImpl::getDocument() const (dom_nodeimpl.h:275)
==27139==    by 0x796D5C9: DOM::NodeBaseImpl::dispatchChildInsertedEvents(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1571)
==27139==    by 0x796DB25: DOM::NodeBaseImpl::appendChild(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1300)
==27139==    by 0x7969C29: DOM::NodeBaseImpl::cloneChildNodes(DOM::NodeImpl*) (dom_nodeimpl.cpp:1415)
==27139==    by 0x797380C: DOM::ElementImpl::finishCloneNode(DOM::ElementImpl*, bool) (dom_elementimpl.cpp:451)
==27139==    by 0x797396E: DOM::ElementImpl::cloneNode(bool) (dom_elementimpl.cpp:436)
==27139==    by 0x7B030FE: DOM::Node::cloneNode(bool) (dom_node.cpp:320)
==27139==    by 0x7A81C3B: KJS::DOMNodeProtoFunc::tryCall(KJS::ExecState*, KJS::Object&, KJS::List const&) (kjs_dom.cpp:557)
==27139==    by 0x7A72C33: KJS::DOMFunction::call(KJS::ExecState*, KJS::Object&, KJS::List const&) (kjs_binding.cpp:114)
==27139==    by 0x7C2B2DD: KJS::Object::call(KJS::ExecState*, KJS::Object&, KJS::List const&) (object.cpp:73)
==27139==    by 0x7BFF659: KJS::FunctionCallNode::evaluate(KJS::ExecState*) const (nodes.cpp:870)
==27139==    by 0x7BFC563: KJS::AssignNode::evaluate(KJS::ExecState*) const (nodes.cpp:1562)
==27139==  Address 0x62128D4 is 12 bytes inside a block of size 60 free'd
==27139==    at 0x401EBFA: operator delete(void*) (in /usr/lib/valgrind/x86-linux/vgpreload_memcheck.so)
==27139==    by 0x799AAE4: DOM::HTMLGenericElementImpl::~HTMLGenericElementImpl() (html_elementimpl.cpp:674)
==27139==    by 0x78FEFFF: khtml::TreeShared<DOM::NodeImpl>::deref() (shared.h:38)
==27139==    by 0x79824B2: DOM::MutationEventImpl::~MutationEventImpl() (dom2_eventsimpl.cpp:921)
==27139==    by 0x78FF3D8: khtml::Shared<DOM::EventImpl>::deref() (shared.h:16)
==27139==    by 0x796D5B4: DOM::NodeBaseImpl::dispatchChildInsertedEvents(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1565)
==27139==    by 0x796DB25: DOM::NodeBaseImpl::appendChild(DOM::NodeImpl*, int&) (dom_nodeimpl.cpp:1300)
==27139==    by 0x7969C29: DOM::NodeBaseImpl::cloneChildNodes(DOM::NodeImpl*) (dom_nodeimpl.cpp:1415)
==27139==    by 0x797380C: DOM::ElementImpl::finishCloneNode(DOM::ElementImpl*, bool) (dom_elementimpl.cpp:451)
==27139==    by 0x797396E: DOM::ElementImpl::cloneNode(bool) (dom_elementimpl.cpp:436)
==27139==    by 0x7B030FE: DOM::Node::cloneNode(bool) (dom_node.cpp:320)
==27139==    by 0x7A81C3B: KJS::DOMNodeProtoFunc::tryCall(KJS::ExecState*, KJS::Object&, KJS::List const&) (kjs_dom.cpp:557)

Comment 4 Maksim Orlovich 2006-09-16 16:46:00 UTC

OK, the problem appears to be that the event refs/deref 'this', which might not be in a tree, and hence unprotected. ugh. Needs some thought.

Comment 5 Frank Reininghaus 2008-06-08 23:44:49 UTC

I think that bug #163500 filed today might be a duplicate of this one (the backtraces look very similar). Unfortunately, the test case mentioned here is not reachable any more. Jiri, if you still have it, could you attach it here? Maybe this test case is easier to analyse than the Apple page that causes the crash in bug #163500. Or maybe you know already what's going on, Maksim? Comment #4 sounds like you had an idea.

Comment 6 Maksim Orlovich 2008-06-30 20:27:54 UTC

SVN commit 826429 by orlovich:

- Properly guard the cloned node when doing a deep clone node, so it doesn't 
get destroyed by a dispatched mutation event. Since the guard requires ref/deref 
we can no longer return a fresh 0-count pointer, so use PassRefPtr instead.

- Don't dispatch mutation events when converting small attribute implementation
into full-blown Attr.

BUG: 163500
BUG: 134165


 M  +1 -1      dom/dom_node.cpp  
 M  +4 -2      ecma/kjs_dom.cpp  
 M  +12 -12    html/htmlparser.cpp  
 M  +20 -19    xml/dom2_rangeimpl.cpp  
 M  +3 -3      xml/dom_docimpl.h  
 M  +13 -10    xml/dom_elementimpl.cpp  
 M  +3 -3      xml/dom_elementimpl.h  
 M  +1 -1      xml/dom_nodeimpl.cpp  
 M  +2 -1      xml/dom_nodeimpl.h  
 M  +3 -3      xml/dom_textimpl.cpp  
 M  +3 -3      xml/dom_textimpl.h  
 M  +4 -4      xml/dom_xmlimpl.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=826429