1396466 - Assertion failure: !existingExpandoObject

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

Assertion failure: !existingExpandoObject, at /src/js/xpconnect/wrappers/XrayWrapper.cpp:1224

#0 0x7f2d6dd22399 in xpc::XrayTraits::attachExpandoObject(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>, nsIPrincipal*) /src/js/xpconnect/wrappers/XrayWrapper.cpp:1215:5
#1 0x7f2d6dd22e00 in xpc::XrayTraits::cloneExpandoChain(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) /src/js/xpconnect/wrappers/XrayWrapper.cpp:1340:34
#2 0x7f2d6dd24003 in xpc::XrayUtils::CloneExpandoChain(JSContext*, JSObject*, JSObject*) /src/js/xpconnect/wrappers/XrayWrapper.cpp:1393:32
#3 0x7f2d7063cc26 in mozilla::dom::ReparentWrapper(JSContext*, JS::Handle<JSObject*>) /src/dom/bindings/BindingUtils.cpp:2266:8
#4 0x7f2d6ef91cba in nsNodeUtils::CloneAndAdopt(nsINode*, bool, bool, nsNodeInfoManager*, JS::Handle<JSObject*>, nsCOMArray<nsINode>*, nsINode*, nsINode**) /src/dom/base/nsNodeUtils.cpp:587:14
#5 0x7f2d6eea478c in nsNodeUtils::Adopt(nsINode*, nsNodeInfoManager*, JS::Handle<JSObject*>, nsCOMArray<nsINode>&) /src/dom/base/nsNodeUtils.h:228:19
#6 0x7f2d6ee440a8 in nsIDocument::AdoptNode(nsINode&, mozilla::ErrorResult&) /src/dom/base/nsDocument.cpp:7935:8
#7 0x7f2d6eea43d0 in nsDocument::AdoptNode(nsIDOMNode*, nsIDOMNode**) /src/dom/base/nsDocument.cpp:7794:34
#8 0x7f2d6ef49052 in AdoptNodeIntoOwnerDoc(nsINode*, nsINode*) /src/dom/base/nsINode.cpp:1532:16
#9 0x7f2d6ef4c23c in nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) /src/dom/base/nsINode.cpp:2439:14
#10 0x7f2d6f4c43a5 in mozilla::dom::NodeBinding::appendChild(JSContext*, JS::Handle<JSObject*>, nsINode*, JSJitMethodCallArgs const&) /src/obj-firefox/dom/bindings/NodeBinding.cpp:885:45
#11 0x7f2d70643e5e in mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) /src/dom/bindings/BindingUtils.cpp:3050:13
#12 0x7f2d755ebf31 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:293:15
#13 0x7f2d755ebadd in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:494:16
#14 0x7f2d755ec975 in InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:539:12
#15 0x7f2d755ecb8c in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:558:10
#16 0x7f2d76119cde in js::ForwardingProxyHandler::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /src/js/src/proxy/Wrapper.cpp:175:12
#17 0x7f2d760e88d8 in js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const /src/js/src/proxy/CrossCompartmentWrapper.cpp:359:23
#18 0x7f2d761049d1 in js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) /src/js/src/proxy/Proxy.cpp:497:21
#19 0x7f2d76106dc6 in js::proxy_Call(JSContext*, unsigned int, JS::Value*) /src/js/src/proxy/Proxy.cpp:757:12
#20 0x7f2d755ebf31 in js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) /src/js/src/jscntxtinlines.h:293:15
#21 0x7f2d755ebc17 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:476:16
#22 0x7f2d755ec975 in InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:539:12
#23 0x7f2d755e13d5 in Interpret(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:3093:18
#24 0x7f2d755cd031 in js::RunScript(JSContext*, js::RunState&) /src/js/src/vm/Interpreter.cpp:434:12
#25 0x7f2d755eba60 in js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) /src/js/src/vm/Interpreter.cpp:512:15
#26 0x7f2d755ec975 in InternalCall(JSContext*, js::AnyInvokeArgs const&) /src/js/src/vm/Interpreter.cpp:539:12
#27 0x7f2d755ecb8c in js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) /src/js/src/vm/Interpreter.cpp:558:10
#28 0x7f2d75e964eb in JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) /src/js/src/jsapi.cpp:2937:12
#29 0x7f2d70131c99 in mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&) /src/obj-firefox/dom/bindings/EventListenerBinding.cpp:47:8
#30 0x7f2d7095fcc1 in void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*) /src/obj-firefox/dist/include/mozilla/dom/EventListenerBinding.h:65:12
#31 0x7f2d7095f846 in mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) /src/dom/events/EventListenerManager.cpp:1109:9
#32 0x7f2d70960ba5 in mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*) /src/dom/events/EventListenerManager.cpp:1283:20
#33 0x7f2d7094c99b in mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:313:17
#34 0x7f2d7094c05f in mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&) /src/dom/events/EventDispatcher.cpp:462:16
#35 0x7f2d7094db3d in mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*) /src/dom/events/EventDispatcher.cpp:822:9
#36 0x7f2d7092eb40 in mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*) /src/dom/events/EventDispatcher.cpp:888:12
#37 0x7f2d6ef4801e in nsINode::DispatchEvent(nsIDOMEvent*, bool*) /src/dom/base/nsINode.cpp:1341:5
#38 0x7f2d6eb6de74 in nsContentUtils::DispatchEvent(nsIDocument*, nsISupports*, nsAString const&, bool, bool, bool, bool*, bool) /src/dom/base/nsContentUtils.cpp:4554:18
#39 0x7f2d6eb6dc1b in nsContentUtils::DispatchTrustedEvent(nsIDocument*, nsISupports*, nsAString const&, bool, bool, bool*) /src/dom/base/nsContentUtils.cpp:4522:10
#40 0x7f2d6ee91ba3 in nsDocument::DispatchContentLoadedEvents() /src/dom/base/nsDocument.cpp:5466:3
#41 0x7f2d6ef09d45 in mozilla::detail::RunnableMethodImpl<nsDocument*, void (nsDocument::*)(), true, (mozilla::RunnableKind)0>::Run() /src/obj-firefox/dist/include/nsThreadUtils.h:1192:13
#42 0x7f2d6ca9d96f in nsThread::ProcessNextEvent(bool, bool*) /src/xpcom/threads/nsThread.cpp:1039:14
#43 0x7f2d6caa2810 in NS_ProcessNextEvent(nsIThread*, bool) /src/xpcom/threads/nsThreadUtils.cpp:521:10
#44 0x7f2d6d60f025 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) /src/ipc/glue/MessagePump.cpp:97:21
#45 0x7f2d6d55fae7 in MessageLoop::RunInternal() /src/ipc/chromium/src/base/message_loop.cc:326:10
#46 0x7f2d6d55f979 in MessageLoop::Run() /src/ipc/chromium/src/base/message_loop.cc:299:3
#47 0x7f2d71e7c6ea in nsBaseAppShell::Run() /src/widget/nsBaseAppShell.cpp:158:27
#48 0x7f2d75065d51 in nsAppStartup::Run() /src/toolkit/components/startup/nsAppStartup.cpp:288:30
#49 0x7f2d751c984e in XREMain::XRE_mainRun() /src/toolkit/xre/nsAppRunner.cpp:4643:22
#50 0x7f2d751cb4fa in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4807:8
#51 0x7f2d751cc3e8 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /src/toolkit/xre/nsAppRunner.cpp:4902:21
#52 0x4ecb68 in do_main(int, char**, char**) /src/browser/app/nsBrowserApp.cpp:236:22
#53 0x4ec480 in main /src/browser/app/nsBrowserApp.cpp:309:16
#54 0x7f2d8bb0582f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
#55 0x41e1b4 in _start (firefox+0x41e1b4)

Comment 1

[Andrew Overholt [:overholt]]

Peter, can you please take a look here?

Comment 2

[Ryan VanderMeulen [:RyanVM]]

INFO: Last good revision: 17920e3ba616e99089ec76e7c35c3dd31650caf3
INFO: First bad revision: 072f8d4a9964129a06d774a5698f7f9f8128c66c
INFO: Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=17920e3ba616e99089ec76e7c35c3dd31650caf3&tochange=072f8d4a9964129a06d774a5698f7f9f8128c66c

Comment 3

[Boris Zbarsky [:bzbarsky]]

Hmm.  I wonder how related this is to bug 1386490.

Comment 4

[Jason Orendorff [:jorendorff]]

Bug 1404107 might fix this. Taking.

Comment 5

[Jason Orendorff [:jorendorff]]

Bug 1404107 does not fix this, but it's pretty easy to fix. Working on it.

Comment 6

[Jason Orendorff [:jorendorff]]

Attachment: Remove Xray expando chains from the weakmap when transplanting nodes

Comment 7

[Jason Orendorff [:jorendorff]]

bz pointed out the problem. When we adopt a node:

- we grab the xray expando chain from the old node,
- transplant it (old node is no longer the target of any xray wrappers, so its expando chain is irrelevant),
- clone the expando chain to the new node.

which works fine until we transplant the same node *back* into that original object. Then we trip an assertion.

The assertion seems worth keeping, so the patch changes the behavior of node adoption to *detach* the xray expando chain from the old node.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8917039 [details] [diff] [review]
Remove Xray expando chains from the weakmap when transplanting nodes

r=me

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Dan, Al, Does this need a sec rating?

Comment 10

[Daniel Veditz [:dveditz]]

Did this turn out to be a potential vulnerability? The description in comment 7 sounds like it isn't (unless replacing the original irrelevant expando leaves things dangling?).

Comment 12

[Boris Zbarsky [:bzbarsky]]

Actually, let me talk this over with Jason to make sure we understand the implications correctly.

Comment 13

[Jason Orendorff [:jorendorff]]

Yeah, after reconsidering what happens if assertions are disabled, I think this is pretty minor after all -- a memory leak.

We can open this bug.

Comment 14

[Jason Orendorff [:jorendorff]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=95c128170fa39bccc95045a86e12fab4987e0f25

Comment 15

[Jason Orendorff [:jorendorff]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/39d2b1b24e0ff48a828c89d6106ba18c0be745a8
Bug 1396466 - Remove Xray expando chains from the weakmap when transplanting nodes. r=bz

Comment 17

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/39d2b1b24e0f

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8917039 [details] [diff] [review]
Remove Xray expando chains from the weakmap when transplanting nodes

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1355109.
[User impact if declined]: Can't land the fix for bug 1355109
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: Bug 1404107
[Is the change risky?]: No
[Why is the change risky/not risky?]: This is just making sure we remove an
   entry we don't plan to use again from a hashtable.  The only risk is that
   we're wrong about "don't plan to use again", but we're fairly confident in
   that.
[String changes made/needed]: None.

Comment 19

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8917039 [details] [diff] [review]
Remove Xray expando chains from the weakmap when transplanting nodes

Needed for bug 1355109, Beta57+

Comment 20

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/4d00f19c020d