bug 714066 - Missed FreeChunkList call in JSRuntime::onOutOfMemory. r=wmccloskey

js/src/jscntxt.cpp

@@ -104,7 +104,6 @@ ThreadData::ThreadData(JSRuntime *rt)
 #ifdef JS_THREADSAFE
     requestDepth(0),
 #endif
-    waiveGCQuota(false),
     tempLifoAlloc(TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE),
     execAlloc(NULL),
     bumpAlloc(NULL),
@@ -1270,34 +1269,9 @@ js_InvokeOperationCallback(JSContext *cx)
 #endif
     JS_UNLOCK_GC(rt);
 
-    if (rt->gcIsNeeded) {
+    if (rt->gcIsNeeded)
         js_GC(cx, rt->gcTriggerCompartment, GC_NORMAL, rt->gcTriggerReason);
 
-        /*
-         * On trace we can exceed the GC quota, see comments in NewGCArena. So
-         * we check the quota and report OOM here when we are off trace.
-         */
-        if (checkOutOfMemory(rt)) {
-#ifdef JS_THREADSAFE
-            /*
-            * We have to wait until the background thread is done in order
-            * to get a correct answer.
-            */
-            {
-                AutoLockGC lock(rt);
-                rt->gcHelperThread.waitBackgroundSweepEnd();
-            }
-            if (checkOutOfMemory(rt)) {
-                js_ReportOutOfMemory(cx);
-                return false;
-            }
-#else
-            js_ReportOutOfMemory(cx);
-            return false;
-#endif
-        }
-    }
-
 #ifdef JS_THREADSAFE
     /*
      * We automatically yield the current context every time the operation
@@ -1598,7 +1572,7 @@ JSRuntime::onOutOfMemory(void *p, size_t nbytes, JSContext *cx)
         AutoLockGC lock(this);
         gcHelperThread.waitBackgroundSweepOrAllocEnd();
 #endif
-        gcChunkPool.expire(this, true);
+        gcChunkPool.expireAndFree(this, true);
     }
     if (!p)
         p = OffTheBooks::malloc_(nbytes);

js/src/jscntxt.h

@@ -140,12 +140,6 @@ struct ThreadData {
     /* Keeper of the contiguous stack used by all contexts in this thread. */
     StackSpace          stackSpace;
 
-    /*
-     * Flag indicating that we are waiving any soft limits on the GC heap
-     * because we want allocations to be infallible (except when we hit OOM).
-     */
-    bool                waiveGCQuota;
-
     /* Temporary arena pool used while compiling and decompiling. */
     static const size_t TEMP_LIFO_ALLOC_PRIMARY_CHUNK_SIZE = 1 << 12;
     LifoAlloc           tempLifoAlloc;

js/src/jsgc.cpp

@@ -517,6 +517,22 @@ ChunkPool::expire(JSRuntime *rt, bool releaseAll)
     return freeList;
 }
 
+static void
+FreeChunkList(Chunk *chunkListHead)
+{
+    while (Chunk *chunk = chunkListHead) {
+        JS_ASSERT(!chunk->info.numArenasFreeCommitted);
+        chunkListHead = chunk->info.next;
+        FreeChunk(chunk);
+    }
+}
+
+void
+ChunkPool::expireAndFree(JSRuntime *rt, bool releaseAll)
+{
+    FreeChunkList(expire(rt, releaseAll));
+}
+
 JS_FRIEND_API(int64_t)
 ChunkPool::countCleanDecommittedArenas(JSRuntime *rt)
 {
@@ -553,16 +569,6 @@ Chunk::release(JSRuntime *rt, Chunk *chunk)
     FreeChunk(chunk);
 }
 
-static void
-FreeChunkList(Chunk *chunkListHead)
-{
-    while (Chunk *chunk = chunkListHead) {
-        JS_ASSERT(!chunk->info.numArenasFreeCommitted);
-        chunkListHead = chunk->info.next;
-        FreeChunk(chunk);
-    }
-}
-
 inline void
 Chunk::prepareToBeFreed(JSRuntime *rt)
 {
@@ -1182,7 +1188,7 @@ js_FinishGC(JSRuntime *rt)
      * Finish the pool after the background thread stops in case it was doing
      * the background sweeping.
      */
-    FreeChunkList(rt->gcChunkPool.expire(rt, true));
+    rt->gcChunkPool.expireAndFree(rt, true);
 
 #ifdef DEBUG
     if (!rt->gcRootsHash.empty())
@@ -1641,12 +1647,6 @@ RunLastDitchGC(JSContext *cx)
 #endif
 }
 
-inline bool
-IsGCAllowed(JSContext *cx)
-{
-    return !JS_THREAD_DATA(cx)->waiveGCQuota;
-}
-
 /* static */ void *
 ArenaLists::refillFreeList(JSContext *cx, AllocKind thingKind)
 {
@@ -1658,7 +1658,7 @@ ArenaLists::refillFreeList(JSContext *cx, AllocKind thingKind)
 
     bool runGC = !!rt->gcIsNeeded;
     for (;;) {
-        if (JS_UNLIKELY(runGC) && IsGCAllowed(cx)) {
+        if (JS_UNLIKELY(runGC)) {
             RunLastDitchGC(cx);
 
             /* Report OOM of the GC failed to free enough memory. */
@@ -1679,14 +1679,11 @@ ArenaLists::refillFreeList(JSContext *cx, AllocKind thingKind)
             return thing;
 
         /*
-         * We failed to allocate. Run the GC if we can unless we have done it
+         * We failed to allocate. Run the GC if we haven't done it already.
-         * already. Otherwise report OOM but first schedule a new GC soon.
+         * Otherwise report OOM.
          */
-        if (runGC || !IsGCAllowed(cx)) {
+        if (runGC)
-            AutoLockGC lock(rt);
-            TriggerGC(rt, gcstats::REFILL);
             break;
-        }
         runGC = true;
     }
 
@@ -2993,12 +2990,10 @@ GCCycle(JSContext *cx, JSCompartment *comp, JSGCInvocationKind gckind)
         js_PurgeThreads_PostGlobalSweep(cx);
 
 #ifdef JS_THREADSAFE
-    if (gckind != GC_LAST_CONTEXT && rt->state != JSRTS_LANDING) {
+    if (cx->gcBackgroundFree) {
         JS_ASSERT(cx->gcBackgroundFree == &rt->gcHelperThread);
         cx->gcBackgroundFree = NULL;
         rt->gcHelperThread.startBackgroundSweep(gckind == GC_SHRINK);
-    } else {
-        JS_ASSERT(!cx->gcBackgroundFree);
     }
 #endif
 
@@ -3279,19 +3274,17 @@ void
 RunDebugGC(JSContext *cx)
 {
 #ifdef JS_GC_ZEAL
-    if (IsGCAllowed(cx)) {
+    JSRuntime *rt = cx->runtime;
-        JSRuntime *rt = cx->runtime;
 
-        /*
+    /*
-         * If rt->gcDebugCompartmentGC is true, only GC the current
+     * If rt->gcDebugCompartmentGC is true, only GC the current
-         * compartment. But don't GC the atoms compartment.
+     * compartment. But don't GC the atoms compartment.
-         */
+     */
-        rt->gcTriggerCompartment = rt->gcDebugCompartmentGC ? cx->compartment : NULL;
+    rt->gcTriggerCompartment = rt->gcDebugCompartmentGC ? cx->compartment : NULL;
-        if (rt->gcTriggerCompartment == rt->atomsCompartment)
+    if (rt->gcTriggerCompartment == rt->atomsCompartment)
-            rt->gcTriggerCompartment = NULL;
+        rt->gcTriggerCompartment = NULL;
-
+    
-        RunLastDitchGC(cx);
+    RunLastDitchGC(cx);
-    }
 #endif
 }
 

js/src/jsgc.h

@@ -808,6 +808,9 @@ class ChunkPool {
      */
     Chunk *expire(JSRuntime *rt, bool releaseAll);
 
+    /* Must be called with the GC lock taken. */
+    void expireAndFree(JSRuntime *rt, bool releaseAll);
+
     /* Must be called either during the GC or with the GC lock taken. */
     JS_FRIEND_API(int64_t) countCleanDecommittedArenas(JSRuntime *rt);
 };