JS and XPC shells need GC rooting love

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
RESOLVED WORKSFORME
13 years ago
6 years ago

People

(Reporter: timeless, Unassigned)

Tracking

({crash})

Trunk
x86
Windows XP
crash
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patchlove][has draft patch])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
cvs update. apply patch for JS_RUNTIME_SIZE. build.
set JS_RUNTIME_SIZE=10

\mozilla\dbg-i686-pc-cygwin\dist\bin\xpcshell.exe

steps:
1. for (a in Components.interfaces);
expected actual results:
typein:1: out of memory
typein:1: out of memory
typein:1: out of memory
2. Components.interfaces[a]
actual result: crash

i set breakpoints in nsJSIID::Release and XPCWrappedNative::Release and
collected records that look like this:
+	this	0x00c7fcf8 {mRefCnt={mValue=0x00000001 }
_mOwningThread={mThread=0x00385040 } mInfo={mRawPtr=0x015fb378
{mRefCnt={mValue=0x00000002 } _mOwningThread={mThread=0x00385040 }
mEntry=0x00bb84a8 {mIID={...} mTypelib={...} mInterface=0x00c7b190 ...} ...} }
}	nsJSIID * const
+
{,,xpcom_core.dll}(*((*(xptiInterfaceInfo*){*}(((*this).mInfo).mRawPtr)).mEntry)).mName
0x00bb84c1 "nsICache"	char [1]
+	this	0x00c7fd40 {mRefCnt={mValue=0x00000001 }
_mOwningThread={mThread=0x00385040 } mMaybeScope=0x00c7b7d0 {gScopes=0x00bb51c8
{gScopes=0x00bb51c8 {gScopes=0x00bb51c8 gDyingScopes=0x00000000
mRuntime=0x00bb4ab0 ...} gDyingScopes=0x00000000 {gScopes=0x00bb51c8
gDyingScopes=0x00000000 mRuntime=??? ...} mRuntime=0x00bb4ab0
{DEBUG_WrappedNativeHashtable=0x00baacf8 mStrings=0x00d12c7c mStrIDs=0x00bb4ab4
...} ...} gDyingScopes=0x00000000 {gScopes=0x00bb51c8 {gScopes=0x00bb51c8
gDyingScopes=0x00000000 mRuntime=0x00bb4ab0 ...} gDyingScopes=0x00000000
{gScopes=0x00bb51c8 gDyingScopes=0x00000000 mRuntime=??? ...} mRuntime=??? ...}
mRuntime=0x00000000 {DEBUG_WrappedNativeHashtable=??? mStrings=0x00d12c7c char
const * * XPCJSRuntime::mStrings mStrIDs=0x00000004 ...} ...} ...}
XPCWrappedNative * const

a = "nsICache", so the fact that it was destroyed doesn't at all surprise me
(and shouldn't surprise anyone reading this bug).

the stack trace for such destructions is (note that i didn't update the stack
for each object, so you should kinda ignore the object pointers, this was from
the first time i hit my breakpoint):
>	xpc3250.dll!nsJSIID::~nsJSIID()  Line 404	C++
 	xpc3250.dll!nsJSIID::`scalar deleting destructor'()  + 0xf	C++
 	xpc3250.dll!nsJSIID::Release()  Line 385 + 0x91	C++
 	xpc3250.dll!XPCWrappedNative::~XPCWrappedNative()  Line 563 + 0x12	C++
 	xpc3250.dll!XPCWrappedNative::`scalar deleting destructor'()  + 0xf	C++
 	xpc3250.dll!XPCWrappedNative::Release()  Line 793 + 0x91	C++
 	xpc3250.dll!XPCWrappedNative::FlatJSObjectFinalized(JSContext * cx=0x00c6ae50,
JSObject * obj=0x00ba7710)  Line 916	C++
 	xpc3250.dll!XPC_WN_NoHelper_Finalize(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00ba7710)  Line 670	C++
 	js3250.dll!js_FinalizeObject(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00ba7710)  Line 2069 + 0x60	C
 	js3250.dll!js_GC(JSContext * cx=0x00c6ae50, unsigned int gcflags=0x00000005) 
Line 1836 + 0xb	C
 	js3250.dll!js_NewGCThing(JSContext * cx=0x00c6ae50, unsigned int
flags=0x00000001, unsigned int nbytes=0x00000008)  Line 637 + 0xb	C
 	js3250.dll!js_NewString(JSContext * cx=0x00c6ae50, unsigned short *
chars=0x015fa7a0, unsigned int length=0x0000000b, unsigned int
gcflag=0x00000000)  Line 2463 + 0x12	C
 	js3250.dll!js_NewStringCopyN(JSContext * cx=0x00c6ae50, const unsigned short *
s=0x0012e7ac, unsigned int n=0x0000000b, unsigned int gcflag=0x00000000)  Line
2578 + 0x15	C
 	js3250.dll!js_AtomizeString(JSContext * cx=0x00c6ae50, JSString *
str=0x0012e7a0, unsigned int flags=0x00000082)  Line 647 + 0x40	C
 	js3250.dll!js_Atomize(JSContext * cx=0x00c6ae50, const char *
bytes=0x00c83b70, unsigned int length=0x0000000b, unsigned int flags=0x00000002)
 Line 717 + 0x16	C
 	js3250.dll!JS_InternString(JSContext * cx=0x00c6ae50, const char *
s=0x00c83b70)  Line 3962 + 0x1c	C
 	xpc3250.dll!XPCNativeInterface::NewInstance(XPCCallContext & ccx={...},
nsIInterfaceInfo * aInfo=0x015fb378)  Line 413 + 0x1b	C++
 	xpc3250.dll!XPCNativeInterface::GetNewOrUsed(XPCCallContext & ccx={...}, const
nsID * iid=0x00bb84a8)  Line 206 + 0x12	C++
 	xpc3250.dll!nsJSIID::NewResolve(nsIXPConnectWrappedNative *
wrapper=0x00c7fd40, JSContext * cx=0x00c6ae50, JSObject * obj=0x00c88980, long
id=0x00ba5974, unsigned int flags=0x00000000, JSObject * * objp=0x0012eb80, int
* _retval=0x0012eb04)  Line 499 + 0x10	C++
 	xpc3250.dll!XPC_WN_Helper_NewResolve(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long idval=0x00ba5974, unsigned int flags=0x00000000, JSObject *
* objp=0x0012ec00)  Line 951 + 0x45	C++
 	js3250.dll!js_LookupPropertyWithFlags(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, unsigned int flags=0x00000000, JSObject * *
objp=0x0012ecd0, JSProperty * * propp=0x0012ecc0)  Line 2524 + 0x4c	C
 	js3250.dll!js_LookupProperty(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, JSObject * * objp=0x0012ecd0, JSProperty * *
propp=0x0012ecc0)  Line 2429 + 0x1b	C
 	js3250.dll!js_GetProperty(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, long * vp=0x0012ecf4)  Line 2714 + 0x19	C
 	js3250.dll!js_TryMethod(JSContext * cx=0x00c6ae50, JSObject * obj=0x00c88980,
JSAtom * atom=0x00bff5f0, unsigned int argc=0x00000000, long * argv=0x00000000,
long * rval=0x0012ed5c)  Line 3749 + 0x1b	C
 	js3250.dll!js_DefaultValue(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, JSType hint=JSTYPE_STRING, long * vp=0x0012ed98)  Line 3152 + 0x22	C
 	js3250.dll!js_ValueToString(JSContext * cx=0x00c6ae50, long v=0x00c88980) 
Line 2702 + 0x19	C
 	js3250.dll!JS_ValueToString(JSContext * cx=0x00c6ae50, long v=0x00c88980) 
Line 550 + 0xd	C
 	xpcshell.exe!ProcessFile(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
const char * filename=0x00000000, _iobuf * file=0x1027c838)  Line 650 + 0xe	C++
 	xpcshell.exe!Process(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
const char * filename=0x00000000)  Line 699 + 0x15	C++
 	xpcshell.exe!ProcessArgs(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
char * * argv=0x00387054, int argc=0x00000000)  Line 827 + 0x11	C++
 	xpcshell.exe!main(int argc=0x00000000, char * * argv=0x00387054, char * *
envp=0x00382f98)  Line 1639 + 0x15	C++
 	xpcshell.exe!mainCRTStartup()  Line 398 + 0x11	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23	

when death happens,
	(void*)this	0xdddddddd	void *
and up a few frames:
	(void*)wrapper	0x00c7fd40	void *
which we know is dead because I recorded its death (the fields are all 0xdddddddd)

stack:
 	xpc3250.dll!XPCNativeSet::FindMember(long name=0x00ba5974, XPCNativeMember * *
pMember=0x0012ea78, unsigned short * pInterfaceIndex=0x0012ea58)  Line 390 + 0x3	C++
 	xpc3250.dll!XPCNativeSet::FindMember(long name=0x00ba5974, XPCNativeMember * *
pMember=0x0012ea78, XPCNativeInterface * * pInterface=0x0012ea80)  Line 428 +
0x14	C++
 	xpc3250.dll!XPCNativeSet::FindMember(long name=0x00ba5974, XPCNativeMember * *
pMember=0x0012ead0, XPCNativeInterface * * pInterface=0x0012eadc, XPCNativeSet *
protoSet=0x00000000, int * pIsLocal=0x0012ead8)  Line 445 + 0x14	C++
>	xpc3250.dll!XPC_WN_Helper_NewResolve(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long idval=0x00ba5974, unsigned int flags=0x00000000, JSObject *
* objp=0x0012ec00)  Line 982 + 0x2b	C++
 	js3250.dll!js_LookupPropertyWithFlags(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, unsigned int flags=0x00000000, JSObject * *
objp=0x0012ecd0, JSProperty * * propp=0x0012ecc0)  Line 2524 + 0x4c	C
 	js3250.dll!js_LookupProperty(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, JSObject * * objp=0x0012ecd0, JSProperty * *
propp=0x0012ecc0)  Line 2429 + 0x1b	C
 	js3250.dll!js_GetProperty(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, long * vp=0x0012ecf4)  Line 2714 + 0x19	C
 	js3250.dll!js_TryMethod(JSContext * cx=0x00c6ae50, JSObject * obj=0x00c88980,
JSAtom * atom=0x00bff5f0, unsigned int argc=0x00000000, long * argv=0x00000000,
long * rval=0x0012ed5c)  Line 3749 + 0x1b	C
 	js3250.dll!js_DefaultValue(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, JSType hint=JSTYPE_STRING, long * vp=0x0012ed98)  Line 3152 + 0x22	C
 	js3250.dll!js_ValueToString(JSContext * cx=0x00c6ae50, long v=0x00c88980) 
Line 2702 + 0x19	C
 	js3250.dll!JS_ValueToString(JSContext * cx=0x00c6ae50, long v=0x00c88980) 
Line 550 + 0xd	C
 	xpcshell.exe!ProcessFile(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
const char * filename=0x00000000, _iobuf * file=0x1027c838)  Line 650 + 0xe	C++
 	xpcshell.exe!Process(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
const char * filename=0x00000000)  Line 699 + 0x15	C++
 	xpcshell.exe!ProcessArgs(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
char * * argv=0x00387054, int argc=0x00000000)  Line 827 + 0x11	C++
 	xpcshell.exe!main(int argc=0x00000000, char * * argv=0x00387054, char * *
envp=0x00382f98)  Line 1639 + 0x15	C++
 	xpcshell.exe!mainCRTStartup()  Line 398 + 0x11	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23	

reproducable? yes.
(Reporter)

Comment 1

13 years ago
further analysis from slightly before death (death is imminent)

the nsJSIID in this stack is dead (it was the same thing that was garbage
collected ...):
+	this->mRefCnt	{mValue=0xdddddddd }	nsAutoRefCnt
And, the corresponding JSObject that was passed to XPC_WN_Helper_NewResolve is
also dead:
-	obj	0x00c88980 {map=0x00c88988 {nrefs=0x00c88998 ops=0x00c884c9
{newObjectMap=0x01200120 destroyObjectMap=0x01200120 lookupProperty=0x01200120
...} nslots=0x00000010 ...} slots=0x00c884c8 }	JSObject *
\-	map	0x00c88988 {nrefs=0x00c88998 ops=0x00c884c9 {newObjectMap=0x01200120
destroyObjectMap=0x01200120 lookupProperty=0x01200120 ...} nslots=0x00000010
...}	JSObjectMap *
 |	nrefs	0x00c88998	long
 |-	ops	0x00c884c9 {newObjectMap=0x01200120 destroyObjectMap=0x01200120
lookupProperty=0x01200120 ...}	JSObjectOps *
 ||	newObjectMap	0x01200120	JSObjectMap * (JSContext *, long, JSObjectOps *,
JSClass *, JSObject *)*
 ||	destroyObjectMap	0x01200120	void (JSContext *, JSObjectMap *)*
 ||	lookupProperty	0x01200120	int (JSContext *, JSObject *, long, JSObject * *,
JSProperty * *)*
 ||	defineProperty	0x01200120	int (JSContext *, JSObject *, long, long, int
(JSContext *, JSObject *, long, long *)*, int (JSContext *, JSObject *, long,
long *)*, unsigned int, JSProperty * *)*
 ||	getProperty	0x01200120	int (JSContext *, JSObject *, long, long *)*
 ||	setProperty	0x01200120	int (JSContext *, JSObject *, long, long *)*
 ||	getAttributes	0x01200120	int (JSContext *, JSObject *, long, JSProperty *,
unsigned int *)*
 ||	setAttributes	0x01200120	int (JSContext *, JSObject *, long, JSProperty *,
unsigned int *)*
 ||	deleteProperty	0x01200120	int (JSContext *, JSObject *, long, long *)*
 ||	defaultValue	0x01200120	int (JSContext *, JSObject *, JSType, long *)*
 ||	enumerate	0x01200120	int (JSContext *, JSObject *, JSIterateOp, long *, long *)*
 ||	checkAccess	0x01200120	int (JSContext *, JSObject *, long, JSAccessMode,
long *, unsigned int *)*
 ||	thisObject	0x01200120	JSObject * (JSContext *, JSObject *)*
 ||	dropProperty	0x01200120	void (JSContext *, JSObject *, JSProperty *)*
 ||	call	0x01200120	int (JSContext *, JSObject *, unsigned int, long *, long *)*
 ||	construct	0x00200120	int (JSContext *, JSObject *, unsigned int, long *,
long *)*
 ||	xdrObject	0x00000000	int (JSXDRState *, JSObject * *)*
 ||	hasInstance	0x00000000	int (JSContext *, JSObject *, long, int *)*
 ||	setProto	0x00000000	int (JSContext *, JSObject *, unsigned long, JSObject *)*
 ||	setParent	0x00000000	int (JSContext *, JSObject *, unsigned long, JSObject *)*
 ||	mark	0x00000000	unsigned long (JSContext *, JSObject *, void *)*
 ||	clear	0x00000000	void (JSContext *, JSObject *)*
 ||	getRequiredSlot	0x00000000	long (JSContext *, JSObject *, unsigned long)*
 |\	setRequiredSlot	0x00000000	int (JSContext *, JSObject *, unsigned long, long)*
 |	nslots	0x00000010	unsigned long
 |\	freeslot	0x00c7e980	unsigned long
 \-	slots	0x00c884c8	long *
  \		0x20012020	long

at this point, i'm fairly comfortable blaming spidermonkey instead of xpconnect,
because it's the responsibility of js_LookupPropertyWithFlags and friends to
keep the objects they pass down alive, and they failed.


>	xpc3250.dll!nsJSIID::NewResolve(nsIXPConnectWrappedNative *
wrapper=0x00c7fd40, JSContext * cx=0x00c6ae50, JSObject * obj=0x00c88980, long
id=0x00ba5974, unsigned int flags=0x00000000, JSObject * * objp=0x0012eb80, int
* _retval=0x0012eb04)  Line 501	C++
 	xpc3250.dll!XPC_WN_Helper_NewResolve(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long idval=0x00ba5974, unsigned int flags=0x00000000, JSObject *
* objp=0x0012ec00)  Line 951 + 0x45	C++
 	js3250.dll!js_LookupPropertyWithFlags(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, unsigned int flags=0x00000000, JSObject * *
objp=0x0012ecd0, JSProperty * * propp=0x0012ecc0)  Line 2524 + 0x4c	C
 	js3250.dll!js_LookupProperty(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, JSObject * * objp=0x0012ecd0, JSProperty * *
propp=0x0012ecc0)  Line 2429 + 0x1b	C
 	js3250.dll!js_GetProperty(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, long id=0x00bff5f0, long * vp=0x0012ecf4)  Line 2714 + 0x19	C
 	js3250.dll!js_TryMethod(JSContext * cx=0x00c6ae50, JSObject * obj=0x00c88980,
JSAtom * atom=0x00bff5f0, unsigned int argc=0x00000000, long * argv=0x00000000,
long * rval=0x0012ed5c)  Line 3749 + 0x1b	C
 	js3250.dll!js_DefaultValue(JSContext * cx=0x00c6ae50, JSObject *
obj=0x00c88980, JSType hint=JSTYPE_STRING, long * vp=0x0012ed98)  Line 3152 + 0x22	C
 	js3250.dll!js_ValueToString(JSContext * cx=0x00c6ae50, long v=0x00c88980) 
Line 2702 + 0x19	C
 	js3250.dll!JS_ValueToString(JSContext * cx=0x00c6ae50, long v=0x00c88980) 
Line 550 + 0xd	C
 	xpcshell.exe!ProcessFile(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
const char * filename=0x00000000, _iobuf * file=0x1027c838)  Line 650 + 0xe	C++
 	xpcshell.exe!Process(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
const char * filename=0x00000000)  Line 699 + 0x15	C++
 	xpcshell.exe!ProcessArgs(JSContext * cx=0x00c6ae50, JSObject * obj=0x00ba7780,
char * * argv=0x00387054, int argc=0x00000000)  Line 827 + 0x11	C++
 	xpcshell.exe!main(int argc=0x00000000, char * * argv=0x00387054, char * *
envp=0x00382f98)  Line 1639 + 0x15	C++
 	xpcshell.exe!mainCRTStartup()  Line 398 + 0x11	C
 	kernel32.dll!_BaseProcessStart@4()  + 0x23	
Assignee: dbradley → general
Component: XPConnect → JavaScript Engine
QA Contact: pschwartau → general
> at this point, i'm fairly comfortable blaming spidermonkey instead of xpconnect,
> because it's the responsibility of js_LookupPropertyWithFlags and friends to
> keep the objects they pass down alive, and they failed.

Wrong (and stop feeling fairly comfortable about a bogus diagnosis).

All input parameters, and gc-things reachable from them by definition, to JS
APIs and object ops, must be live gc-things protected by callers.  Callees do
not have to root what they do not create themselves.  js_LookupPropertyWithFlags
does not itself create gc-things.  If a resolve hook does, it's up to that
JSClass implementation to protect.

/be
Assignee: general → dbradley
Component: JavaScript Engine → XPConnect
(Reporter)

Comment 3

13 years ago
continuing further along the call stack toward ProcessFile, the same dead object
is present in the js_ValueToString frame, so this is probably a problem in
ProcessFile or JS_ValueToString.

given that the relevant fragment from ProcessFile is the same as equivalent code
in jsshell (xpcshell is supposed to, and for the most part does) mirror jsshell.
Assignee: dbradley → general
Component: XPConnect → JavaScript Engine
So per comment 2, it's obvious that Process (js.c) and ProcessFile
(xpcshell.cpp) need to protect their jsval result locals.  This bug could be
used to fix both, since xpcshell.cpp is based on js.c.  This is not a critical bug.

/be
Severity: critical → normal
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 5

13 years ago
Created attachment 181592 [details] [diff] [review]
patch

yeah, i eventually figured that out.
Assignee: general → timeless
Status: NEW → ASSIGNED
Attachment #181592 - Flags: review?(brendan)
Comment on attachment 181592 [details] [diff] [review]
patch

>+                if (ok) {
>+                    str = JS_ValueToString(cx, result);
>+

Feel free to crunch here.  A blank line may make it breathe more, but right
below, there's no blank line after the if-else, before the JS_RemoveRoot.  I
say minimize such inconsistent changes to this old code.

>+                    if (str)
>+                        fprintf(gOutFile, "%s\n", JS_GetStringBytes(str));
>+                    else
>+                        ok = JS_FALSE;
>+                    JS_RemoveRoot(cx, &result);
>+    {
>+        JSBool ok =
>+        JS_DefineProperty(cx, obj, "arguments", OBJECT_TO_JSVAL(argsObj),
>+                           NULL, NULL, 0);

Nothing lines up right here, and the block scope for ok is unusual in light of
prevailing style.

>+        JS_LeaveLocalRootScope(cx);
>+        if (!ok)
>+            return 1;
>     }
> 
>     length = argc - i;
>+++ xpconnect/shell/xpcshell.cpp	23 Apr 2005 00:22:00 -0000
>@@ -201,6 +210,10 @@ Dump(JSContext *cx, JSObject *obj, uintN
> 
>     char *bytes = JS_GetStringBytes(str);
>     bytes = strdup(bytes);
>+    if (!bytes) {
>+        JS_ReportOutOfMemory(cx);
>+        return JS_FALSE;
>+    }

Why not use JS_strdup instead?

/be
Flags: testcase?

Comment 7

13 years ago
Created attachment 194342 [details] [diff] [review]
address comments

Taking over from timeless.
Assignee: timeless → b.jacques
Attachment #181592 - Attachment is obsolete: true
Attachment #194342 - Flags: review?(brendan)

Updated

13 years ago
Attachment #181592 - Flags: review?(brendan)
(Reporter)

Comment 8

13 years ago
Comment on attachment 194342 [details] [diff] [review]
address comments

+    bytes = JS_strdup(cx, bytes);

no need for:
JS_ReportOutOfMemory(cx);

JS_strdup does it for you :)
Attachment #194342 - Flags: review?(brendan) → review-

Comment 9

13 years ago
Created attachment 194366 [details] [diff] [review]
address comments, v2
Attachment #194342 - Attachment is obsolete: true
Attachment #194366 - Flags: review?(brendan)
Comment on attachment 194366 [details] [diff] [review]
address comments, v2

>@@ -335,19 +339,26 @@ ProcessArgs(JSContext *cx, JSObject *obj
> 
>     /*
>      * Create arguments early and define it to root it, so it's safe from any
>      * GC calls nested below, and so it is available to -f <file> arguments.
>      */
>     argsObj = JS_NewArrayObject(cx, 0, NULL);
>     if (!argsObj)
>         return 1;
>-    if (!JS_DefineProperty(cx, obj, "arguments", OBJECT_TO_JSVAL(argsObj),
>-                           NULL, NULL, 0)) {
>+
>+    if (!JS_EnterLocalRootScope(cx))
>         return 1;
>+
>+    {
>+        JSBool ok = JS_DefineProperty(cx, obj, "arguments", OBJECT_TO_JSVAL(argsObj),
>+                                      NULL, NULL, 0);
>+        JS_LeaveLocalRootScope(cx);
>+        if (!ok)
>+            return 1;
>     }

This is no longer necessary, nor should it be for any single API call --
JS_DefineProperty protects the value being set by storing it in the property's
slot before calling any addProperty hook that might run a GC.

The same principle should save us from rooting elsewhere in this patch.  I'll
make sure that's the case and patch this bug as needed.

/be
Attachment #194366 - Flags: review?(brendan) → review-
Taking.

/be
Assignee: b.jacques → brendan
Status: ASSIGNED → NEW

Updated

12 years ago
Summary: crash [@ XPCNativeSet::FindMember] → JS and XPC shells need GC rooting love
I am a poor owner for this bug -- Igor, would you be willing to take it?

/be

Updated

11 years ago
Assignee: brendan → igor

Updated

11 years ago
Assignee: igor → general

Updated

9 years ago
Whiteboard: [patchlove][has draft patch]

Comment 13

8 years ago
Should this be closed?  

From reading timeless' patch, I think the conservative GC will take care of those problems.

Comment 14

6 years ago
All this has been changed heavily, presuming WFM.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.