Fix leak of JSScript when a JSFunction is collected in a later GC than its function object. This changes GCX_PRIVATE to GCX_FUNCTION, and is essentially the same as the finalization part of the changes from bug 375808 (by igor). b=389757 r=igor a1.9=brendan

js/src/jsfun.c

@@ -1252,30 +1252,18 @@ static void
 fun_finalize(JSContext *cx, JSObject *obj)
 {
     JSFunction *fun;
-    JSScript *script;
 
     /* No valid function object should lack private data, but check anyway. */
     fun = (JSFunction *) JS_GetPrivate(cx, obj);
     if (!fun)
         return;
 
+    /*
+     * This works because obj is finalized before JSFunction. See
+     * comments in js_GC before the finalization loop.
+     */
     if (fun->object == obj)
         fun->object = NULL;
-
-    /*
-     * Null-check of i.script is required since the parser sets interpreted
-     * very early.
-     *
-     * Here js_IsAboutToBeFinalized works because obj is finalized before
-     * JSFunction. See comments in js_GC before the finalization loop.
-     */
-    if (FUN_INTERPRETED(fun) && fun->u.i.script &&
-        js_IsAboutToBeFinalized(cx, fun))
-    {
-        script = fun->u.i.script;
-        fun->u.i.script = NULL;
-        js_DestroyScript(cx, script);
-    }
 }
 
 #if JS_HAS_XDR
@@ -2196,10 +2184,10 @@ js_NewFunction(JSContext *cx, JSObject *funobj, JSNative native, uintN nargs,
     JS_PUSH_SINGLE_TEMP_ROOT(cx, OBJECT_TO_JSVAL(funobj), &tvr);
 
     /*
-     * Allocate fun after allocating funobj so slot allocation in js_NewObject
-     * does not wipe out fun from newborn[GCX_PRIVATE].
+     * Allocate fun after allocating funobj so allocations in js_NewObject
+     * and hooks called from it do not wipe out fun from newborn[GCX_FUNCTION].
      */
-    fun = (JSFunction *) js_NewGCThing(cx, GCX_PRIVATE, sizeof(JSFunction));
+    fun = (JSFunction *) js_NewGCThing(cx, GCX_FUNCTION, sizeof(JSFunction));
     if (!fun)
         goto out;
 
@@ -2224,6 +2212,22 @@ out:
     return fun;
 }
 
+void
+js_FinalizeFunction(JSContext *cx, JSFunction *fun)
+{
+    JSScript *script;
+
+    /*
+     * Null-check of i.script is required since the parser sets interpreted
+     * very early.
+     */
+    if (FUN_INTERPRETED(fun) && fun->u.i.script) {
+        script = fun->u.i.script;
+        fun->u.i.script = NULL;
+        js_DestroyScript(cx, script);
+    }
+}
+
 JSObject *
 js_CloneFunctionObject(JSContext *cx, JSObject *funobj, JSObject *parent)
 {

js/src/jsfun.h

@@ -111,6 +111,9 @@ extern JSFunction *
 js_NewFunction(JSContext *cx, JSObject *funobj, JSNative native, uintN nargs,
                uintN flags, JSObject *parent, JSAtom *atom);
 
+extern void
+js_FinalizeFunction(JSContext *cx, JSFunction *fun);
+
 extern JSObject *
 js_CloneFunctionObject(JSContext *cx, JSObject *funobj, JSObject *parent);
 

js/src/jsgc.c

@@ -248,7 +248,7 @@ static uint8 GCTypeToTraceKindMap[GCX_NTYPES] = {
     JSTRACE_STRING,     /* GCX_STRING */
     JSTRACE_DOUBLE,     /* GCX_DOUBLE */
     JSTRACE_STRING,     /* GCX_MUTABLE_STRING */
-    JSTRACE_FUNCTION,   /* GCX_PRIVATE */
+    JSTRACE_FUNCTION,   /* GCX_FUNCTION */
     JSTRACE_NAMESPACE,  /* GCX_NAMESPACE */
     JSTRACE_QNAME,      /* GCX_QNAME */
     JSTRACE_XML,        /* GCX_XML */
@@ -560,7 +560,7 @@ static GCFinalizeOp gc_finalizers[GCX_NTYPES] = {
     (GCFinalizeOp) js_FinalizeString,           /* GCX_STRING */
     (GCFinalizeOp) js_FinalizeDouble,           /* GCX_DOUBLE */
     (GCFinalizeOp) js_FinalizeString,           /* GCX_MUTABLE_STRING */
-    NULL,                                       /* GCX_PRIVATE */
+    (GCFinalizeOp) js_FinalizeFunction,         /* GCX_FUNCTION */
     (GCFinalizeOp) js_FinalizeXMLNamespace,     /* GCX_NAMESPACE */
     (GCFinalizeOp) js_FinalizeXMLQName,         /* GCX_QNAME */
     (GCFinalizeOp) js_FinalizeXML,              /* GCX_XML */
@@ -582,7 +582,7 @@ static const char *gc_typenames[GCX_NTYPES] = {
     "newborn string",
     "newborn double",
     "newborn mutable string",
-    "newborn private",
+    "newborn function",
     "newborn Namespace",
     "newborn QName",
     "newborn XML",
@@ -2398,8 +2398,8 @@ restart:
      * rather than nest badly and leave the unmarked newborn to be swept.
      *
      * Here we need to ensure that JSObject instances are finalized before GC-
-     * allocated JSFunction instances so fun_finalize from jsfun.c can get the
-     * proper result from the call to js_IsAboutToBeFinalized. For that we
+     * allocated JSFunction instances so fun_finalize from jsfun.c can clear
+     * the weak pointer from the JSFunction back to the JSObject.  For that we
      * simply finalize the list containing JSObject first since the static
      * assert at the beginning of the file guarantees that JSFunction instances
      * are allocated from a different list.

js/src/jsgc.h

@@ -55,7 +55,7 @@ JS_BEGIN_EXTERN_C
 #define GCX_DOUBLE              2               /* jsdouble */
 #define GCX_MUTABLE_STRING      3               /* JSString that's mutable --
                                                    single-threaded only! */
-#define GCX_PRIVATE             4               /* private (unscanned) data */
+#define GCX_FUNCTION            4               /* JSFunction */
 #define GCX_NAMESPACE           5               /* JSXMLNamespace */
 #define GCX_QNAME               6               /* JSXMLQName */
 #define GCX_XML                 7               /* JSXML */

js/src/xpconnect/src/nsXPConnect.cpp

@@ -673,7 +673,7 @@ static uint8 GCTypeToTraceKindMap[GCX_NTYPES] = {
     JSTRACE_STRING,     /* GCX_STRING (unused) */
     JSTRACE_DOUBLE,     /* GCX_DOUBLE (unused) */
     JSTRACE_STRING,     /* GCX_MUTABLE_STRING (unused) */
-    JSTRACE_FUNCTION,   /* GCX_PRIVATE (unused) */
+    JSTRACE_FUNCTION,   /* GCX_FUNCTION (unused) */
     JSTRACE_NAMESPACE,  /* GCX_NAMESPACE */
     JSTRACE_QNAME,      /* GCX_QNAME */
     JSTRACE_XML         /* GCX_XML */