Last Comment Bug 440982 - To avoid calling JS at unsafe times from JS_GC, jsds_ScriptHookProc should not get the script hook unless it needs to and it is safe to call
: To avoid calling JS at unsafe times from JS_GC, jsds_ScriptHookProc should no...
: assertion, fixed1.8.1.24, fixed1.9.0.12, fixed1.9.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: timeless
: jsd
Depends on:
  Show dependency treegraph
Reported: 2008-06-21 04:32 PDT by bronek
Modified: 2011-07-08 00:24 PDT (History)
4 users (show)
dveditz: wanted1.9.1.x+
dveditz: wanted1.9.0.x+
dveditz: wanted1.8.1.x+
bzbarsky: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

only grab hook if we need it (1.20 KB, patch)
2009-05-27 19:47 PDT, timeless
brendan: review+
brendan: superreview+
Details | Diff | Splinter Review
updated comment (1.30 KB, patch)
2009-06-01 03:00 PDT, timeless
timeless: review+
timeless: superreview+
shaver: approval1.9.1+
dveditz: approval1.9.0.12+
Details | Diff | Splinter Review

Description bronek 2008-06-21 04:32:30 PDT
User-Agent:       Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv: Gecko/20080617 Firefox/
Build Identifier: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv: Gecko/20080617 Firefox/

#0  JS_Assert (s=0x281582b2 "!rt->gcRunning", file=0x28158291 "jsgc.c", ln=1393) at jsutil.c:63
63          abort();
[New Thread 0x834b100 (LWP 100215)]
[New Thread 0x9475500 (LWP 100213)]
[New Thread 0x94f9300 (LWP 100220)]
[New Thread 0x8dd2100 (LWP 100217)]
[New Thread 0x89f3600 (LWP 100216)]
[New Thread 0x8106800 (LWP 100211)]
[New Thread 0x8106500 (LWP 100210)]
[New Thread 0x8103100 (LWP 100159)]
(gdb) bt
#0  JS_Assert (s=0x281582b2 "!rt->gcRunning", file=0x28158291 "jsgc.c", ln=1393) at jsutil.c:63
#1  0x280e1008 in js_NewGCThing (cx=0x8ed2680, flags=1, nbytes=8) at jsgc.c:1393
        rt = (JSRuntime *) 0x81fa000
        flindex = 0
        doGC = Variable "doGC" is not available.
#2  0x28138baa in js_NewString (cx=0x8ed2680, chars=0x8608c20, length=42, gcflag=0) at jsstr.c:2442
        str = Variable "str" is not available.
#3  0x280b8de1 in js_AtomizeString (cx=0x8ed2680, str=0xbfbfdc80, flags=Variable "flags" is not available.
) at jsatom.c:660
        gen = 168171
        keyHash = 2150442760
        key = -1077945212
        state = (JSAtomState *) 0x81fa0e8
        table = (JSHashTable *) 0x81a7160
        he = Variable "he" is not available.
#4  0x280b9044 in js_Atomize (cx=0x8ed2680, bytes=0x28eec0d8 "global_for_XPCJSContextStack_SafeJSContext", length=42, flags=0)
    at jsatom.c:735
        chars = (jschar *) 0x8608c20
        str = Variable "str" is not available.
#5  0x2810393c in js_GetClassId (cx=0x8ed2680, clasp=0x28ef27c0, idp=0xbfbfdcf8) at jsobj.c:2379
#6  0x28109628 in js_NewObject (cx=0x8ed2680, clasp=0x28ef27c0, proto=0x0, parent=0x0) at jsobj.c:2401
#7  0x280a8449 in JS_NewObject (cx=0x8ed2680, clasp=0x28ef27c0, proto=0x0, parent=0x0) at jsapi.c:2393
#8  0x28eb9ba7 in XPCJSContextStack::GetSafeJSContext (this=0x81ac150, aSafeJSContext=0xbfbfde18) at xpcthreadcontext.cpp:180
#9  0x28e94327 in XPCCallContext (this=0xbfbfddfc, callerLanguage=XPCContext::LANG_NATIVE, cx=0x0, obj=0x0, funobj=0x0, name=0,
    argc=4294967295, argv=0x0, rval=0x0) at xpccallcontext.cpp:93
#10 0x28ec0ce5 in nsXPCWrappedJSClass::DelegatedQueryInterface (this=0xb123f40, self=0xb123fa0, aIID=@0x2909beb4,
    aInstancePtr=0xbfbfdf18) at xpcwrappedjsclass.cpp:544
#11 0x28ebb9a5 in nsXPCWrappedJS::QueryInterface (this=0xb123fa0, aIID=@0x2909beb4, aInstancePtr=0xbfbfdf18) at xpcwrappedjs.cpp:106
#12 0x28190731 in nsQueryInterface::operator() (this=Variable "this" is not available.
) at nsCOMPtr.cpp:47
#13 0x2909af0d in nsCOMPtr<jsdIScriptHook>::assign_from_qi (this=0xbfbfdf44, qi={mRawPtr = 0xb123fa0}, aIID=@0x2909beb4)
    at nsCOMPtr.h:1232
#14 0x29098676 in jsds_ScriptHookProc (jsdc=0x8188050, jsdscript=0xa07acc0, creating=0, callerdata=0x0) at nsCOMPtr.h:645
#15 0x2908dabb in jsd_DestroyScriptHookProc (cx=0x818b300, script=0x9e2f500, callerdata=0x8188050) at jsd_scpt.c:628
#16 0x2813499b in js_CallDestroyScriptHook (cx=0x818b300, script=0x9e2f500) at jsscript.c:1458
#17 0x281355c4 in js_DestroyScript (cx=0x818b300, script=0x9e2f500) at jsscript.c:1464
#18 0x280d9765 in fun_finalize (cx=0x818b300, obj=0xea04e38) at jsfun.c:1211
#19 0x2810887c in js_FinalizeObject (cx=0x818b300, obj=0xea04e38) at jsobj.c:2762
#20 0x280e021f in js_GC (cx=0x818b300, gckind=GC_NORMAL) at jsgc.c:3031
#21 0x280ac405 in JS_GC (cx=0x818b300) at jsapi.c:1873
#22 0x2995443c in nsJSContext::Notify (this=0x8652300, timer=0xac081c0) at nsJSEnvironment.cpp:2227
#23 0x282006e6 in nsTimerImpl::Fire (this=0xac081c0) at nsTimerImpl.cpp:397
#24 0x2820197f in handleTimerEvent (event=0x9d13a30) at nsTimerImpl.cpp:459
#25 0x281f94b4 in PL_HandleEvent (self=0x9d13a30) at plevent.c:688
#26 0x281f984c in PL_ProcessPendingEvents (self=0x81e5b80) at plevent.c:623
#27 0x281fcce6 in nsEventQueueImpl::ProcessPendingEvents (this=0x81e5b50) at nsEventQueue.cpp:448
#28 0x290ea845 in event_processor_callback (source=0x85effc0, condition=G_IO_IN, data=0x81e5b50) at nsAppShell.cpp:67
#29 0x28a8af29 in g_io_channel_unix_get_fd () from /usr/local/lib/
#30 0x28a5968c in g_main_context_dispatch () from /usr/local/lib/
#31 0x28a5c6d2 in g_main_context_check () from /usr/local/lib/
#32 0x28a5c995 in g_main_loop_run () from /usr/local/lib/
#33 0x2841d294 in gtk_main () from /usr/local/lib/
#34 0x290eac05 in nsAppShell::Run (this=0x82b6c80) at nsAppShell.cpp:139
#35 0x291b24f8 in nsAppStartup::Run (this=0x82b7af0) at nsAppStartup.cpp:151
#36 0x08054476 in XRE_main (argc=1, argv=0xbfbfe794, aAppData=0x8069720) at nsAppRunner.cpp:2817
#37 0x0804aff1 in main (argc=100159, argv=0x0) at nsBrowserApp.cpp:61

Reproducible: Always
Comment 1 timeless 2009-05-27 19:47:48 PDT
Created attachment 380028 [details] [diff] [review]
only grab hook if we need it

right, so the js we're running isn't obvious, 

It's listed as coming from this:
nsCOMPtr<jsdIScriptHook>::assign_from_qi, thankfully there's only one of those in this function. The script hook of course is written in JS.

702 jsds_ScriptHookProc (JSDContext* jsdc, JSDScript* jsdscript, JSBool creating,
703                      void* callerdata)
704 {

710     nsCOMPtr<jsdIScriptHook> hook;
711     gJsds->GetScriptHook (getter_AddRefs(hook));
Comment 2 Brendan Eich [:brendan] 2009-05-27 21:12:00 PDT
Comment on attachment 380028 [details] [diff] [review]
only grab hook if we need it

That was easy. Good fixing there!

Comment 3 Brendan Eich [:brendan] 2009-05-27 21:12:48 PDT
Isn't this a problem on trunk and 1.9.1 branch? Please nominate if so.

Comment 4 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-05-28 05:54:41 PDT
A checkin message that describes the change ("only get script hook if we're going to call it, to avoid possible calls back into script under JS_GC's notification", perhaps) rather than the state that, post-patch, no longer exists would probably be more useful.
Comment 5 timeless 2009-06-01 02:34:40 PDT
brendan: yes, it is, the bug is ancient and should go to all live branches.

shaver: yes, thanks.
Comment 6 timeless 2009-06-01 03:00:47 PDT
Created attachment 380799 [details] [diff] [review]
updated comment

asking for approval for all branches (and someone should push when approved)
Comment 7 Mike Shaver (:shaver -- probably not reading bugmail closely) 2009-06-02 15:33:03 PDT
Comment on attachment 380799 [details] [diff] [review]
updated comment

Comment 8 Samuel Sidler (old account; do not CC) 2009-06-03 16:28:40 PDT
This needs to land somewhere and bake before we'll take it for 1.9.0.
Comment 9 Boris Zbarsky [:bz] 2009-06-04 18:23:28 PDT
Comment 10 Boris Zbarsky [:bz] 2009-06-04 19:56:00 PDT
Comment 11 Daniel Veditz [:dveditz] 2009-06-10 15:43:49 PDT
Comment on attachment 380799 [details] [diff] [review]
updated comment

Approved for, a=dveditz for release-drivers
Comment 12 Daniel Veditz [:dveditz] 2009-06-10 15:44:59 PDT
Comment on attachment 380799 [details] [diff] [review]
updated comment

Approved for, a=dveditz for release-drivers
Comment 13 Daniel Veditz [:dveditz] 2009-06-25 14:14:31 PDT
Checking in js/jsd/jsd_xpc.cpp;
/cvsroot/mozilla/js/jsd/jsd_xpc.cpp,v  <--  jsd_xpc.cpp
new revision: 1.91; previous revision: 1.90
Comment 14 Al Billings [:abillings] 2009-06-30 17:09:22 PDT
I assume that there is no real method to verify this fix?
Comment 15 Mark Banner (:standard8) 2010-02-05 03:07:53 PST
Checked for
Checking in js/jsd/jsd_xpc.cpp;
/cvsroot/mozilla/js/jsd/jsd_xpc.cpp,v  <--  jsd_xpc.cpp
new revision:; previous revision:
Comment 16 Al Billings [:abillings] 2010-02-19 15:40:00 PST
(In reply to comment #14)
> I assume that there is no real method to verify this fix?

And now, almost eight months later, I ask again. Mark?

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