From ae74a2fe3c949ef653f2f59ae0582bc2e68b4b7a Mon Sep 17 00:00:00 2001 From: Igor Bukanov Date: Thu, 29 Dec 2011 22:22:21 +0100 Subject: [PATCH] bug 714066 - Missed FreeChunkList call in JSRuntime::onOutOfMemory. r=wmccloskey --- js/src/jscntxt.cpp | 30 ++------------------ js/src/jscntxt.h | 6 ---- js/src/jsgc.cpp | 71 +++++++++++++++++++++------------------------- js/src/jsgc.h | 3 ++ 4 files changed, 37 insertions(+), 73 deletions(-) diff --git a/js/src/jscntxt.cpp b/js/src/jscntxt.cpp index cc10655ab5c..1b901cb3d00 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), @@ -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); 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 84a9f10ce6d..acdadd9dd79 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) { @@ -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 - * 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; } @@ -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 - * 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 d4bfa3a09aa..72fdc4db766 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); };