915613 - Crash in xpc::GetObjectPrincipal when using ctypes

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Chris Coulson]

I thought this might be a bug in my addon, but I'm pretty convinced now that this is actually a Gecko bug, introduced by bug 868122 (specifically https://hg.mozilla.org/mozilla-central/rev/e4522e262dbe).

I see this crash in a Thunderbird addon. Basically, my addon calls g_list_foreach() from glib using ctypes. One of the parameters to g_list_foreach is a callback, which I pass another C function (defined with Library.declare()) to. g_list_foreach calls this callback without the stack unwinding, and triggers a crash that looks like this (I've trimmed the irrelevant frames from this):

#0  xpc::GetObjectPrincipal (obj=obj@entry=0x0) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/xpconnect/wrappers/AccessCheck.cpp:41
#1  0x00007ffff2de4a9d in XPCJSContextStack::Push (this=0x7ffff6c342d0, cx=0x7fffd2437e00) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/xpconnect/src/XPCJSContextStack.cpp:76
#2  0x00007ffff2de6eb1 in CTypesActivityCallback (cx=<optimised out>, type=<optimised out>) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/xpconnect/src/XPCJSRuntime.cpp:1122
#3  XPCJSRuntime::CTypesActivityCallback (cx=0x0, type=3527638528) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/xpconnect/src/XPCJSRuntime.cpp:1119
#4  0x00007ffff3d0690c in js::ctypes::CClosure::ClosureStub (cif=0x7fffde129200, result=0x7ffffffedcf0, args=0x7ffffffedb60, userData=0x7fffcf543c40)
    at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/ctypes/CTypes.cpp:6122
#5  0x00007ffff3d17717 in ffi_closure_unix64_inner (closure=0x7ffff7dfd208, rvalue=0x7ffffffedcf0, reg_args=0x7ffffffedc40, argp=0x7ffffffedd10 "\324\335\376\377\377\177")
    at /build/buildd/thunderbird-24.0~b1+build4/./mozilla/js/src/ctypes/libffi/src/x86/ffi64.c:621
#6  0x00007ffff3d17bec in ffi_closure_unix64 () from /usr/lib/thunderbird/libxul.so
#7  0x00007ffff0e9ae08 in g_list_foreach (list=<optimised out>, func=0x7ffff7dfd208, user_data=0x0) at /build/buildd/glib2.0-2.37.6/./glib/glist.c:949
#8  0x00007ffff3d17a84 in ffi_call_unix64 () from /usr/lib/thunderbird/libxul.so
#9  0x00007ffff3d17350 in ffi_call (cif=0x7fffe218d380, fn=0x7ffff0e9ade0 <g_list_foreach>, rvalue=0x0, avalue=<optimised out>)
    at /build/buildd/thunderbird-24.0~b1+build4/./mozilla/js/src/ctypes/libffi/src/x86/ffi64.c:485
#10 0x00007ffff3d0d9de in js::ctypes::FunctionType::Call (cx=0x7fffd2437e00, argc=0, vp=0x7ffffffee568) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/ctypes/CTypes.cpp:5818
#11 0x00007ffff3a3af01 in CallJSNative (args=..., native=<optimised out>, cx=0x7fffd2437e00) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/jscntxtinlines.h:321
#12 js::Invoke (cx=cx@entry=0x7fffd2437e00, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/vm/Interpreter.cpp:474
#13 0x00007ffff3a4a57d in js::Invoke (cx=cx@entry=0x7fffd2437e00, thisv=..., fval=..., argc=3, argv=<optimised out>, rval=0x7fffe21407c8)
    at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/vm/Interpreter.cpp:531
#14 0x00007ffff3b6f5f8 in js::DirectProxyHandler::call (this=this@entry=0x7ffff5096720 <js::CrossCompartmentWrapper::singleton>, cx=cx@entry=0x7fffd2437e00, proxy=..., proxy@entry=..., 
    args=...) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/jsproxy.cpp:479
#15 0x00007ffff3be2d61 in js::CrossCompartmentWrapper::call (this=0x7ffff5096720 <js::CrossCompartmentWrapper::singleton>, cx=0x7fffd2437e00, wrapper=..., args=...)
    at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/jswrapper.cpp:447
#16 0x00007ffff3b776c8 in js::Proxy::call (cx=0x7fffd2437e00, proxy=proxy@entry=..., args=...) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/jsproxy.cpp:2616
#17 0x00007ffff3b777cf in proxy_Call (cx=<optimised out>, argc=<optimised out>, vp=<optimised out>) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/jsproxy.cpp:3185
#18 0x00007ffff3a3af01 in CallJSNative (args=..., native=<optimised out>, cx=0x7fffd2437e00) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/jscntxtinlines.h:321
#19 js::Invoke (cx=cx@entry=0x7fffd2437e00, args=..., construct=construct@entry=js::NO_CONSTRUCT) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/vm/Interpreter.cpp:474
#20 0x00007ffff3a41d70 in Interpret (cx=cx@entry=0x7fffd2437e00, state=...) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/vm/Interpreter.cpp:2298
#21 0x00007ffff3a487c6 in js::RunScript (cx=cx@entry=0x7fffd2437e00, state=...) at /build/buildd/thunderbird-24.0~b1+build4/mozilla/js/src/vm/Interpreter.cpp:438

The issue seems to be that the context created by ctypes doesn't have a default global, but XPCJSContextStack::Push() requires that it does since https://hg.mozilla.org/mozilla-central/rev/e4522e262dbe landed

Comment 1

[Bobby Holley (:bholley)]

Yeah, we shouldn't be creating this random JSContext in js-ctypes. We should use the SafeJSContext on the main thread and the singleton JSContext everywhere else. I'll write a patch.

Comment 2

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=e69d56d12f86

Comment 3

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=53305db7561b

Comment 4

[Bobby Holley (:bholley)]

Attachment: Introduce a mechanism to get a default context for a runtime, and use that in js-ctypes. v1

This causes us to use the SafeJSContext on main thread, and the singleton
JSContext on workers. I opted for the slightly-heavier-weight dynamic callback,
rather than just setting it as a member on the runtime, because we like to delay
the creation of the SafeJSContext a bit, so I didn't want to spin it up right
when we spin up the runtime.

Comment 5

[Bobby Holley (:bholley)]

CCing khuey and bent so that they're aware that, once this patch lands, we'll be asserting that there's only ever one cx per worker runtime.

Comment 6

[Jason Orendorff [:jorendorff]]

Comment on attachment 806927 [details] [diff] [review]
Introduce a mechanism to get a default context for a runtime, and use that in js-ctypes. v1

Review of attachment 806927 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=me.

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +1210,5 @@
>  }
>  
> +// static
> +
> +JSContext*

Seems like the blank line after "// static" is accidental.

Comment 7

[Bobby Holley (:bholley)]

https://tbpl.mozilla.org/?tree=Try&rev=cdbcc0b6f1bd

Comment 8

[Bobby Holley (:bholley)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/944e6d1fd979

Comment 9

[Chris Coulson]

Thanks for fixing this :)

Comment 10

[Bobby Holley (:bholley)]

(In reply to Chris Coulson from comment #9)
> Thanks for fixing this :)

No problem! When the patches get merged to m-c, can you grab a build and verify that the crash goes away?

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/944e6d1fd979