Bug 1573458 - Leave the atoms zone when performing a GC r=tcampbell
authorJon Coppeard <jcoppeard@mozilla.com>
Fri, 16 Aug 2019 16:26:30 +0000
changeset 488663 ffeb52190484d1fd714d2c5452e561818a6d1f49
parent 488662 7b37e3f241fbdac1229b2c44a8363f1337a939a8
child 488664 7eebe81972078705a05b42488e23265a9948be94
child 488665 74064a61eb294e3b6e2fd8b05aa9f189f08af889
push id113915
push userdvarga@mozilla.com
push dateSun, 18 Aug 2019 09:56:49 +0000
treeherdermozilla-inbound@7eebe8197207 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstcampbell
bugs1573458
milestone70.0a1
first release with
nightly linux32
ffeb52190484 / 70.0a1 / 20190818094547 / files
nightly linux64
ffeb52190484 / 70.0a1 / 20190818094547 / files
nightly mac
ffeb52190484 / 70.0a1 / 20190818094547 / files
nightly win32
ffeb52190484 / 70.0a1 / 20190818094547 / files
nightly win64
ffeb52190484 / 70.0a1 / 20190818094547 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1573458 - Leave the atoms zone when performing a GC r=tcampbell Entering the atoms zone with AutoAllocInAtomsZone is a bit of a special case and we don't support entering another realm in this state. Unfortunately this can happen during GC in a couple of place. The patch temporarily leaves the atoms zone during GC so that callbacks can enter whatever zones they like. Differential Revision: https://phabricator.services.mozilla.com/D42312
js/src/builtin/TestingFunctions.cpp
js/src/gc/GC.cpp
js/src/gc/Nursery.cpp
js/src/jit-test/tests/gc/bug-1573458.js
js/src/vm/Realm.h
--- a/js/src/builtin/TestingFunctions.cpp
+++ b/js/src/builtin/TestingFunctions.cpp
@@ -4397,16 +4397,20 @@ static void minorGC(JSContext* cx, JSGCS
     info->active = true;
   }
 }
 
 // Process global, should really be runtime-local.
 static MajorGC majorGCInfo;
 static MinorGC minorGCInfo;
 
+static void enterNullRealm(JSContext* cx, JSGCStatus status, void* data) {
+  JSAutoNullableRealm enterRealm(cx, nullptr);
+}
+
 } /* namespace gcCallback */
 
 static bool SetGCCallback(JSContext* cx, unsigned argc, Value* vp) {
   CallArgs args = CallArgsFromVp(argc, vp);
 
   if (args.length() != 1) {
     JS_ReportErrorASCII(cx, "Wrong number of arguments");
     return false;
@@ -4484,16 +4488,18 @@ static bool SetGCCallback(JSContext* cx,
         gcstats::Statistics::MAX_SUSPENDED_PHASES) {
       JS_ReportErrorASCII(cx, "Nesting depth too large, would overflow");
       return false;
     }
 
     gcCallback::majorGCInfo.phases = phases;
     gcCallback::majorGCInfo.depth = depth;
     JS_SetGCCallback(cx, gcCallback::majorGC, &gcCallback::majorGCInfo);
+  } else if (StringEqualsAscii(action, "enterNullRealm")) {
+    JS_SetGCCallback(cx, gcCallback::enterNullRealm, nullptr);
   } else {
     JS_ReportErrorASCII(cx, "Unknown GC callback action");
     return false;
   }
 
   args.rval().setUndefined();
   return true;
 }
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -7719,16 +7719,17 @@ void GCRuntime::collect(bool nonincremen
   }
 
   stats().writeLogMessage("GC starting in state %s",
                           StateName(incrementalState));
 
   AutoTraceLog logGC(TraceLoggerForCurrentThread(), TraceLogger_GC);
   AutoStopVerifyingBarriers av(rt, IsShutdownGC(reason));
   AutoEnqueuePendingParseTasksAfterGC aept(*this);
+  AutoMaybeLeaveAtomsZone leaveAtomsZone(rt->mainContextFromOwnThread());
 
 #ifdef DEBUG
   if (IsShutdownGC(reason)) {
     marker.markQueue.clear();
     marker.queuePos = 0;
   }
 #endif
 
@@ -7926,16 +7927,18 @@ void GCRuntime::minorGC(JS::GCReason rea
   MOZ_ASSERT(!JS::RuntimeHeapIsBusy());
 
   MOZ_ASSERT_IF(reason == JS::GCReason::EVICT_NURSERY,
                 !rt->mainContextFromOwnThread()->suppressGC);
   if (rt->mainContextFromOwnThread()->suppressGC) {
     return;
   }
 
+  AutoMaybeLeaveAtomsZone leaveAtomsZone(rt->mainContextFromOwnThread());
+
   // Note that we aren't collecting the updated alloc counts from any helper
   // threads.  We should be but I'm not sure where to add that
   // synchronisation.
   uint32_t numAllocs =
       rt->mainContextFromOwnThread()->getAndResetAllocsThisZoneSinceMinorGC();
   for (ZonesIter zone(rt, WithAtoms); !zone.done(); zone.next()) {
     numAllocs += zone->getAndResetTenuredAllocsSinceMinorGC();
   }
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -1146,17 +1146,16 @@ float js::Nursery::doPretenuring(JSRunti
     JSContext* cx = rt->mainContextFromOwnThread();
     uint32_t threshold = tunables().pretenureGroupThreshold();
     for (auto& entry : tenureCounts.entries) {
       if (entry.count < threshold) {
         continue;
       }
 
       ObjectGroup* group = entry.group;
-      AutoMaybeLeaveAtomsZone leaveAtomsZone(cx);
       AutoRealm ar(cx, group);
       AutoSweepObjectGroup sweep(group);
       if (group->canPreTenure(sweep)) {
         group->setShouldPreTenure(sweep, cx);
         pretenureCount++;
       }
     }
   }
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/gc/bug-1573458.js
@@ -0,0 +1,4 @@
+gczeal(0);
+setGCCallback({action: "enterNullRealm"});
+gczeal(2, 1);
+Symbol();
--- a/js/src/vm/Realm.h
+++ b/js/src/vm/Realm.h
@@ -887,18 +887,19 @@ class MOZ_RAII AutoAllocInAtomsZone {
   AutoAllocInAtomsZone(const AutoAllocInAtomsZone&) = delete;
   AutoAllocInAtomsZone& operator=(const AutoAllocInAtomsZone&) = delete;
 
  public:
   inline explicit AutoAllocInAtomsZone(JSContext* cx);
   inline ~AutoAllocInAtomsZone();
 };
 
-// For the one place where we need to enter a realm when we may have been
-// allocating in the the atoms zone, this leaves the atoms zone temporarily.
+// During GC we sometimes need to enter a realm when we may have been allocating
+// in the the atoms zone. This leaves the atoms zone temporarily. This happens
+// in embedding callbacks and when we need to mark object groups as pretenured.
 class MOZ_RAII AutoMaybeLeaveAtomsZone {
   JSContext* const cx_;
   bool wasInAtomsZone_;
   AutoMaybeLeaveAtomsZone(const AutoMaybeLeaveAtomsZone&) = delete;
   AutoMaybeLeaveAtomsZone& operator=(const AutoMaybeLeaveAtomsZone&) = delete;
 
  public:
   inline explicit AutoMaybeLeaveAtomsZone(JSContext* cx);