[heap]: Make addition of detached contexts robust for GC

ulan · Commit Bot · commit b33a8508ccad · 2019-11-06T17:59:21.000Z
The (age, context) pair has to be added atomically in to the weak array of detached contexts. Otherwise, GC may happen after insertion of age and observe inconsistent state. Bug: chromium:1016703 Change-Id: Icb20bed4359904b2d976986a236558542e314bbf Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1895573 Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Reviewed-by: Toon Verwaest <verwaest@chromium.org> Cr-Commit-Position: refs/heads/master@{#64820}
diff --git a/src/execution/isolate.cc b/src/execution/isolate.cc
@@ -4273,9 +4273,8 @@ void Isolate::AddDetachedContext(Handle<Context> context) {
   HandleScope scope(this);
   Handle<WeakArrayList> detached_contexts = factory()->detached_contexts();
   detached_contexts = WeakArrayList::AddToEnd(
-      this, detached_contexts, MaybeObjectHandle(Smi::kZero, this));
-  detached_contexts = WeakArrayList::AddToEnd(this, detached_contexts,
-                                              MaybeObjectHandle::Weak(context));
+      this, detached_contexts, MaybeObjectHandle(Smi::kZero, this),
+      MaybeObjectHandle::Weak(context));
   heap()->set_detached_contexts(*detached_contexts);
 }
 
diff --git a/src/objects/fixed-array.h b/src/objects/fixed-array.h
@@ -338,6 +338,12 @@ class WeakArrayList : public HeapObject {
       Isolate* isolate, Handle<WeakArrayList> array,
       const MaybeObjectHandle& value);
 
+  // A version that adds to elements. This ensures that the elements are
+  // inserted atomically w.r.t GC.
+  V8_EXPORT_PRIVATE static Handle<WeakArrayList> AddToEnd(
+      Isolate* isolate, Handle<WeakArrayList> array,
+      const MaybeObjectHandle& value1, const MaybeObjectHandle& value2);
+
   inline MaybeObject Get(int index) const;
   inline MaybeObject Get(Isolate* isolate, int index) const;
 
diff --git a/src/objects/objects.cc b/src/objects/objects.cc
@@ -3955,6 +3955,20 @@ Handle<WeakArrayList> WeakArrayList::AddToEnd(Isolate* isolate,
   return array;
 }
 
+Handle<WeakArrayList> WeakArrayList::AddToEnd(Isolate* isolate,
+                                              Handle<WeakArrayList> array,
+                                              const MaybeObjectHandle& value1,
+                                              const MaybeObjectHandle& value2) {
+  int length = array->length();
+  array = EnsureSpace(isolate, array, length + 2);
+  // Reload length; GC might have removed elements from the array.
+  length = array->length();
+  array->Set(length, *value1);
+  array->Set(length + 1, *value2);
+  array->set_length(length + 2);
+  return array;
+}
+
 bool WeakArrayList::IsFull() { return length() == capacity(); }
 
 // static
diff --git a/test/mjsunit/regress/regress-1016703.js b/test/mjsunit/regress/regress-1016703.js
@@ -0,0 +1,15 @@
+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-gc
+
+let realms = [];
+for (let i = 0; i < 4; i++) {
+  realms.push(Realm.createAllowCrossRealmAccess());
+}
+
+for (let i = 0; i < 4; i++) {
+  Realm.detachGlobal(realms[i]);
+  gc();
+}
