bug 658864 - preventing GC when reporting OOM on other threads. r=anygregor
authorIgor Bukanov <igor@mir2.org>
Sun, 22 May 2011 20:50:08 +0200
changeset 70612 87cb1807aab9a1e8fc775370849434dc0bf7e176
parent 70611 6f4ca81b13d57b4b2e816d485d67ef0cff214692
child 70614 036db2581cd116fd25c410759ef156f2aa7a4ca6
push id20369
push usercleary@mozilla.com
push dateMon, 06 Jun 2011 20:24:54 +0000
treeherdermozilla-central@3589f8cefd83 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanygregor
bugs658864
milestone6.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 658864 - preventing GC when reporting OOM on other threads. r=anygregor
js/src/jscntxt.cpp
js/src/jscntxt.h
js/src/jsgc.cpp
js/src/jslock.h
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -816,17 +816,17 @@ js_ReportOutOfMemory(JSContext *cx)
         JSDebugErrorHook hook = cx->debugHooks->debugErrorHook;
         if (hook &&
             !hook(cx, msg, &report, cx->debugHooks->debugErrorHookData)) {
             onError = NULL;
         }
     }
 
     if (onError) {
-        AutoScopedAssign<bool> ss(&cx->runtime->inOOMReport, true);
+        AutoAtomicIncrement incr(&cx->runtime->inOOMReport);
         onError(cx, msg, &report);
     }
 }
 
 void
 js_ReportOutOfScriptQuota(JSContext *maybecx)
 {
     if (maybecx)
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -705,17 +705,17 @@ struct JSRuntime {
     size_t               mjitDataSize;
 #endif
 
     /*
      * To ensure that cx->malloc does not cause a GC, we set this flag during
      * OOM reporting (in js_ReportOutOfMemory). If a GC is requested while
      * reporting the OOM, we ignore it.
      */
-    bool                 inOOMReport;
+    int32               inOOMReport;
 
 #if defined(MOZ_GCTIMER) || defined(JSGC_TESTPILOT)
     struct GCData {
         /*
          * Timestamp of the first GCTimer -- application runtime is determined
          * relative to this value.
          */
         uint64      firstEnter;
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2618,19 +2618,16 @@ AutoGCSession::~AutoGCSession()
  * GC, repeatedly if necessary, until we think we have not created any new
  * garbage and no other threads are demanding more GC. We disable inlining
  * to ensure that the bottom of the stack with possible GC roots recorded in
  * js_GC excludes any pointers we use during the marking implementation.
  */
 static JS_NEVER_INLINE void
 GCCycle(JSContext *cx, JSCompartment *comp, JSGCInvocationKind gckind  GCTIMER_PARAM)
 {
-    if (JS_ON_TRACE(cx))
-        return;
-
     JSRuntime *rt = cx->runtime;
 
     /*
      * Recursive GC is no-op and a call from another thread waits the started
      * GC cycle to finish.
      */
     if (rt->gcMarkAndSweep) {
 #ifdef JS_THREADSAFE
@@ -2641,16 +2638,27 @@ GCCycle(JSContext *cx, JSCompartment *co
         }
 #endif
         return;
     }
 
     AutoGCSession gcsession(cx);
 
     /*
+     * Don't GC if any thread is reporting an OOM. We check the flag after we
+     * have set up the GC session and know that the thread that reported OOM
+     * is either the current thread or waits for the GC to complete on this
+     * thread.
+     */
+    if (rt->inOOMReport) {
+        JS_ASSERT(gckind != GC_LAST_CONTEXT);
+        return;
+    }
+
+    /*
      * We should not be depending on cx->compartment in the GC, so set it to
      * NULL to look for violations.
      */
     SwitchToCompartment sc(cx, (JSCompartment *)NULL);
 
     JS_ASSERT(!rt->gcCurrentCompartment);
     rt->gcCurrentCompartment = comp;
 
@@ -2692,29 +2700,30 @@ GCCycle(JSContext *cx, JSCompartment *co
         (*c)->setGCLastBytes((*c)->gcBytes);
 }
 
 void
 js_GC(JSContext *cx, JSCompartment *comp, JSGCInvocationKind gckind)
 {
     JSRuntime *rt = cx->runtime;
 
-    /* Don't GC while reporting an OOM. */
-    if (rt->inOOMReport)
-        return;
-
     /*
      * Don't collect garbage if the runtime isn't up, and cx is not the last
      * context in the runtime.  The last context must force a GC, and nothing
      * should suppress that final collection or there may be shutdown leaks,
      * or runtime bloat until the next context is created.
      */
     if (rt->state != JSRTS_UP && gckind != GC_LAST_CONTEXT)
         return;
 
+    if (JS_ON_TRACE(cx)) {
+        JS_ASSERT(gckind != GC_LAST_CONTEXT);
+        return;
+    }
+
     RecordNativeStackTopForGC(cx);
 
     GCTIMER_BEGIN(rt, comp);
 
     do {
         /*
          * Let the API user decide to defer a GC if it wants to (unless this
          * is the last context).  Invoke the callback regardless. Sample the
--- a/js/src/jslock.h
+++ b/js/src/jslock.h
@@ -213,25 +213,47 @@ js_CompareAndSwap(jsword *w, jsword ov, 
     return (*w == ov) ? *w = nv, JS_TRUE : JS_FALSE;
 }
 
 #define JS_ATOMIC_SET_MASK(w, mask) (*(w) |= (mask))
 #define JS_ATOMIC_CLEAR_MASK(w, mask) (*(w) &= ~(mask))
 
 #endif
 
+#ifdef __cplusplus
+
+namespace js {
+
 #ifdef JS_THREADSAFE
-namespace js {
 class AutoLock {
   private:
     JSLock *lock;
 
   public:
     AutoLock(JSLock *lock) : lock(lock) { JS_ACQUIRE_LOCK(lock); }
     ~AutoLock() { JS_RELEASE_LOCK(lock); }
 };
-}  /* namespace js */
 # define JS_AUTO_LOCK_GUARD(name, l) AutoLock name((l));
 #else
 # define JS_AUTO_LOCK_GUARD(name, l)
 #endif
 
+class AutoAtomicIncrement {
+    int32 *p;
+    JS_DECL_USE_GUARD_OBJECT_NOTIFIER
+
+  public:
+    AutoAtomicIncrement(int32 *p JS_GUARD_OBJECT_NOTIFIER_PARAM)
+      : p(p) {
+        JS_GUARD_OBJECT_NOTIFIER_INIT;
+        JS_ATOMIC_INCREMENT(p);
+    }
+
+    ~AutoAtomicIncrement() {
+        JS_ATOMIC_DECREMENT(p);
+    }
+};
+
+} /* namespace js */
+
+#endif
+
 #endif /* jslock_h___ */