Last Comment Bug 509824 - Accessing ScriptSecurityManager off the main thread
: Accessing ScriptSecurityManager off the main thread
Status: VERIFIED FIXED
[needs 1.9.1 approval request]
: verified1.9.2
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: mozilla1.9.3a1
Assigned To: Blake Kaplan (:mrbkap) (please use needinfo!)
:
Mentors:
Depends on:
Blocks: 503926
  Show dependency treegraph
 
Reported: 2009-08-11 14:55 PDT by Ben Turner (not reading bugmail, use the needinfo flag!)
Modified: 2010-02-10 12:39 PST (History)
4 users (show)
benjamin: blocking1.9.2+
mbeltzner: blocking1.9.0.19-
dveditz: wanted1.9.0.x+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
beta1-fixed
.8+
.8-fixed


Attachments
Proposed fix (1.14 KB, patch)
2009-08-11 15:09 PDT, Blake Kaplan (:mrbkap) (please use needinfo!)
bent.mozilla: review+
jst: superreview+
dveditz: approval1.9.1.8+
Details | Diff | Review

Description Ben Turner (not reading bugmail, use the needinfo flag!) 2009-08-11 14:55:13 PDT
Stack:

#0  nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject (this=0x1ac373c0, ccx=@0xb0aa1ebc, jsobj=0x234d8a60, aIID=@0xba1dd0) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcwrappedjsclass.cpp:262
#1  0x00b15848 in nsXPCWrappedJSClass::GetRootJSObject (this=0x1ac373c0, ccx=@0xb0aa1ebc, aJSObj=0x234d8a60) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcwrappedjsclass.cpp:837
#2  0x00b0df9a in nsXPCWrappedJS::GetNewOrUsed (ccx=@0xb0aa1ebc, aJSObj=0x234d8a60, aIID=@0xb0aa1d14, aOuter=0x0, wrapperResult=0xb0aa195c) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcwrappedjs.cpp:326
#3  0x00af1cf2 in XPCConvert::JSObject2NativeInterface (ccx=@0xb0aa1ebc, dest=0xb0aa1c44, src=0x234d8a60, iid=0xb0aa1d14, aOuter=0x0, pErr=0xb0aa1d10) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcconvert.cpp:1451
#4  0x00af3343 in XPCConvert::JSData2Native (ccx=@0xb0aa1ebc, d=0xb0aa1c44, s=592284256, type=@0xb0aa1d47, useAllocator=0, iid=0xb0aa1d14, pErr=0xb0aa1d10) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcconvert.cpp:1017
#5  0x00b1e511 in XPCWrappedNative::CallMethod (ccx=@0xb0aa1ebc, mode=XPCWrappedNative::CALL_SETTER) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcwrappednative.cpp:2550
#6  0x00b2fc8d in XPCWrappedNative::SetAttribute (ccx=@0xb0aa1ebc) at xpcprivate.h:2396
#7  0x00b2b3cb in XPC_WN_GetterSetter (cx=0x169ce00, obj=0x234c4e00, argc=1, argv=0x16b1a30, vp=0xb0aa1fe0) at /src/mozilla/domthreads/js/src/xpconnect/src/xpcwrappednativejsops.cpp:1772
#8  0x00326108 in js_Invoke (cx=0x169ce00, argc=1, vp=0x16b1a28, flags=2) at jsinterp.cpp:1371
#9  0x003264b0 in js_InternalInvoke (cx=0x169ce00, obj=0x234c4e00, fval=592284288, flags=0, argc=1, argv=0xb0aa2658, rval=0xb0aa2658) at jsinterp.cpp:1451
#10 0x00326747 in js_InternalGetOrSet (cx=0x169ce00, obj=0x234c4e00, id=505212076, fval=592284288, mode=JSACC_WRITE, argc=1, argv=0xb0aa2658, rval=0xb0aa2658) at jsinterp.cpp:1521
#11 0x00333c39 in js_SetSprop (cx=0x169ce00, sprop=0x140c2f0, obj=0x234c4e00, vp=0xb0aa2658) at jsscope.h:585
#12 0x00340ddd in js_SetPropertyHelper (cx=0x169ce00, obj=0x234c4e00, id=505212076, cacheResult=1, vp=0xb0aa2658) at /src/mozilla/domthreads/js/src/jsobj.cpp:4510
#13 0x0030dd39 in js_Interpret (cx=0x169ce00) at /src/mozilla/domthreads/js/src/jsinterp.cpp:4833
#14 0x003249b1 in js_Execute (cx=0x169ce00, chain=0x234c4e00, script=0x1da53360, down=0x0, flags=0, result=0xb0aa2b4c) at jsinterp.cpp:1644
#15 0x002a08bd in JS_ExecuteScript (cx=0x169ce00, obj=0x234c4e00, script=0x1da53360, rval=0xb0aa2b4c) at /src/mozilla/domthreads/js/src/jsapi.cpp:5025
#16 0x11ea2704 in nsDOMWorkerScriptLoader::ExecuteScripts (this=0x23529a60, aCx=0x169ce00) at /Users/bent/src/mozilla/domthreads/dom/src/threads/nsDOMWorkerScriptLoader.cpp:269
#17 0x11ea467b in nsDOMWorkerScriptLoader::LoadScripts (this=0x23529a60, aCx=0x169ce00, aURLs=@0xb0aa2bf8, aForWorker=1) at /Users/bent/src/mozilla/domthreads/dom/src/threads/nsDOMWorkerScriptLoader.cpp:148
#18 0x11ea54b2 in nsDOMWorkerScriptLoader::LoadScript (this=0x23529a60, aCx=0x169ce00, aURL=@0x183f37ec, aForWorker=1) at /Users/bent/src/mozilla/domthreads/dom/src/threads/nsDOMWorkerScriptLoader.cpp:164
#19 0x11e92dbc in nsDOMWorker::CompileGlobalObject (this=0x183f37a0, aCx=0x169ce00) at /Users/bent/src/mozilla/domthreads/dom/src/threads/nsDOMWorker.cpp:1597
#20 0x11e92f0b in nsDOMWorker::SetGlobalForContext (this=0x183f37a0, aCx=0x169ce00) at /Users/bent/src/mozilla/domthreads/dom/src/threads/nsDOMWorker.cpp:1504
#21 0x11e88991 in nsDOMWorkerRunnable::Run (this=0x183f3990) at /Users/bent/src/mozilla/domthreads/dom/src/threads/nsDOMThreadService.cpp:384
#22 0x005a8625 in nsThreadPool::Run (this=0x1da73e20) at /Users/bent/src/mozilla/domthreads/xpcom/threads/nsThreadPool.cpp:219
#23 0x005a3b52 in nsThread::ProcessNextEvent (this=0x1da56ee0, mayWait=1, result=0xb0aa2ecc) at /Users/bent/src/mozilla/domthreads/xpcom/threads/nsThread.cpp:527
#24 0x005281ce in NS_ProcessNextEvent_P (thread=0x1da56ee0, mayWait=1) at nsThreadUtils.cpp:230
#25 0x005a3d61 in nsThread::ThreadFunc (arg=0x1da56ee0) at /Users/bent/src/mozilla/domthreads/xpcom/threads/nsThread.cpp:254
#26 0x0016b363 in _pt_root (arg=0x1da68280) at /src/mozilla/domthreads/nsprpub/pr/src/pthreads/ptthread.c:228
#27 0x95b13155 in _pthread_start ()
#28 0x95b13012 in thread_start ()

Blake says we need to not do XPCWrapper::GetSecurityManager if we're not on the main thread.
Comment 1 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-08-11 15:09:28 PDT
Created attachment 393892 [details] [diff] [review]
Proposed fix

This is not the best fix -- it would be better to use nsXPConnect::GetSecurityManagerForJSContext but that addrefs (so is too slow) and hands back an nsIXPCSecurityManager which doesn't have all of the right functions on it. Since, as of now, we can't do security checks like this off the main thread that matter, this patch should be fine.
Comment 2 Ben Turner (not reading bugmail, use the needinfo flag!) 2009-08-12 12:24:10 PDT
Comment on attachment 393892 [details] [diff] [review]
Proposed fix

Don't you want to reverse the order of those two checks? Seems like you'll avoid the IsMainThread check more often if it comes second.

r=me either way.
Comment 3 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-08-12 12:37:25 PDT
the IsMainThread condition is actually a lot faster than the STOBJ_IS_SYSTEM one, so I'd prefer to keep the checks in that order. Did you mean to stamp the patch?
Comment 4 Ben Turner (not reading bugmail, use the needinfo flag!) 2009-08-12 12:54:17 PDT
Comment on attachment 393892 [details] [diff] [review]
Proposed fix

I did!
Comment 5 Ben Turner (not reading bugmail, use the needinfo flag!) 2009-08-18 09:22:01 PDT
Blake, you gonna check this in?
Comment 6 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-08-18 20:57:48 PDT
http://hg.mozilla.org/mozilla-central/rev/af56e987529f
Comment 7 Ben Turner (not reading bugmail, use the needinfo flag!) 2009-08-23 20:06:27 PDT
This is needed on 1.9.2, also :)
Comment 8 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-08-23 23:31:30 PDT
Comment on attachment 393892 [details] [diff] [review]
Proposed fix

Have I mentioned that we have too many branches?
Comment 9 Ben Turner (not reading bugmail, use the needinfo flag!) 2009-08-24 21:45:53 PDT
You're already blocking1.9.2+ (assuming that means what it used to...)
Comment 10 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-08-24 22:36:34 PDT
That means I can't keep track of the rules!
Comment 11 Blake Kaplan (:mrbkap) (please use needinfo!) 2009-08-25 15:15:33 PDT
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/4bcf06d8ce71
Comment 12 Henrik Skupin (:whimboo) 2009-09-04 03:52:24 PDT
Verified fixed based on check-ins on 1.9.2 and trunk.
Comment 13 Daniel Veditz [:dveditz] 2010-01-12 14:53:24 PST
Is attachment 393892 [details] [diff] [review] what you want to check in on the 1.9.1 branch?
Comment 14 Blake Kaplan (:mrbkap) (please use needinfo!) 2010-01-13 11:49:02 PST
Comment on attachment 393892 [details] [diff] [review]
Proposed fix

Yes.
Comment 15 Daniel Veditz [:dveditz] 2010-01-15 13:23:57 PST
Comment on attachment 393892 [details] [diff] [review]
Proposed fix

Approved for 1.9.1.8, a=dveditz for release-drivers
Comment 16 Blake Kaplan (:mrbkap) (please use needinfo!) 2010-01-19 15:40:12 PST
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d27ac705e2d6

Note You need to log in before you can comment on or make changes to this bug.