bug 505315 - constructing GC free lists during finalization. r=brendan

js/src/jsapi.cpp

@@ -4867,7 +4867,7 @@ JS_CompileUCFunctionForPrincipals(JSContext *cx, JSObject *obj,
 #endif
 
   out:
-    cx->weakRoots.newbornObject = FUN_OBJECT(fun);
+    cx->weakRoots.finalizableNewborns[FINALIZE_FUNCTION] = fun;
     JS_POP_TEMP_ROOT(cx, &tvr);
 
   out2:

js/src/jsarray.cpp

@@ -3463,7 +3463,7 @@ js_NewArrayObject(JSContext *cx, jsuint length, jsval *vector, JSBool holey)
     JS_POP_TEMP_ROOT(cx, &tvr);
 
     /* Set/clear newborn root, in case we lost it.  */
-    cx->weakRoots.newbornObject = obj;
+    cx->weakRoots.finalizableNewborns[FINALIZE_OBJECT] = obj;
     return obj;
 }
 

js/src/jscntxt.cpp

@@ -93,6 +93,7 @@ FinishThreadData(JSThreadData *data)
 {
 #ifdef DEBUG
     /* All GC-related things must be already removed at this point. */
+    data->gcFreeLists.assertEmpty();
     for (size_t i = 0; i != JS_ARRAY_LENGTH(data->scriptsToGC); ++i)
         JS_ASSERT(!data->scriptsToGC[i]);
     for (size_t i = 0; i != JS_ARRAY_LENGTH(data->nativeEnumCache); ++i)
@@ -109,10 +110,7 @@ FinishThreadData(JSThreadData *data)
 static void
 PurgeThreadData(JSContext *cx, JSThreadData *data)
 {
-    /*
-     * Clear the double free list to release all the pre-allocated doubles.
-     */
-    data->doubleFreeList = NULL;
+    data->gcFreeLists.purge();
 
     js_PurgeGSNCache(&data->gsnCache);
 
@@ -271,15 +269,15 @@ thread_purger(JSDHashTable *table, JSDHashEntryHdr *hdr, uint32 /* index */,
         js_DestroyScriptsToGC(cx, &thread->data);
 
         /*
-         * The following is potentially suboptimal as it also zeros the cache
+         * The following is potentially suboptimal as it also zeros the caches
          * in data, but the code simplicity wins here.
          */
+        thread->data.gcFreeLists.purge();
         js_PurgeCachedNativeEnumerators(cx, &thread->data);
         DestroyThread(thread);
         return JS_DHASH_REMOVE;
     }
     PurgeThreadData(cx, &thread->data);
-    memset(thread->gcFreeLists, 0, sizeof(thread->gcFreeLists));
     return JS_DHASH_NEXT;
 }
 

js/src/jscntxt.h

@@ -289,8 +289,7 @@ typedef struct JSFunctionMeter {
 #endif
 
 struct JSThreadData {
-    /* List of pre-allocated doubles. */
-    JSGCDoubleCell      *doubleFreeList;
+    JSGCFreeLists       gcFreeLists;
 
     /*
      * The GSN cache is per thread since even multi-cx-per-thread embeddings
@@ -366,8 +365,6 @@ struct JSThread {
     /* Indicates that the thread is waiting in ClaimTitle from jslock.cpp. */
     JSTitle             *titleToShare;
 
-    JSGCThing           *gcFreeLists[FINALIZE_LIMIT];
-
     /* Factored out of JSThread for !JS_THREADSAFE embedding in JSRuntime. */
     JSThreadData        data;
 };

js/src/jsgc.h

@@ -126,27 +126,6 @@ typedef struct JSPtrTable {
 extern JSBool
 js_RegisterCloseableIterator(JSContext *cx, JSObject *obj);
 
-/*
- * Allocates a new GC thing of the given size. After a successful allocation
- * the caller must fully initialize the thing before calling any function that
- * can potentially trigger GC. This will ensure that GC tracing never sees junk
- * values stored in the partially initialized thing.
- */
-extern JSObject*
-js_NewGCObject(JSContext *cx);
-
-extern JSString*
-js_NewGCString(JSContext *cx);
-
-extern JSString*
-js_NewGCExternalString(JSContext *cx, uintN type);
-
-extern JSFunction*
-js_NewGCFunction(JSContext *cx);
-
-extern JSXML*
-js_NewGCXML(JSContext *cx);
-
 /*
  * Allocate a new double jsval and store the result in *vp. vp must be a root.
  * The function does not copy the result into any weak root.
@@ -281,28 +260,78 @@ IsFinalizableStringKind(unsigned thingKind)
            thingKind <= unsigned(FINALIZE_EXTERNAL_STRING_LAST);
 }
 
-typedef struct JSGCArenaInfo JSGCArenaInfo;
-typedef struct JSGCArenaList JSGCArenaList;
-typedef struct JSGCChunkInfo JSGCChunkInfo;
+/*
+ * Allocates a new GC thing. After a successful allocation the caller must
+ * fully initialize the thing before calling any function that can potentially
+ * trigger GC. This will ensure that GC tracing never sees junk values stored
+ * in the partially initialized thing.
+ */
+extern void *
+NewFinalizableGCThing(JSContext *cx, unsigned thingKind);
+
+static inline JSObject *
+js_NewGCObject(JSContext *cx)
+{
+    return (JSObject *) NewFinalizableGCThing(cx, FINALIZE_OBJECT);
+}
+
+static inline JSString *
+js_NewGCString(JSContext *cx)
+{
+    return (JSString *) NewFinalizableGCThing(cx, FINALIZE_STRING);
+}
+
+static inline JSString *
+js_NewGCExternalString(JSContext *cx, uintN type)
+{
+    JS_ASSERT(type < JS_EXTERNAL_STRING_LIMIT);
+    return (JSString *) NewFinalizableGCThing(cx,
+                                              FINALIZE_EXTERNAL_STRING0 + type);
+}
+
+static inline JSFunction*
+js_NewGCFunction(JSContext *cx)
+{
+    return (JSFunction *) NewFinalizableGCThing(cx, FINALIZE_FUNCTION);
+}
+
+#if JS_HAS_XML_SUPPORT
+static inline JSXML *
+js_NewGCXML(JSContext *cx)
+{
+    return (JSXML *) NewFinalizableGCThing(cx, FINALIZE_XML);
+}
+#endif
+
+struct JSGCArenaInfo;
+struct JSGCChunkInfo;
 
 struct JSGCArenaList {
-    JSGCArenaInfo   *last;          /* last allocated GC arena */
-    uint32          lastCount;      /* number of allocated things in the last
-                                       arena */
+    JSGCArenaInfo   *head;          /* list start */
+    JSGCArenaInfo   *cursor;        /* arena with free things */
     uint32          thingKind;      /* one of JSFinalizeGCThingKind */
     uint32          thingSize;      /* size of things to allocate on this list
                                      */
-    JSGCThing       *freeList;      /* list of free GC things */
-};
-
-union JSGCDoubleCell {
-    double          number;
-    JSGCDoubleCell  *link;
 };
 
 struct JSGCDoubleArenaList {
-    JSGCArenaInfo   *first;             /* first allocated GC arena */
-    JSGCArenaInfo   *cursor;            /* next arena with free cells */
+    JSGCArenaInfo   *head;          /* list start */
+    JSGCArenaInfo   *cursor;        /* next arena with free cells */
+};
+
+struct JSGCFreeLists {
+    JSGCThing       *doubles;
+    JSGCThing       *finalizables[FINALIZE_LIMIT];
+
+    void purge();
+
+#ifdef DEBUG
+    void assertEmpty() const {
+        JS_ASSERT(!doubles);
+        for (size_t i = 0; i != JS_ARRAY_LENGTH(finalizables); ++i)
+            JS_ASSERT(!finalizables[i]);
+    }
+#endif
 };
 
 extern void
@@ -310,19 +339,16 @@ js_DestroyScriptsToGC(JSContext *cx, JSThreadData *data);
 
 struct JSWeakRoots {
     /* Most recently created things by type, members of the GC's root set. */
-    JSObject        *newbornObject;
+    void            *finalizableNewborns[FINALIZE_LIMIT];
     jsdouble        *newbornDouble;
-    JSString        *newbornString;
-#if JS_HAS_XML_SUPPORT
-    JSXML           *newbornXML;
-#endif
-    JSString        *newbornExternalString[JS_EXTERNAL_STRING_LIMIT];
 
     /* Atom root for the last-looked-up atom on this context. */
     jsval           lastAtom;
 
     /* Root for the result of the most recent js_InternalInvoke call. */
     jsval           lastInternalResult;
+
+    void mark(JSTracer *trc);
 };
 
 #define JS_CLEAR_WEAK_ROOTS(wr) (memset((wr), 0, sizeof(JSWeakRoots)))

js/src/jsobj.cpp

@@ -2247,9 +2247,6 @@ js_NewObjectWithGivenProto(JSContext *cx, JSClass *clasp, JSObject *proto,
         obj->map = const_cast<JSObjectMap *>(ops->objectMap);
     }
 
-    /* Check that the newborn root still holds the object. */
-    JS_ASSERT_IF(!cx->localRootStack, cx->weakRoots.newbornObject == obj);
-
     /*
      * Do not call debug hooks on trace, because we might be in a non-_FAIL
      * builtin. See bug 481444.
@@ -2260,7 +2257,7 @@ js_NewObjectWithGivenProto(JSContext *cx, JSClass *clasp, JSObject *proto,
         cx->debugHooks->objectHook(cx, obj, JS_TRUE,
                                    cx->debugHooks->objectHookData);
         JS_UNKEEP_ATOMS(cx->runtime);
-        cx->weakRoots.newbornObject = obj;
+        cx->weakRoots.finalizableNewborns[FINALIZE_OBJECT] = obj;
     }
 
 out:
@@ -5514,7 +5511,8 @@ js_GetClassPrototype(JSContext *cx, JSObject *scope, jsid id,
              * instance that delegates to this object, or just query the
              * prototype for its class.
              */
-            cx->weakRoots.newbornObject = JSVAL_TO_OBJECT(v);
+            cx->weakRoots.finalizableNewborns[FINALIZE_OBJECT] =
+                JSVAL_TO_OBJECT(v);
         }
     }
     *protop = JSVAL_IS_OBJECT(v) ? JSVAL_TO_OBJECT(v) : NULL;

js/src/jsops.cpp

@@ -3475,7 +3475,7 @@ BEGIN_CASE(JSOP_ENDINIT)
     JS_ASSERT(regs.sp - StackBase(fp) >= 1);
     lval = FETCH_OPND(-1);
     JS_ASSERT(JSVAL_IS_OBJECT(lval));
-    cx->weakRoots.newbornObject = JSVAL_TO_OBJECT(lval);
+    cx->weakRoots.finalizableNewborns[FINALIZE_OBJECT] = JSVAL_TO_OBJECT(lval);
 END_CASE(JSOP_ENDINIT)
 
 BEGIN_CASE(JSOP_INITPROP)

js/src/jstracer.cpp

@@ -2864,7 +2864,7 @@ NativeToValueBase(JSContext* cx, jsval& v, JSTraceType type, double* slot)
          * It's not safe to trigger the GC here, so use an emergency heap if we
          * are out of double boxes.
          */
-            if (JS_THREAD_DATA(cx)->doubleFreeList) {
+            if (JS_THREAD_DATA(cx)->gcFreeLists.doubles) {
 #ifdef DEBUG
             JSBool ok =
 #endif