Skip write barriers when updating the weak hash table.

test/cctest/test-heap.cc

Index: test/cctest/test-heap.cc
diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index 22d2458ceb98f02b155c36e608a5add4c0a97b09..78bb0cb48413e5490e0e2f2d92fac818b5c1eb82 100644
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -40,9 +40,7 @@
 
 using namespace v8::internal;
 
-
-// Go through all incremental marking steps in one swoop.
-static void SimulateIncrementalMarking() {
+static void StartIncrementalMarking() {
   MarkCompactCollector* collector = CcTest::heap()->mark_compact_collector();
   IncrementalMarking* marking = CcTest::heap()->incremental_marking();
   if (collector->IsConcurrentSweepingInProgress()) {
@@ -53,6 +51,12 @@ static void SimulateIncrementalMarking() {
     marking->Start();
   }
   CHECK(marking->IsMarking());
+}
+
+
+static void CompleteIncrementalMarking() {
+  IncrementalMarking* marking = CcTest::heap()->incremental_marking();
+  CHECK(marking->IsMarking());
   while (!marking->IsComplete()) {
     marking->Step(MB, IncrementalMarking::NO_GC_VIA_STACK_GUARD);
   }
@@ -60,6 +64,13 @@ static void SimulateIncrementalMarking() {
 }
 
 
+// Go through all incremental marking steps in one swoop.
+static void SimulateIncrementalMarking() {
+  StartIncrementalMarking();
+  CompleteIncrementalMarking();
+}
+
+
 static void CheckMap(Map* map, int type, int instance_size) {
   CHECK(map->IsHeapObject());
 #ifdef DEBUG
@@ -3888,6 +3899,44 @@ 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;

[Hannes Payer (out of office), 2014/05/14 06:47:45]

why i::FLAG_compilation_cache = false; ?

[ulan, 2014/05/14 07:27:10]

To avoid caching the generated code below. So that nothing keeps code objects
alive and it dies as soon as we leave the local scope.

+  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++) {
+    StartIncrementalMarking();

[Michael Starzinger, 2014/05/14 08:38:30]

I don't fully understand the reason for splitting SimuateIncrementalMarking.
Would it have the same effect to call SimulateIncrementalMarking here (before
the compilation) and below (after the compilation)? It should have the same
semantics AFAICT. If my assumption holds, then let's undo the split.

[ulan, 2014/05/14 08:54:04]

You're right! Calling SimulateIncrementalMarking at the beginning is sufficient
to reproduce the leak. I reverted this part of the change.

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

[Hannes Payer (out of office), 2014/05/14 06:47:45]

Why do we start incremental marking again here? We started it above.

[ulan, 2014/05/14 07:27:10]

This functions starts incremental marking if it is not active.

The JS call above can trigger GC, we might need to restart marking here,
otherwise CompleteIncrementalMarking will fail.

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