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

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}

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);
 }
 

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;
 

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

test/mjsunit/regress/regress-1016703.js

+// 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();
+}