Commit d8178cef authored by Jon Coppeard's avatar Jon Coppeard
Browse files

Bug 1594273 - Add an RAII class which empties the nursery as well as preparing... 

Bug 1594273 - Add an RAII class which empties the nursery as well as preparing for tracing r=sfink CLOSED TREE

Nursery eviction needs to be correctly interleaved with the other preparation steps since finishing the current GC can cause arbitrary code to run (and hence nursery allocations to be made) via calling the GC callbacks.

Differential Revision: https://phabricator.services.mozilla.com/D52161

--HG--
extra : source : 50b2bb645118f2650abfc9f40d409114e57746bd
extra : histedit_source : 4ee3a63419883791eceed3957fa7c328424f9e6d
parent 59387abc

js/src/gc/GC.cpp

+7 −0
Original line number Diff line number Diff line
@@ -7720,6 +7720,13 @@ bool GCRuntime::gcIfRequested() { 
}



void js::gc::FinishGC(JSContext* cx, JS::GCReason reason) {

  // Calling this when GC is suppressed won't have any effect.

  MOZ_ASSERT(!cx->suppressGC);



  // GC callbacks may run arbitrary code, including JS. Check this regardless of

  // whether we GC for this invocation.

  MOZ_ASSERT(cx->isNurseryAllocAllowed());



  if (JS::IsIncrementalGCInProgress(cx)) {

    JS::PrepareForIncrementalGC(cx);

    JS::FinishIncrementalGC(cx, reason);

js/src/gc/GCInternals.h

+65 −57
Original line number Diff line number Diff line
@@ -20,6 +20,57 @@ 
namespace js {

namespace gc {



/*

 * There are a couple of classes here that serve mostly as "tokens" indicating

 * that a precondition holds. Some functions force the caller to possess such a

 * token because they require the precondition to hold, and it is better to make

 * the precondition explicit at the API entry point than to crash in an

 * assertion later on when it is relied upon.

 */



struct MOZ_RAII AutoAssertNoNurseryAlloc {

#ifdef DEBUG

  AutoAssertNoNurseryAlloc();

  ~AutoAssertNoNurseryAlloc();

#else

  AutoAssertNoNurseryAlloc() {}

#endif

};



/*

 * A class that serves as a token that the nursery in the current thread's zone

 * group is empty.

 */

class MOZ_RAII AutoAssertEmptyNursery {

 protected:

  JSContext* cx;



  mozilla::Maybe<AutoAssertNoNurseryAlloc> noAlloc;



  // Check that the nursery is empty.

  void checkCondition(JSContext* cx);



  // For subclasses that need to empty the nursery in their constructors.

  AutoAssertEmptyNursery() : cx(nullptr) {}



 public:

  explicit AutoAssertEmptyNursery(JSContext* cx) : cx(nullptr) {

    checkCondition(cx);

  }



  AutoAssertEmptyNursery(const AutoAssertEmptyNursery& other)

      : AutoAssertEmptyNursery(other.cx) {}

};



/*

 * Evict the nursery upon construction. Serves as a token indicating that the

 * nursery is empty. (See AutoAssertEmptyNursery, above.)

 */

class MOZ_RAII AutoEmptyNursery : public AutoAssertEmptyNursery {

 public:

  explicit AutoEmptyNursery(JSContext* cx);

};



class MOZ_RAII AutoCheckCanAccessAtomsDuringGC {

#ifdef DEBUG

  JSRuntime* runtime;
@@ -99,6 +150,20 @@ class MOZ_RAII AutoPrepareForTracing : private AutoFinishGC, 
        AutoTraceSession(cx->runtime()) {}

};



// This class should be used by any code that needs exclusive access to the heap

// in order to trace through it.

//

// This version also empties the nursery after finishing any ongoing GC.

class MOZ_RAII AutoEmptyNurseryAndPrepareForTracing : private AutoFinishGC,

                                                      public AutoEmptyNursery,

                                                      public AutoTraceSession {

 public:

  explicit AutoEmptyNurseryAndPrepareForTracing(JSContext* cx)

      : AutoFinishGC(cx, JS::GCReason::PREPARE_FOR_TRACING),

        AutoEmptyNursery(cx),

        AutoTraceSession(cx->runtime()) {}

};



AbortReason IsIncrementalGCUnsafe(JSRuntime* rt);



#ifdef JS_GC_ZEAL
@@ -244,63 +309,6 @@ struct TenureCountCache { 
  }

};



struct MOZ_RAII AutoAssertNoNurseryAlloc {

#ifdef DEBUG

  AutoAssertNoNurseryAlloc();

  ~AutoAssertNoNurseryAlloc();

#else

  AutoAssertNoNurseryAlloc() {}

#endif

};



/*

 * There are a couple of classes here that serve mostly as "tokens" indicating

 * that a condition holds. Some functions force the caller to possess such a

 * token because they would misbehave if the condition were false, and it is

 * far more clear to make the condition visible at the point where it can be

 * affected rather than just crashing in an assertion down in the place where

 * it is relied upon.

 */



/*

 * A class that serves as a token that the nursery in the current thread's zone

 * group is empty.

 */

class MOZ_RAII AutoAssertEmptyNursery {

 protected:

  JSContext* cx;



  mozilla::Maybe<AutoAssertNoNurseryAlloc> noAlloc;



  // Check that the nursery is empty.

  void checkCondition(JSContext* cx);



  // For subclasses that need to empty the nursery in their constructors.

  AutoAssertEmptyNursery() : cx(nullptr) {}



 public:

  explicit AutoAssertEmptyNursery(JSContext* cx) : cx(nullptr) {

    checkCondition(cx);

  }



  AutoAssertEmptyNursery(const AutoAssertEmptyNursery& other)

      : AutoAssertEmptyNursery(other.cx) {}

};



/*

 * Evict the nursery upon construction. Serves as a token indicating that the

 * nursery is empty. (See AutoAssertEmptyNursery, above.)

 *

 * Note that this is very improper subclass of AutoAssertHeapBusy, in that the

 * heap is *not* busy within the scope of an AutoEmptyNursery. I will most

 * likely fix this by removing AutoAssertHeapBusy, but that is currently

 * waiting on jonco's review.

 */

class MOZ_RAII AutoEmptyNursery : public AutoAssertEmptyNursery {

 public:

  explicit AutoEmptyNursery(JSContext* cx);

};



extern void DelayCrossCompartmentGrayMarking(JSObject* src);



inline bool IsOOMReason(JS::GCReason reason) {

js/src/gc/PublicIterators.cpp

+3 −4
Original line number Diff line number Diff line
@@ -145,13 +145,12 @@ template <typename T, typename Callback> 
static void IterateScriptsImpl(JSContext* cx, Realm* realm, void* data,

                               Callback scriptCallback) {

  MOZ_ASSERT(!cx->suppressGC);

  AutoEmptyNursery empty(cx);

  AutoPrepareForTracing prep(cx);

  AutoEmptyNurseryAndPrepareForTracing prep(cx);

  JS::AutoSuppressGCAnalysis nogc;



  if (realm) {

    Zone* zone = realm->zone();

    for (auto iter = zone->cellIter<T>(empty); !iter.done(); iter.next()) {

    for (auto iter = zone->cellIter<T>(prep); !iter.done(); iter.next()) {

      T* script = iter;

      if (script->realm() != realm) {

        continue;
@@ -160,7 +159,7 @@ static void IterateScriptsImpl(JSContext* cx, Realm* realm, void* data, 
    }

  } else {

    for (ZonesIter zone(cx->runtime(), SkipAtoms); !zone.done(); zone.next()) {

      for (auto iter = zone->cellIter<T>(empty); !iter.done(); iter.next()) {

      for (auto iter = zone->cellIter<T>(prep); !iter.done(); iter.next()) {

        T* script = iter;

        DoScriptCallback(cx, data, script, scriptCallback, nogc);

      }

js/src/gc/RootMarking.cpp

+1 −2
Original line number Diff line number Diff line
@@ -312,8 +312,7 @@ void js::TraceRuntime(JSTracer* trc) { 
  MOZ_ASSERT(!trc->isMarkingTracer());



  JSRuntime* rt = trc->runtime();

  rt->gc.evictNursery();

  AutoPrepareForTracing prep(rt->mainContextFromOwnThread());

  AutoEmptyNurseryAndPrepareForTracing prep(rt->mainContextFromOwnThread());

  gcstats::AutoPhase ap(rt->gc.stats(), gcstats::PhaseKind::TRACE_HEAP);

  rt->gc.traceRuntime(trc, prep);

}