For the bug 638660 we need that any JSClass::trace should allow to to be invoked from non-main thread and even potentially allow to be invoked for the same object from different threads at the same time. XPC_WN_Shared_Trace and XPC_WN_Helper_Trace from xpcwrappednativejsops.cpp violates this as they call GetWrappedNativeOfJSObject that in turn instantiate XPCCallContxt. The latter fails for non-main thread. One way to fix this is to change GetWrappedNativeOfJSObject not to depend on XPCCallContxt and even JSContext. If this is not possible in general, then we should provide a light version of the function tailored for GC marking needs.
I was wrong about CallContext instantiating during marking. That is not used. So what is only problematic is a call to a security manager when XPCWrappedNative::GetWrappedNativeOfJSObject calls XPCWrapper::Unwrap.
Summary: avoid ccx, allocations and any global state change when marking xpconnect wrappers → avoid potential global state change when marking xpconnect wrappers
Created attachment 536146 [details] [diff] [review] v1 The patch passes null as cx parameter to XPCWrapper::GetWrappedNativeOfJSObject and changes the latter to use XPCWrapper::UnsafeUnwrapSecurityWrapper if so.
Attachment #536146 - Flags: review?(gal)
Comment on attachment 536146 [details] [diff] [review] v1 Man this is some ugly code in xpconnect. Thanks for the quick turn-around igor.
Attachment #536146 - Flags: review?(gal) → review+
blake, do we still need that security check and could we do a compartment check instead?
For now, we probably need to keep the security check. Once we have a compartment-per-window, we could probably switch to a compartment check (if I'm understanding your question correctly).
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.