660430 - avoid potential global state change when marking xpconnect wrappers

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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.

Comment 1

[Igor Bukanov]

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.

Comment 2

[Igor Bukanov]

Attachment: v1

The patch passes null as cx parameter to XPCWrapper::GetWrappedNativeOfJSObject and changes the latter to use XPCWrapper::UnsafeUnwrapSecurityWrapper if so.

Comment 3

[Andreas Gal :gal]

Comment on attachment 536146 [details] [diff] [review]
v1

Man this is some ugly code in xpconnect. Thanks for the quick turn-around igor.

Comment 4

[Andreas Gal :gal]

blake, do we still need that security check and could we do a compartment check instead?

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

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).

Comment 6

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/131a5a94e29a

Comment 7

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

http://hg.mozilla.org/mozilla-central/rev/131a5a94e29a