From f684ef021a6d19e85be78d0c5650b187df3fb1fc Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
Date: Thu, 23 Apr 2020 16:27:01 -0700
Subject: [PATCH] [weakrefs] Update js_weak_refs worklist after scavenges

The js_weak_refs worklist is currently not updated after scavenges,
unlike other weak reference worklist.

Bug: v8:8179, chromium:1073981
Change-Id: I48172606995253edb8a0c96f2b7e2dc34cd3d0d6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2163827
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67350}
---
 src/heap/incremental-marking.cc  | 21 +++++++
 test/cctest/test-js-weak-refs.cc | 94 ++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc
index ab644d3fd2..b625eb25c4 100644
--- a/src/heap/incremental-marking.cc
+++ b/src/heap/incremental-marking.cc
@@ -658,6 +658,27 @@ void IncrementalMarking::UpdateWeakReferencesAfterScavenge() {
         DCHECK(!Heap::InYoungGeneration(candidate));
       });
 #endif
+
+  if (FLAG_harmony_weak_refs) {
+    weak_objects_->js_weak_refs.Update(
+        [](JSWeakRef js_weak_ref_in, JSWeakRef* js_weak_ref_out) -> bool {
+          JSWeakRef forwarded = ForwardingAddress(js_weak_ref_in);
+
+          if (!forwarded.is_null()) {
+            *js_weak_ref_out = forwarded;
+            return true;
+          }
+
+          return false;
+        });
+
+#ifdef DEBUG
+    // TODO(syg, marja): Support WeakCells in the young generation.
+    weak_objects_->weak_cells.Iterate([](WeakCell weak_cell) {
+      DCHECK(!Heap::InYoungGeneration(weak_cell));
+    });
+#endif
+  }
 }
 
 void IncrementalMarking::UpdateMarkedBytesAfterScavenge(
diff --git a/test/cctest/test-js-weak-refs.cc b/test/cctest/test-js-weak-refs.cc
index e36406c345..d6c09a1fd4 100644
--- a/test/cctest/test-js-weak-refs.cc
+++ b/test/cctest/test-js-weak-refs.cc
@@ -6,6 +6,7 @@
 #include "src/execution/microtask-queue.h"
 #include "src/handles/handles-inl.h"
 #include "src/heap/factory-inl.h"
+#include "src/heap/heap-inl.h"
 #include "src/objects/js-objects.h"
 #include "src/objects/js-weak-refs-inl.h"
 #include "test/cctest/cctest.h"
@@ -874,5 +875,98 @@ TEST(TestRemoveUnregisterToken) {
   }
 }
 
+TEST(JSWeakRefScavengedInWorklist) {
+  FLAG_harmony_weak_refs = true;
+  if (!FLAG_incremental_marking) {
+    return;
+  }
+
+  ManualGCScope manual_gc_scope;
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+
+  {
+    HandleScope outer_scope(isolate);
+    Handle<JSWeakRef> weak_ref;
+
+    // Make a WeakRef that points to a target, both of which become unreachable.
+    {
+      HandleScope inner_scope(isolate);
+      Handle<JSObject> js_object =
+          isolate->factory()->NewJSObject(isolate->object_function());
+      Handle<JSWeakRef> inner_weak_ref = ConstructJSWeakRef(js_object, isolate);
+      CHECK(Heap::InYoungGeneration(*js_object));
+      CHECK(Heap::InYoungGeneration(*inner_weak_ref));
+
+      weak_ref = inner_scope.CloseAndEscape(inner_weak_ref);
+    }
+
+    // Do marking. This puts the WeakRef above into the js_weak_refs worklist
+    // since its target isn't marked.
+    CHECK(
+        heap->mark_compact_collector()->weak_objects()->js_weak_refs.IsEmpty());
+    heap::SimulateIncrementalMarking(heap, true);
+    CHECK(!heap->mark_compact_collector()
+               ->weak_objects()
+               ->js_weak_refs.IsEmpty());
+  }
+
+  // Now collect both weak_ref and its target. The worklist should be empty.
+  CcTest::CollectGarbage(NEW_SPACE);
+  CHECK(heap->mark_compact_collector()->weak_objects()->js_weak_refs.IsEmpty());
+
+  // The mark-compactor shouldn't see zapped WeakRefs in the worklist.
+  CcTest::CollectAllGarbage();
+}
+
+TEST(JSWeakRefTenuredInWorklist) {
+  FLAG_harmony_weak_refs = true;
+  if (!FLAG_incremental_marking) {
+    return;
+  }
+
+  ManualGCScope manual_gc_scope;
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  Heap* heap = isolate->heap();
+
+  HandleScope outer_scope(isolate);
+  Handle<JSWeakRef> weak_ref;
+
+  // Make a WeakRef that points to a target. The target becomes unreachable.
+  {
+    HandleScope inner_scope(isolate);
+    Handle<JSObject> js_object =
+        isolate->factory()->NewJSObject(isolate->object_function());
+    Handle<JSWeakRef> inner_weak_ref = ConstructJSWeakRef(js_object, isolate);
+    CHECK(Heap::InYoungGeneration(*js_object));
+    CHECK(Heap::InYoungGeneration(*inner_weak_ref));
+
+    weak_ref = inner_scope.CloseAndEscape(inner_weak_ref);
+  }
+  JSWeakRef old_weak_ref_location = *weak_ref;
+
+  // Do marking. This puts the WeakRef above into the js_weak_refs worklist
+  // since its target isn't marked.
+  CHECK(heap->mark_compact_collector()->weak_objects()->js_weak_refs.IsEmpty());
+  heap::SimulateIncrementalMarking(heap, true);
+  CHECK(
+      !heap->mark_compact_collector()->weak_objects()->js_weak_refs.IsEmpty());
+
+  // Now collect weak_ref's target. We still have a Handle to weak_ref, so it is
+  // moved and remains on the worklist.
+  CcTest::CollectGarbage(NEW_SPACE);
+  JSWeakRef new_weak_ref_location = *weak_ref;
+  CHECK_NE(old_weak_ref_location, new_weak_ref_location);
+  CHECK(
+      !heap->mark_compact_collector()->weak_objects()->js_weak_refs.IsEmpty());
+
+  // The mark-compactor should see the moved WeakRef in the worklist.
+  CcTest::CollectAllGarbage();
+  CHECK(heap->mark_compact_collector()->weak_objects()->js_weak_refs.IsEmpty());
+  CHECK(weak_ref->target().IsUndefined(isolate));
+}
+
 }  // namespace internal
 }  // namespace v8