Skip write barriers when updating the weak hash table.

test/cctest/test-heap.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 22 matching lines @@
 #include "compilation-cache.h"
 #include "execution.h"
 #include "factory.h"
 #include "macro-assembler.h"
 #include "global-handles.h"
 #include "stub-cache.h"
 #include "cctest.h"
 
 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()) {
     collector->WaitUntilSweepingCompleted();
   }
   CHECK(marking->IsMarking() || marking->IsStopped());
   if (marking->IsStopped()) {
     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);
   }
   CHECK(marking->IsComplete());
 }
 
 
+// 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
   CHECK(CcTest::heap()->Contains(map));
 #endif
   CHECK_EQ(CcTest::heap()->meta_map(), map->map());
   CHECK_EQ(type, map->instance_type());
   CHECK_EQ(instance_size, map->instance_size());
 }
 
@@ skipping 3808 matching lines @@
 
   // Now make sure that a gc should get rid of the function
   for (int i = 0; i < 4; i++) {
     heap->CollectAllGarbage(Heap::kAbortIncrementalMarkingMask);
   }
 
   ASSERT(code->marked_for_deoptimization());
 }
 
 
+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;
   OS::SNPrintF(source,
               "function %s() { return 0; }"
               "%s(); %s();"
               "%%OptimizeFunctionOnNextCall(%s);"
               "%s();", name, name, name, name, name);
   CompileRun(source.start());
   Handle<JSFunction> fun =
@@ skipping 304 matching lines @@
       "array;");
 
   Handle<JSObject> o =
       v8::Utils::OpenHandle(*v8::Handle<v8::Object>::Cast(result));
   CHECK(heap->InOldPointerSpace(o->elements()));
   CHECK(heap->InOldPointerSpace(*o));
   Page* page = Page::FromAddress(o->elements()->address());
   CHECK(page->WasSwept() ||
         Marking::IsBlack(Marking::MarkBitFrom(o->elements())));
 }