From 222e9fbd53ca56c0186568f9db3b0a5b30489fce Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Fri, 30 Dec 2011 00:33:44 +0100 Subject: [PATCH] bug 714066 - Missed FreeChunkList call in JSRuntime::onOutOfMemory. r=wmccloskey --- js/src/jscntxt.cpp | 3 +- js/src/jscntxt.h | 6 ---- js/src/jsgc.cpp | 71 +++++++++++++++++++++------------------------- js/src/jsgc.h | 3 ++ 4 files changed, 36 insertions(+), 47 deletions(-) diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index cc10655ab5c..e1de74165c0 100644 --- a/js/src/jscntxt.cpp +++ b/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), @@ -1598,7 +1597,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); diff --git a/js/src/jscntxt.h b/js/src/jscntxt.h index 7a878701a9e..59d60ce4506 100644 --- a/js/src/jscntxt.h +++ b/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; diff --git a/js/src/jsgc.cpp b/js/src/jsgc.cpp index 4f26bcfd57d..f399808e9fa 100644 --- a/js/src/jsgc.cpp +++ b/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) { @@ -1211,7 +1217,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()) @@ -1670,12 +1676,6 @@ RunLastDitchGC(JSContext *cx) #endif } -inline bool -IsGCAllowed(JSContext *cx) -{ - return !JS_THREAD_DATA(cx)->waiveGCQuota; -} - /* static */ void * ArenaLists::refillFreeList(JSContext *cx, AllocKind thingKind) { @@ -1687,7 +1687,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. */ @@ -1708,14 +1708,11 @@ ArenaLists::refillFreeList(JSContext *cx, AllocKind thingKind) return thing; /* - * We failed to allocate. Run the GC if we can unless we have done it - * already. Otherwise report OOM but first schedule a new GC soon. + * We failed to allocate. Run the GC if we haven't done it already. + * Otherwise report OOM. */ - if (runGC || !IsGCAllowed(cx)) { - AutoLockGC lock(rt); - TriggerGC(rt, gcstats::REFILL); + if (runGC) break; - } runGC = true; } @@ -3022,12 +3019,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 @@ -3308,19 +3303,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 - * compartment. But don't GC the atoms compartment. - */ - rt->gcTriggerCompartment = rt->gcDebugCompartmentGC ? cx->compartment : NULL; - if (rt->gcTriggerCompartment == rt->atomsCompartment) - rt->gcTriggerCompartment = NULL; - - RunLastDitchGC(cx); - } + /* + * If rt->gcDebugCompartmentGC is true, only GC the current + * compartment. But don't GC the atoms compartment. + */ + rt->gcTriggerCompartment = rt->gcDebugCompartmentGC ? cx->compartment : NULL; + if (rt->gcTriggerCompartment == rt->atomsCompartment) + rt->gcTriggerCompartment = NULL; + + RunLastDitchGC(cx); #endif } diff --git a/js/src/jsgc.h b/js/src/jsgc.h index 9f50438106e..5dab399accf 100644 --- a/js/src/jsgc.h +++ b/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); };