585232 - Assertion when returning JS array as jsval in xpcom service

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[David Dahl :ddahl]

My idl looks like this:

#include "nsISupports.idl"
#include "nsIVariant.idl"

[scriptable, uuid(4ee8fb89-6e23-44c7-a6b5-f17d7d136709)]
interface nsIConsoleStorageService : nsISupports
{
  jsval createStorageBackend(in AString aId);
  
  void removeStorageBackend(in AString aId);

  void recordEvent(in AString aId, in jsval aEvent);

  void recordGlobalEvent(in jsval aEvent);

  jsval getStorageBackend(in AString aId);  

  jsval getGlobalStorageBackend();  
};  


My test suite was working fine until the latest tm-merge:(

Comment 1

[David Dahl :ddahl]

Attachment: backtrace

Comment 2

[David Dahl :ddahl]

Test error:

http://pastebin.mozilla.org/763234

Comment 3

[Luke Wagner [:luke]]

It looks like this is from the changes in bug 578547.

Comment 4

[David Dahl :ddahl]

Sorry, i forgot to link to the source patch: bug 568629



I am using a service to collect and store console messages. I blame sdwilsh for telling me to use jsvals. it is pretty nice to use them.

these tests worked until the tm-merge:

+XPCOMUtils.defineLazyServiceGetter(this, "consoleStorage",
+                                   "@mozilla.org/console-storage-service;1",
+                                   "nsIConsoleStorageService");
+function test() {
+  content.location = TEST_URI;
+
+  waitForExplicitFinish();
+  browser.addEventListener("DOMContentLoaded", function onLoad(event) {
+    browser.removeEventListener("DOMContentLoaded", onLoad, false);
+    executeSoon(function () {
+      ok(consoleStorage, "consoleStorageSvc exists");
+
+      var _backend = consoleStorage.createStorageBackend("foo");
+      ok(_backend.push, "we have a backend, it is an array");
+
+      consoleStorage.recordEvent("foo", {fakeMessage: "there was an event"});
+      var eventArray = consoleStorage.getStorageBackend("foo");
+      ok(eventArray[0].fakeMessage,
+         "a message was retreived: " + eventArray[0].fakeMessage);
+
+      var _globalBackend = consoleStorage.getGlobalStorageBackend();
+      ok(_globalBackend.length == 1, "We have a global backend");
+
+      finish();
+    });
+
+  }, false);


No that I am doing things correctly - or anything like that:)

Comment 5

[Luke Wagner [:luke]]

Attachment: maybe fix

So this fix is based on a partial understanding of whats going on here, so it might be totally wrong, but at least it'll get things moving.

The problem, it seems, is that JSData2Native is being called at the end of nsXPCWrappedJSClass::CallMethod to convert the JS return val to the native return val, where we don't want to use an allocator, since it would leak.  However, IIUC, the caller has already done JSData2Native(useAllocator=true), so the jsval has already been allocated and all we have to do is write to it.  This seems truthy looking at the backtrace in the above comment and looking at the other uses of useAllocator in JSData2Native.

Comment 6

[David Dahl :ddahl]

In the meantime I have changed my idl to use nsIVariant and am golden for now.

Comment 7

[Luke Wagner [:luke]]

David: if its not too much trouble (and I understand if it is), could you see whether the attached patch fixes your problem?

Comment 8

[David Dahl :ddahl]

(In reply to comment #7)
> David: if its not too much trouble (and I understand if it is), could you see
> whether the attached patch fixes your problem?

Not a problem, I knew I forgot to do something today.

Comment 9

[David Dahl :ddahl]

(In reply to comment #7)
> David: if its not too much trouble (and I understand if it is), could you see
> whether the attached patch fixes your problem?

I applied your patch, then reverted my patch to use jsvals. I then built in js/src/xpconnect and toolkit/library and got the exact same ASSERTION. Did I build correctly?

Comment 10

[Luke Wagner [:luke]]

Hmm... well you couldn't have gotten the *exact* same assertion, since the patch replaced that assertion with a branch :)  If you did get the exact same assertion, I would verify that the patch has been applied (it is very short) and/or rebuild the whole browser (although the dirs you mentioned sound right).  If it is a different assertion, could you post?

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

XPConnect is linked into layout, so in order to see a change in XPConnect, you need to rebuild js/src/xpconnect, layout/build, and toolkit/library.

Comment 12

[David Dahl :ddahl]

(In reply to comment #11)
> XPConnect is linked into layout, so in order to see a change in XPConnect, you
> need to rebuild js/src/xpconnect, layout/build, and toolkit/library.

ah. Ok, i have to re-run the build and test then.

Comment 13

[Luke Wagner [:luke]]

Pushed the patch:
http://hg.mozilla.org/tracemonkey/rev/aaa87f0f1afe

David, please post if there are still problems.

Comment 14

[David Dahl :ddahl]

(In reply to comment #13)
> Pushed the patch:
> http://hg.mozilla.org/tracemonkey/rev/aaa87f0f1afe
> 
> David, please post if there are still problems.

I applied the patch attached to this bug, then ran make like so:

make -C js/src/xpconnect && make -C layout/build && make -C toolkit/library  

The patches on bug 568629 were already applied. Crash!

*** Start BrowserChrome Test Results ***
TEST-INFO | checking window state
TEST-START | chrome://mochikit/content/browser/toolkit/components/console/hudservice/tests/browser/browser_ConsoleApiTests.js
Chrome file doesn't exist: /home/ddahl/code/moz/mozilla-central/obj-i686-pc-linux-gnu-debug/_tests/testing/mochitest/browser/toolkit/components/console/hudservice/tests/browser/head.js
[New Thread 0x48b00b70 (LWP 5115)]
pldhash: for the table at address 0x486a5958, the given entrySize of 52 probably favors chaining over double hashing.
[New Thread 0x48dc6b70 (LWP 5116)]
[New Thread 0x48fc7b70 (LWP 5117)]
[New Thread 0x491c8b70 (LWP 5118)]
WARNING: Could not fetch previous flags, URI will be treated like referrer: 'NS_FAILED(rv)', file /home/ddahl/code/moz/mozilla-central/mozilla-central/docshell/base/nsDocShell.cpp, line 10193
++DOMWINDOW == 14 (0x486cdab8) [serial = 14] [outer = 0x470ebe80]
[New Thread 0x49500b70 (LWP 5123)]
i am dumping thisJavaScript strict warning: chrome://mochikit/content/browser/toolkit/components/console/hudservice/tests/browser/browser_ConsoleApiTests.js, line 63: reference to undefined property win.wrappedJSObject.console

Program received signal SIGSEGV, Segmentation fault.
0x41104863 in XPCConvert::JSData2Native (ccx=..., d=0xbfffc2a0, s=..., type=..., useAllocator=0, iid=0xbfffc0ac, pErr=0x0)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcconvert.cpp:638
638	                **((jsval**)d) = s;
(gdb) bt
#0  0x41104863 in XPCConvert::JSData2Native (ccx=..., d=0xbfffc2a0, s=..., type=..., useAllocator=0, iid=0xbfffc0ac, pErr=0x0)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcconvert.cpp:638
#1  0x4112b21a in nsXPCWrappedJSClass::CallMethod (this=0x487f84c0, wrapper=0x487ff400, methodIndex=3, info=0x443a82a8, nativeParams=0xbfffc150)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1784
#2  0x41120fdf in nsXPCWrappedJS::CallMethod (this=0x487ff400, methodIndex=3, info=0x443a82a8, params=0xbfffc150)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcwrappedjs.cpp:570
#3  0x418ffc63 in PrepareAndDispatch (methodIndex=3, self=0x487c4420, args=0xbfffc224)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcstubs_gcc_x86_unix.cpp:95
#4  0x40c5f467 in nsCommonWindowSH::GlobalResolve (aWin=0x486cda80, cx=0x45961c00, obj=0x48837cc0, str=0x45508dc0, did_resolve=0xbfffc464)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/dom/base/nsDOMClassInfo.cpp:6477
#5  0x40c604ec in nsCommonWindowSH::NewResolve (this=0x48213e00, wrapper=0x48770100, cx=0x45961c00, obj=0x48837cc0, id=..., flags=1, objp=0xbfffc630, _retval=0xbfffc634)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/dom/base/nsDOMClassInfo.cpp:6839
#6  0x411421f2 in XPC_WN_Helper_NewResolve (cx=0x45961c00, obj=0x48837cc0, id=..., flags=1, objp=0xbfffc734)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1191
#7  0x424a193a in CallResolveOp (cx=0x45961c00, start=0x48837cc0, obj=0x48837cc0, id=..., flags=1, objp=0xbfffc8dc, propp=0xbfffc8d8, recursedp=0xbfffc7bf)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4333
#8  0x424a1e6d in js_LookupPropertyWithFlags (cx=0x45961c00, obj=0x48837cc0, id=..., flags=1, objp=0xbfffc8dc, propp=0xbfffc8d8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4425
#9  0x424a1757 in js_LookupProperty (cx=0x45961c00, obj=0x48837cc0, id=..., objp=0xbfffc8dc, propp=0xbfffc8d8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4260
#10 0x423eec6d in JSObject::lookupProperty (this=0x48837cc0, cx=0x45961c00, id=..., objp=0xbfffc8dc, propp=0xbfffc8d8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.h:672
#11 0x423e561b in LookupPropertyById (cx=0x45961c00, obj=0x48837cc0, id=..., flags=1, objp=0xbfffc8dc, propp=0xbfffc8d8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsapi.cpp:3043
#12 0x423e727d in GetPropertyDescriptorById (cx=0x45961c00, obj=0x48837cc0, id=..., flags=1, own=0, desc=0xbfffc958)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsapi.cpp:3471
#13 0x423e759d in JS_GetPropertyDescriptorById (cx=0x45961c00, obj=0x48837cc0, id=..., flags=1, desc=0xbfffc958)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsapi.cpp:3523
#14 0x40c5fc3d in nsCommonWindowSH::NewResolve (this=0x48213280, wrapper=0x459dd740, cx=0x45961c00, obj=0x47258a50, id=..., flags=1, objp=0xbfffcba0, _retval=0xbfffcba4)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/dom/base/nsDOMClassInfo.cpp:6618
#15 0x411421f2 in XPC_WN_Helper_NewResolve (cx=0x45961c00, obj=0x47258a50, id=..., flags=1, objp=0xbfffcca4)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1191
#16 0x424a193a in CallResolveOp (cx=0x45961c00, start=0x47258a50, obj=0x47258a50, id=..., flags=1, objp=0xbfffce4c, propp=0xbfffce48, recursedp=0xbfffcd2f)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4333
#17 0x424a1e6d in js_LookupPropertyWithFlags (cx=0x45961c00, obj=0x47258a50, id=..., flags=1, objp=0xbfffce4c, propp=0xbfffce48)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4425
#18 0x424a1757 in js_LookupProperty (cx=0x45961c00, obj=0x47258a50, id=..., objp=0xbfffce4c, propp=0xbfffce48)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4260
#19 0x423eec6d in JSObject::lookupProperty (this=0x47258a50, cx=0x45961c00, id=..., objp=0xbfffce4c, propp=0xbfffce48)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.h:672
#20 0x423e561b in LookupPropertyById (cx=0x45961c00, obj=0x47258a50, id=..., flags=1, objp=0xbfffce4c, propp=0xbfffce48)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsapi.cpp:3043
#21 0x423e727d in GetPropertyDescriptorById (cx=0x45961c00, obj=0x47258a50, id=..., flags=1, own=0, desc=0xbfffcef8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsapi.cpp:3471
---Type <return> to continue, or q <return> to quit--- 
#22 0x423e759d in JS_GetPropertyDescriptorById (cx=0x45961c00, obj=0x47258a50, id=..., flags=1, desc=0xbfffcef8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsapi.cpp:3523
#23 0x4115d738 in XPCWrapper::GetPropertyAttrs (cx=0x45961c00, obj=0x47258a50, interned_id=..., flags=1, wantDetails=0, desc=0xbfffcef8)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/XPCWrapper.cpp:1031
#24 0x4115bfbd in XPCWrapper::NewResolve (cx=0x45961c00, wrapperObj=0x48846360, wantDetails=0, innerObj=0x47258a50, id=..., flags=1, objp=0xbfffcfd4)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/XPCWrapper.cpp:556
#25 0x4114fe8b in XPC_SJOW_NewResolve (cx=0x45961c00, obj=0x48846360, id=..., flags=1, objp=0xbfffcfd4)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/XPCSafeJSObjectWrapper.cpp:758
#26 0x424a193a in CallResolveOp (cx=0x45961c00, start=0x48846360, obj=0x48846360, id=..., flags=1, objp=0xbfffd0c8, propp=0xbfffd0c0, recursedp=0xbfffd05f)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4333
#27 0x424a1e6d in js_LookupPropertyWithFlags (cx=0x45961c00, obj=0x48846360, id=..., flags=65535, objp=0xbfffd0c8, propp=0xbfffd0c0)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4425
#28 0x424a2e63 in js_GetPropertyHelper (cx=0x45961c00, obj=0x48846360, id=..., getHow=1, vp=0xbfffd3d8) at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsobj.cpp:4765
#29 0x4246a533 in js::Interpret (cx=0x45961c00) at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsinterp.cpp:4022
#30 0x42483b59 in InvokeCommon<JSBool (*)(JSContext*, JSObject*, uintN, js::Value*, js::Value*)> (cx=0x45961c00, fun=0x47229cf0, script=0x46beaa60, native=0, args=..., flags=0)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsinterp.cpp:588
#31 0x424813db in js::Invoke (cx=0x45961c00, args=..., flags=0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsinterp.cpp:714
#32 0x42481506 in js::InvokeFriendAPI (cx=0x45961c00, args=..., flags=0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/jsinterp.cpp:737
#33 0x4112ad12 in nsXPCWrappedJSClass::CallMethod (this=0x487de520, wrapper=0x487f1500, methodIndex=3, info=0x44365778, nativeParams=0xbfffe160)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcwrappedjsclass.cpp:1692
#34 0x41120fdf in nsXPCWrappedJS::CallMethod (this=0x487f1500, methodIndex=3, info=0x44365778, params=0xbfffe160)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/js/src/xpconnect/src/xpcwrappedjs.cpp:570
#35 0x418ffc63 in PrepareAndDispatch (methodIndex=3, self=0x487c42f0, args=0xbfffe234)
    at /home/ddahl/code/moz/mozilla-central/mozilla-central/xpcom/reflect/xptcall/src/md/unix/xptcstubs_gcc_x86_unix.cpp:95
#36 0x418e2e4a in nsThread::ProcessNextEvent (this=0x43abb2e0, mayWait=0, result=0xbfffe2cc) at /home/ddahl/code/moz/mozilla-central/mozilla-central/xpcom/threads/nsThread.cpp:547
#37 0x418702c5 in NS_ProcessNextEvent_P (thread=0x43abb2e0, mayWait=0) at nsThreadUtils.cpp:250
#38 0x4170d670 in mozilla::ipc::MessagePump::Run (this=0x43af0b50, aDelegate=0x43a2e7c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/glue/MessagePump.cpp:118
#39 0x4194756b in MessageLoop::RunInternal (this=0x43a2e7c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/chromium/src/base/message_loop.cc:219
#40 0x419474eb in MessageLoop::RunHandler (this=0x43a2e7c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/chromium/src/base/message_loop.cc:202
#41 0x4194748f in MessageLoop::Run (this=0x43a2e7c0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/ipc/chromium/src/base/message_loop.cc:176
#42 0x415b1fc2 in nsBaseAppShell::Run (this=0x453f36a0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/widget/src/xpwidgets/nsBaseAppShell.cpp:175
#43 0x41305781 in nsAppStartup::Run (this=0x454a26d0) at /home/ddahl/code/moz/mozilla-central/mozilla-central/toolkit/components/startup/src/nsAppStartup.cpp:191
#44 0x40335c42 in XRE_main (argc=5, argv=0xbfffea74, aAppData=0x43a10380) at /home/ddahl/code/moz/mozilla-central/mozilla-central/toolkit/xre/nsAppRunner.cpp:3659
#45 0x08049af7 in main (argc=5, argv=0xbfffea74) at /home/ddahl/code/moz/mozilla-central/mozilla-central/browser/app/nsBrowserApp.cpp:158

Comment 15

[David Dahl :ddahl]

note: i do need to update my tree it is about 2 days out of date.

Comment 16

[Luke Wagner [:luke]]

I built debug 32-bit TM tip build on Linux with the v.0.1.7 patch from bug 568629 applied.  I don't get any crashes, but I do get failures, all of the form:

TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/toolkit/components/console/hudservice/tests/browser/browser_ConsoleApiTests.js | we have a window by window ID: null

I put a breakpoint in the T_JSVAL case in JSData2native and it does not hit.  Is there anything else I should do to repro?

Comment 17

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/aaa87f0f1afe