Bug 37798

Reproducable on Mac, using guardmalloc: (gdb) set env DYLD_INSERT_LIBRARIES /usr/lib/libgmalloc.dylib (gdb) run Starting program: /Users/nikolaszimmermann/Coding/WebKit/WebKitBuild/Debug/DumpRenderTree LayoutTests/svg/custom/use-instanceRoot-as-event-target.xhtml GuardMalloc: Allocations will be placed on 16 byte boundaries. GuardMalloc: - Some buffer overruns may not be noticed. GuardMalloc: - Applications using vector instructions (e.g., SSE or Altivec) should work. GuardMalloc: GuardMalloc version 18 GuardMalloc: Allocations will be placed on 16 byte boundaries. GuardMalloc: - Some buffer overruns may not be noticed. GuardMalloc: - Applications using vector instructions (e.g., SSE or Altivec) should work. GuardMalloc: GuardMalloc version 18 Reading symbols for shared libraries . done Reading symbols for shared libraries . done GuardMalloc: Allocations will be placed on 16 byte boundaries. GuardMalloc: - Some buffer overruns may not be noticed. GuardMalloc: - Applications using vector instructions (e.g., SSE or Altivec) should work. GuardMalloc: GuardMalloc version 18 Reading symbols for shared libraries ++ done Reading symbols for shared libraries ..... done Reading symbols for shared libraries + done ASSERTION FAILED: m_wrapper || !m_jsFunction (/Users/nikolaszimmermann/Coding/WebKit/WebCore/bindings/js/JSEventListener.h:83 JSC::JSObject* WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext*) const) Program received signal EXC_BAD_ACCESS, Could not access memory. Reason: KERN_PROTECTION_FAILURE at address: 0x00000000 0x00000000 in ?? () (gdb) bt #0 0x00000000 in ?? () #1 0x0452f6f7 in WebCore::JSEventListener::jsFunction (this=0xdea91fe0, scriptExecutionContext=0xd995f144) at JSEventListener.h:83 #2 0x045d0afc in WebCore::JSEventListener::handleEvent (this=0xdea91fe0, scriptExecutionContext=0xd995f144, event=0xe2ddaf80) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/bindings/js/JSEventListener.cpp:68 #3 0x0434f842 in WebCore::EventTarget::fireEventListeners (this=0xdd62af00, event=0xe2ddaf80, d=0xdd664fa0, entry=@0xdea97fe0) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/EventTarget.cpp:315 #4 0x0434ff16 in WebCore::EventTarget::fireEventListeners (this=0xdd62af00, event=0xe2ddaf80) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/EventTarget.cpp:276 #5 0x048359d1 in WebCore::Node::handleLocalEvents (this=0xdd62af00, event=0xe2ddaf80) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/Node.cpp:2588 #6 0x0483611d in WebCore::Node::dispatchGenericEvent (this=0xdd62af00, prpEvent=@0xbfffd2ac) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/Node.cpp:2731 #7 0x0483661f in WebCore::Node::dispatchEvent (this=0xdd62af00, prpEvent=@0xbfffd38c) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/Node.cpp:2651 #8 0x048347f5 in WebCore::Node::dispatchMouseEvent (this=0xdd62af00, eventType=@0xd400cdcc, button=-1, detail=0, pageX=50, pageY=50, screenX=-9950, screenY=-9950, ctrlKey=false, altKey=false, shiftKey=false, metaKey=false, isSimulated=false, relatedTargetArg=0xddd13fc0, underlyingEvent=@0xbfffd454) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/Node.cpp:2943 #9 0x04834d01 in WebCore::Node::dispatchMouseEvent (this=0xdd62af00, event=@0xbfffd754, eventType=@0xd400cdcc, detail=0, relatedTarget=0xddd13fc0) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/Node.cpp:2852 #10 0x0433e7be in WebCore::EventHandler::updateMouseEventTargetNode (this=0xd2f65e70, targetNode=0xdd62af00, mouseEvent=@0xbfffd754, fireMouseOverOut=true) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/page/EventHandler.cpp:1792 #11 0x0433e868 in WebCore::EventHandler::dispatchMouseEvent (this=0xd2f65e70, eventType=@0xd400cdbc, targetNode=0xdd62af00, clickCount=0, mouseEvent=@0xbfffd754, setUnder=true) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/page/EventHandler.cpp:1806 #12 0x04342fc3 in WebCore::EventHandler::handleMouseMoveEvent (this=0xd2f65e70, mouseEvent=@0xbfffd754, hoveredNode=0xbfffd6dc) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/page/EventHandler.cpp:1480 #13 0x04343061 in WebCore::EventHandler::mouseMoved (this=0xd2f65e70, event=@0xbfffd754) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/page/EventHandler.cpp:1367 #14 0x043491e5 in WebCore::EventHandler::mouseMoved (this=0xd2f65e70, event=0xe2dbefc0) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/page/mac/EventHandlerMac.mm:610 #15 0x00c1d822 in -[WebHTMLView(WebPrivate) _updateMouseoverWithEvent:] (self=0xd3b1dfa0, _cmd=0xd029b2, event=0xe2dbefc0) at /Users/nikolaszimmermann/Coding/WebKit/WebKit/mac/WebView/WebHTMLView.mm:1617 Will investigate soon...

Also on Leopard: http://build.webkit.org/results/Leopard%20Intel%20Debug%20(Tests)/r58843%20(13962)/results.html

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

Oh my, I know what's going on, a pretty evil race. Here are some notes from my last gdb session with breakpoints on SVGElementInstance & JSSVGElementInstance constructor/destructors. #1) Initial creation of SVGElementInstance root object Breakpoint 7, WebCore::SVGElementInstance::SVGElementInstance (this=0x1bd08a50, useElement=0x1bd5ee40, originalElement=@0xbfffdfcc) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/svg/SVGElementInstance.cpp:49 #2) Initial association between the shadowTreeElement & the SVGElementInstance Breakpoint 5, WebCore::SVGElementInstance::setShadowTreeElement (this=0x1bd08a50, element=0x1bd5f030) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/svg/SVGElementInstance.cpp:81 #3) Initial creation of the JS wrapper used from the test Breakpoint 12, WebCore::JSSVGElementInstance::JSSVGElementInstance (this=0xf50380, structure=@0xbfffda84, globalObject=0xf4a940, impl=@0xbfffda88) at /Users/nikolaszimmermann/Coding/WebKit/WebKitBuild/Debug/DerivedSources/WebCore/JSSVGElementInstance.cpp:214 .... At some point, removeAttribute() is called on the correspondingElement of the SVGElementInstance (aka. the original element that the <use> element is referencing), which causes a reclone of the <use shadow tree (lazily! as soon as updateFromElement is called on RenderSVGShadowTreeRootContainer). This is expected, as we're not trying to synchronize the trees, as it's too hard to get right (would require lots of changes in core methods like addAttribute/setAttribute/appendChild/etc..) #4) Recreation of a SVGElementInstance (note that the OLD SVGElementInstance has NOT been destructed yet, as the JSSVGElementInstance is still holding a RefPtr on it) Breakpoint 7, WebCore::SVGElementInstance::SVGElementInstance (this=0x1bd07f90, useElement=0x1bd5ee40, originalElement=@0xbfffb0bc) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/svg/SVGElementInstance.cpp:49 #5) Reassociation of the SVGElementInstance with its shadowTreeElement Breakpoint 5, WebCore::SVGElementInstance::setShadowTreeElement (this=0x1bd07f90, element=0x1bd85370) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/svg/SVGElementInstance.cpp:81 #6) Destruction of the OLD JSSVGElementInstance object (with injected hacks to force GC at exactly this point) Breakpoint 14, WebCore::JSSVGElementInstance::~JSSVGElementInstance (this=0xf50380) at /Users/nikolaszimmermann/Coding/WebKit/WebKitBuild/Debug/DerivedSources/WebCore/JSSVGElementInstance.cpp:218 #7) Assertion fires... ASSERTION FAILED: m_wrapper || !m_jsFunction (/Users/nikolaszimmermann/Coding/WebKit/WebCore/bindings/js/JSEventListener.h:83 JSC::JSObject* WebCore::JSEventListener::jsFunction(WebCore::ScriptExecutionContext*) const) (gdb) p m_wrapper $4 = { <WTFNoncopyable::Noncopyable> = { <WTF::FastAllocBase> = {<No data fields>}, <No data fields>}, members of JSC::WeakGCPtr<JSC::JSObject>: m_ptr = 0xf50380 (gdb) p wrapper() $3 = (class JSC::JSObject *) 0x0 Backtrace is equal to the one I've posted before. Here is the analysis: Stepping up to Node::handleLocalEvents, reveals: #4 0x047213bd in WebCore::Node::handleLocalEvents (this=0x1bd85370, event=0x1bd79f30) at /Users/nikolaszimmermann/Coding/WebKit/WebCore/dom/Node.cpp:2588 handleLocalEvents has been called on the _NEW_ shadowTreeElement (lookup the this pointer above). The JSEventListener is associated with the JSSVGEementInstance object that has just been destructed. A side-note before: SVGElementInstance just pretends to be an EventTarget, but in fact it shares an event listener list with the original correspondingElement (as per SVG spec) -> addEventListener is in fact forwarded to the correspondingElement. But the JSEventListeners (created through JSSVGElementInstance): imp->addEventListener(ustringToAtomicString(args.at(0).toString(exec)), JSEventListener::create(asObject(listener), castedThis, false, currentWorld(exec)), args.at(2).toBoolean(exec)); are created using the _OLD_ JSSVGElementInstance object (which has just been destructed by GC above). The offending code is: useElement.instanceRoot.addEventListener(eventHandler, ...); rectElement.removeAttribute("foo") <-- causes reclone, new SVGElementInstances are born hackToForceGC(); <-- will destruct JSSVGElementInstance moveMouseOverRectInstance(); <-- should fire eventHandler, but hits assertion, because there is no JSSVGElementInstance object anymore So the problem basically exists because we have JSEventListeners floating around which have associated wrappers that are destructed. Glad the assertion actually exists, otherwhise I would have never found it... Not yet sure how to fix this, but I wanted to share this analysis, so it's clear what happens.

Removing PlatformOnly keyword - this happens on all platforms.

Created attachment 55368 [details] Patch

Comment on attachment 55368 [details] Patch WebCore/ChangeLog:14 + it, but residing in the correspondingElement() event listener list (and thus existant!). The last sentence isn't entirely english. registrated and existant aren't words. WebCore/svg/SVGElementInstance.cpp:92 + if (!element || !element->document()) I can never remember if this is the right ->document() pointer or ownerDocument(). Should this be element->inDocument()? WebCore/svg/SVGElementInstance.cpp:150 + ASSERT_NOT_REACHED(); Seems we should add comments in teh code to explain *why* these are ASSERT_NOT_REACHED() LayoutTests/ChangeLog:8 + Make use-instanceRoot-as-event-target.xhtml behave more correctly. Due a copy&paste problem test #7 was not executed. Fixed. copy & paste Seems we need TestObject.idl changes to correspond with your codegenerator changes. Otherwise this looks fantastic!

(In reply to comment #7) > (From update of attachment 55368 [details]) > WebCore/ChangeLog:14 > + it, but residing in the correspondingElement() event listener list > (and thus existant!). > The last sentence isn't entirely english. registrated and existant aren't > words. Fixed. > WebCore/svg/SVGElementInstance.cpp:92 > + if (!element || !element->document()) > I can never remember if this is the right ->document() pointer or > ownerDocument(). Should this be element->inDocument()? I used || !element->inDocument(). Didn't know about this shortcut even. > > WebCore/svg/SVGElementInstance.cpp:150 > + ASSERT_NOT_REACHED(); > Seems we should add comments in teh code to explain *why* these are > ASSERT_NOT_REACHED() Done. > > LayoutTests/ChangeLog:8 > + Make use-instanceRoot-as-event-target.xhtml behave more correctly. > Due a copy&paste problem test #7 was not executed. Fixed. > copy & paste Done. > > Seems we need TestObject.idl changes to correspond with your codegenerator > changes. No this only affects SVGElementInstance.idl generation, nothing else. Going to upload a new patch...

Created attachment 55397 [details] Patch

Comment on attachment 55397 [details] Patch Great work and realy tricky bug. r=me. Reviewed this patch to get bots green again.

Committed r58960: <http://trac.webkit.org/changeset/58960>