590346 - TM: Global Object created in GlobalForLocation needs its own compartment

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Comment 1

[Gregor Wagner [:gwagner]]

stack:
Reading symbols for shared libraries . done
Assertion failure: (thingKind == js::gc::FINALIZE_STRING) || (thingKind == js::gc::FINALIZE_SHORT_STRING), at /Users/idefix2/moz/ws3/js/src/jsgcinlines.h:65

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00000001018bd542 in JS_Assert (s=0x101c58010 "(thingKind == js::gc::FINALIZE_STRING) || (thingKind == js::gc::FINALIZE_SHORT_STRING)", file=0x101c57fb8 "/Users/idefix2/moz/ws3/js/src/jsgcinlines.h", ln=65) at /Users/idefix2/moz/ws3/js/src/jsutil.cpp:80
80	    *((int *) NULL) = 0;  /* To continue from here in GDB: "return" then "continue". */
(gdb) bt
#0  0x00000001018bd542 in JS_Assert (s=0x101c58010 "(thingKind == js::gc::FINALIZE_STRING) || (thingKind == js::gc::FINALIZE_SHORT_STRING)", file=0x101c57fb8 "/Users/idefix2/moz/ws3/js/src/jsgcinlines.h", ln=65) at /Users/idefix2/moz/ws3/js/src/jsutil.cpp:80
#1  0x000000010177095f in NewFinalizableGCThing<JSFunction> (cx=0x10688f250, thingKind=1) at jsgcinlines.h:63
#2  0x0000000101770a54 in js_NewGCFunction (cx=0x10688f250) at jsgcinlines.h:114
#3  0x00000001017c04b8 in js::detail::NewObject<false, true> (cx=0x10688f250, clasp=0x1027a7b00, proto=0x117d09960, parent=0x117d07510) at jsobjinlines.h:725
#4  0x00000001017c059f in js::NewFunction (cx=0x10688f250, parent=0x117d07510) at jsobjinlines.h:757
#5  0x00000001017c060b in js_NewFunction (cx=0x10688f250, funobj=0x0, native=0x100f24618 <Dump(JSContext*, JSObject*, unsigned int, jsval_layout*, jsval_layout*)>, nargs=1, flags=0, parent=0x117d07510, atom=0x117d05e40) at /Users/idefix2/moz/ws3/js/src/jsfun.cpp:2720
#6  0x00000001017c083c in js_DefineFunction (cx=0x10688f250, obj=0x117d07510, atom=0x117d05e40, native=0x100f24618 <Dump(JSContext*, JSObject*, unsigned int, jsval_layout*, jsval_layout*)>, nargs=1, attrs=0) at /Users/idefix2/moz/ws3/js/src/jsfun.cpp:2875
#7  0x000000010175734b in JS_DefineFunction (cx=0x10688f250, obj=0x117d07510, name=0x101bcf4e0 "dump", call=0x100f24618 <Dump(JSContext*, JSObject*, unsigned int, jsval_layout*, jsval_layout*)>, nargs=1, attrs=0) at /Users/idefix2/moz/ws3/js/src/jsapi.cpp:4390
#8  0x0000000101757571 in JS_DefineFunctions (cx=0x10688f250, obj=0x117d07510, fs=0x10279e800) at /Users/idefix2/moz/ws3/js/src/jsapi.cpp:4375
#9  0x0000000100f25804 in mozJSComponentLoader::GlobalForLocation (this=0x10684b550, aComponentFile=0x10686fff0, aURI=0x10688eee0, aGlobal=0x10688f0b0, aLocation=0x10688f0b8, exception=0x0) at /Users/idefix2/moz/ws3/js/src/xpconnect/loader/mozJSComponentLoader.cpp:1222
#10 0x0000000100f28865 in mozJSComponentLoader::LoadModuleImpl (this=0x10684b550, aSourceFile=0x10686fff0, aKey=@0x7fff5fbfe890, aComponentURI=0x10688eee0) at /Users/idefix2/moz/ws3/js/src/xpconnect/loader/mozJSComponentLoader.cpp:793
#11 0x0000000100f291a2 in mozJSComponentLoader::LoadModule (this=0x10684b550, aComponentFile=0x10686fff0) at /Users/idefix2/moz/ws3/js/src/xpconnect/loader/mozJSComponentLoader.cpp:728
#12 0x000000010157867f in nsComponentManagerImpl::KnownModule::Load (this=0x10686fd70) at /Users/idefix2/moz/ws3/xpcom/components/nsComponentManager.cpp:929
#13 0x0000000101578789 in nsFactoryEntry::GetFactory (this=0x10686f680) at /Users/idefix2/moz/ws3/xpcom/components/nsComponentManager.cpp:1918
#14 0x0000000101578a80 in nsComponentManagerImpl::CreateInstanceByContractID (this=0x10680f750, aContractID=0x10684b4e8 "@mozilla.org/weave/service;1", aDelegate=0x0, aIID=@0x101c886d0, aResult=0x7fff5fbfebd0) at /Users/idefix2/moz/ws3/xpcom/components/nsComponentManager.cpp:1280
#15 0x00000001015776db in nsComponentManagerImpl::GetServiceByContractID (this=0x10680f750, aContractID=0x10684b4e8 "@mozilla.org/weave/service;1", aIID=@0x101c886d0, result=0x7fff5fbfece8) at /Users/idefix2/moz/ws3/xpcom/components/nsComponentManager.cpp:1646
#16 0x0000000101506337 in CallGetService (aContractID=0x10684b4e8 "@mozilla.org/weave/service;1", aIID=@0x101c886d0, aResult=0x7fff5fbfece8) at nsComponentManagerUtils.cpp:94
#17 0x0000000101506368 in nsGetServiceByContractIDWithError::operator() (this=0x7fff5fbfed90, aIID=@0x101c886d0, aInstancePtr=0x7fff5fbfece8) at nsComponentManagerUtils.cpp:288
#18 0x0000000101505071 in nsCOMPtr_base::assign_from_gs_contractid_with_error (this=0x7fff5fbfee00, gs=@0x7fff5fbfed90, iid=@0x101c886d0) at nsCOMPtr.cpp:141
#19 0x0000000100fb20ac in nsCOMPtr<nsISupports>::operator= (this=0x7fff5fbfee00, rhs=@0x7fff5fbfed90) at nsCOMPtr.h:1054
#20 0x0000000100fb1a04 in nsAppStartupNotifier::Observe (this=0x10684b410, aSubject=0x0, aTopic=0x101b16a1a "app-startup", someData=0x0) at /Users/idefix2/moz/ws3/embedding/components/appstartup/src/nsAppStartupNotifier.cpp:100
#21 0x000000010002e2af in XRE_main (argc=1, argv=0x7fff5fbff8b8, aAppData=0x105415b70) at /Users/idefix2/moz/ws3/toolkit/xre/nsAppRunner.cpp:3494
#22 0x0000000100001297 in main (argc=1, argv=0x7fff5fbff8b8) at /Users/idefix2/moz/ws3/browser/app/nsBrowserApp.cpp:158

Comment 2

[Andreas Gal :gal]

Attachment: patch

Comment 3

[Andreas Gal :gal]

Comment on attachment 468854 [details] [diff] [review]
patch

Please test.

Comment 4

[Gregor Wagner [:gwagner]]

Attachment: stack

Still doesn't work.

Comment 5

[Gregor Wagner [:gwagner]]

XPCNativeMember::Resolve calls XPCJSContextStack::GetSafeJSContext and from there we get a new context that points to the defaultCompartment. Seems like bug 590333 is not complete.

Comment 6

[Gregor Wagner [:gwagner]]

JSAutoEnterCompartment autocompartment(cx, tempGlobal); in xpc_CreateGlobalObject resets the compartment again once we leave the function.

Comment 7

[Andreas Gal :gal]

But it should be reset to the previous value, not the default compartment. No?

Comment 8

[Gregor Wagner [:gwagner]]

But the previous value is the defaultCompartment.

In GetSafeJSContext we create a new Context and for this context we create a new global object with xpc_CreateGlobalObject. Within xpc_CreateGlobalObject we create a new compartment and call JSAutoEnterCompartment to switch the defaultcompartment with the new compartment. Once we leave the function again and return to GetSafeJSContext we have the defaultCompartment in this context again because JSAutoEnterCompartment gets out of scope.

Comment 9

[Andreas Gal :gal]

Attachment: patch

Comment 10

[Andreas Gal :gal]

Please test again. Great diagnosis. Thanks!

Comment 11

[Gregor Wagner [:gwagner]]

The patches got mixed up a little bit. The original patch was for GlobalForLocation but your new patch should have gone into bug 590333.
I will land the original patch with the r+ from mrbkap and post the new patch from comment 9 in bug 590333 again.

Comment 12

[Gregor Wagner [:gwagner]]

http://hg.mozilla.org/tracemonkey/rev/aef91431ff40

Comment 13

[Robert Sayre]

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

Comment 15

[Chris Leary [:cdleary] (not checking bugmail)]

Marking as fixed. If that's not correct, please reopen and remove fixed-in-tracemonkey.