From 876b48f384750a9b90ebe80f826f69c77f2710d2 Mon Sep 17 00:00:00 2001
From: "ulan@chromium.org"
 <ulan@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Wed, 14 May 2014 09:12:21 +0000
Subject: [PATCH] Skip write barriers when updating the weak hash table.

Write barrier on the weak hash table makes all its pointers strong,
which can cause a memory leak.

BUG=359401
LOG=Y
TEST=cctest/test-heap/NoWeakHashTableLeakWithIncrementalMarking
R=hpayer@chromium.org, mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/284773004

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21299 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/heap.cc              |  2 ++
 src/mark-compact.cc      |  1 +
 src/objects.cc           | 12 +++++++++---
 test/cctest/test-heap.cc | 37 ++++++++++++++++++++++++++++++++++++-
 4 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/src/heap.cc b/src/heap.cc
index f0c9154..17f31ff 100644
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -5460,6 +5460,8 @@ void Heap::AddWeakObjectToCodeDependency(Handle<Object> obj,
                                          Handle<DependentCode> dep) {
   ASSERT(!InNewSpace(*obj));
   ASSERT(!InNewSpace(*dep));
+  // This handle scope keeps the table handle local to this function, which
+  // allows us to safely skip write barriers in table update operations.
   HandleScope scope(isolate());
   Handle<WeakHashTable> table(WeakHashTable::cast(weak_object_to_code_table_),
                               isolate());
diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index ff6d2e3..9103910 100644
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -2634,6 +2634,7 @@ void MarkCompactCollector::ClearNonLiveReferences() {
         ClearDependentCode(DependentCode::cast(value));
         table->set(key_index, heap_->the_hole_value());
         table->set(value_index, heap_->the_hole_value());
+        table->ElementRemoved();
       }
     }
   }
diff --git a/src/objects.cc b/src/objects.cc
index f9a52e5..b7928af 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -16168,7 +16168,10 @@ Handle<WeakHashTable> WeakHashTable::Put(Handle<WeakHashTable> table,
   int entry = table->FindEntry(key);
   // Key is already in table, just overwrite value.
   if (entry != kNotFound) {
-    table->set(EntryToValueIndex(entry), *value);
+    // TODO(ulan): Skipping write barrier is a temporary solution to avoid
+    // memory leaks. Remove this once we have special visitor for weak fixed
+    // arrays.
+    table->set(EntryToValueIndex(entry), *value, SKIP_WRITE_BARRIER);
     return table;
   }
 
@@ -16184,8 +16187,11 @@ void WeakHashTable::AddEntry(int entry,
                              Handle<Object> key,
                              Handle<Object> value) {
   DisallowHeapAllocation no_allocation;
-  set(EntryToIndex(entry), *key);
-  set(EntryToValueIndex(entry), *value);
+  // TODO(ulan): Skipping write barrier is a temporary solution to avoid
+  // memory leaks. Remove this once we have special visitor for weak fixed
+  // arrays.
+  set(EntryToIndex(entry), *key, SKIP_WRITE_BARRIER);
+  set(EntryToValueIndex(entry), *value, SKIP_WRITE_BARRIER);
   ElementAdded();
 }
 
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 22d2458..757b36d 100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -40,7 +40,6 @@
 
 using namespace v8::internal;
 
-
 // Go through all incremental marking steps in one swoop.
 static void SimulateIncrementalMarking() {
   MarkCompactCollector* collector = CcTest::heap()->mark_compact_collector();
@@ -3888,6 +3887,42 @@ TEST(ObjectsInOptimizedCodeAreWeak) {
 }
 
 
+TEST(NoWeakHashTableLeakWithIncrementalMarking) {
+  if (i::FLAG_always_opt || !i::FLAG_crankshaft) return;
+  if (!i::FLAG_incremental_marking) return;
+  i::FLAG_weak_embedded_objects_in_optimized_code = true;
+  i::FLAG_allow_natives_syntax = true;
+  i::FLAG_compilation_cache = false;
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  v8::internal::Heap* heap = CcTest::heap();
+
+  if (!isolate->use_crankshaft()) return;
+  HandleScope outer_scope(heap->isolate());
+  for (int i = 0; i < 3; i++) {
+    SimulateIncrementalMarking();
+    {
+      LocalContext context;
+      HandleScope scope(heap->isolate());
+      EmbeddedVector<char, 256> source;
+      OS::SNPrintF(source,
+                   "function bar%d() {"
+                   "  return foo%d(1);"
+                   "};"
+                   "function foo%d(x) { with (x) { return 1 + x; } };"
+                   "bar%d();"
+                   "bar%d();"
+                   "bar%d();"
+                   "%OptimizeFunctionOnNextCall(bar%d);"
+                   "bar%d();", i, i, i, i, i, i, i, i);
+      CompileRun(source.start());
+    }
+    heap->CollectAllGarbage(i::Heap::kNoGCFlags);
+  }
+  WeakHashTable* table = WeakHashTable::cast(heap->weak_object_to_code_table());
+  CHECK_EQ(0, table->NumberOfElements());
+}
+
 
 static Handle<JSFunction> OptimizeDummyFunction(const char* name) {
   EmbeddedVector<char, 256> source;
-- 
2.7.4