Reland r19897 "Fix memory leak caused by treating Code::next_code_link as strong...

R=mstarzinger@chromium.org

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20282 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/heap.cc b/src/heap.cc
index 8d321ad..6374433 100644 (file)
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -1775,6 +1775,18 @@ static Object* VisitWeakList(Heap* heap,
 }
 
 
+template <class T>
+static void ClearWeakList(Heap* heap,
+                          Object* list) {
+  Object* undefined = heap->undefined_value();
+  while (list != undefined) {
+    T* candidate = reinterpret_cast<T*>(list);
+    list = WeakListVisitor<T>::WeakNext(candidate);
+    WeakListVisitor<T>::SetWeakNext(candidate, undefined);
+  }
+}
+
+
 template<>
 struct WeakListVisitor<JSFunction> {
   static void SetWeakNext(JSFunction* function, Object* next) {
@@ -1868,7 +1880,11 @@ struct WeakListVisitor<Context> {
     }
   }
 
-  static void VisitPhantomObject(Heap*, Context*) {
+  static void VisitPhantomObject(Heap* heap, Context* context) {
+    ClearWeakList<JSFunction>(heap,
+        context->get(Context::OPTIMIZED_FUNCTIONS_LIST));
+    ClearWeakList<Code>(heap, context->get(Context::OPTIMIZED_CODE_LIST));
+    ClearWeakList<Code>(heap, context->get(Context::DEOPTIMIZED_CODE_LIST));
   }
 
   static int WeakNextOffset() {
@@ -4159,6 +4175,7 @@ MaybeObject* Heap::CreateCode(const CodeDesc& desc,
   code->set_is_crankshafted(crankshafted);
   code->set_deoptimization_data(empty_fixed_array(), SKIP_WRITE_BARRIER);
   code->set_raw_type_feedback_info(undefined_value());
+  code->set_next_code_link(undefined_value());
   code->set_handler_table(empty_fixed_array(), SKIP_WRITE_BARRIER);
   code->set_gc_metadata(Smi::FromInt(0));
   code->set_ic_age(global_ic_age_);

diff --git a/src/mark-compact.cc b/src/mark-compact.cc
index 4eb178b..f04a8bc 100644 (file)
--- a/src/mark-compact.cc
+++ b/src/mark-compact.cc
@@ -1863,6 +1863,10 @@ class RootMarkingVisitor : public ObjectVisitor {
     for (Object** p = start; p < end; p++) MarkObjectByPointer(p);
   }
 
+  // Skip the weak next code link in a code object, which is visited in
+  // ProcessTopOptimizedFrame.
+  void VisitNextCodeLink(Object** p) { }
+
  private:
   void MarkObjectByPointer(Object** p) {
     if (!(*p)->IsHeapObject()) return;

diff --git a/src/objects-inl.h b/src/objects-inl.h
index cad7b63..acfbb43 100644 (file)
--- a/src/objects-inl.h
+++ b/src/objects-inl.h
@@ -1393,6 +1393,11 @@ void HeapObject::IteratePointer(ObjectVisitor* v, int offset) {
 }
 
 
+void HeapObject::IterateNextCodeLink(ObjectVisitor* v, int offset) {
+  v->VisitNextCodeLink(reinterpret_cast<Object**>(FIELD_ADDR(this, offset)));
+}
+
+
 double HeapNumber::value() {
   return READ_DOUBLE_FIELD(this, kValueOffset);
 }
@@ -5787,6 +5792,7 @@ ACCESSORS(Code, relocation_info, ByteArray, kRelocationInfoOffset)
 ACCESSORS(Code, handler_table, FixedArray, kHandlerTableOffset)
 ACCESSORS(Code, deoptimization_data, FixedArray, kDeoptimizationDataOffset)
 ACCESSORS(Code, raw_type_feedback_info, Object, kTypeFeedbackInfoOffset)
+ACCESSORS(Code, next_code_link, Object, kNextCodeLinkOffset)
 
 
 void Code::WipeOutHeader() {
@@ -5815,20 +5821,6 @@ void Code::set_type_feedback_info(Object* value, WriteBarrierMode mode) {
 }
 
 
-Object* Code::next_code_link() {
-  CHECK(kind() == OPTIMIZED_FUNCTION);
-  return raw_type_feedback_info();
-}
-
-
-void Code::set_next_code_link(Object* value, WriteBarrierMode mode) {
-  CHECK(kind() == OPTIMIZED_FUNCTION);
-  set_raw_type_feedback_info(value);
-  CONDITIONAL_WRITE_BARRIER(GetHeap(), this, kTypeFeedbackInfoOffset,
-                            value, mode);
-}
-
-
 int Code::stub_info() {
   ASSERT(kind() == COMPARE_IC || kind() == COMPARE_NIL_IC ||
          kind() == BINARY_OP_IC || kind() == LOAD_IC);

diff --git a/src/objects-visiting-inl.h b/src/objects-visiting-inl.h
index 4f14988..31117bb 100644 (file)
--- a/src/objects-visiting-inl.h
+++ b/src/objects-visiting-inl.h
@@ -899,6 +899,7 @@ void Code::CodeIterateBody(ObjectVisitor* v) {
   IteratePointer(v, kHandlerTableOffset);
   IteratePointer(v, kDeoptimizationDataOffset);
   IteratePointer(v, kTypeFeedbackInfoOffset);
+  IterateNextCodeLink(v, kNextCodeLinkOffset);
   IteratePointer(v, kConstantPoolOffset);
 
   RelocIterator it(this, mode_mask);
@@ -933,6 +934,9 @@ void Code::CodeIterateBody(Heap* heap) {
   StaticVisitor::VisitPointer(
       heap,
       reinterpret_cast<Object**>(this->address() + kTypeFeedbackInfoOffset));
+  StaticVisitor::VisitNextCodeLink(
+      heap,
+      reinterpret_cast<Object**>(this->address() + kNextCodeLinkOffset));
   StaticVisitor::VisitPointer(
       heap,
       reinterpret_cast<Object**>(this->address() + kConstantPoolOffset));

diff --git a/src/objects-visiting.h b/src/objects-visiting.h
index 41e5fd6..de8ca6d 100644 (file)
--- a/src/objects-visiting.h
+++ b/src/objects-visiting.h
@@ -414,6 +414,8 @@ class StaticMarkingVisitor : public StaticVisitorBase {
   INLINE(static void VisitCodeAgeSequence(Heap* heap, RelocInfo* rinfo));
   INLINE(static void VisitExternalReference(RelocInfo* rinfo)) { }
   INLINE(static void VisitRuntimeEntry(RelocInfo* rinfo)) { }
+  // Skip the weak next code link in a code object.
+  INLINE(static void VisitNextCodeLink(Heap* heap, Object** slot)) { }
 
   // TODO(mstarzinger): This should be made protected once refactoring is done.
   // Mark non-optimize code for functions inlined into the given optimized

diff --git a/src/objects.h b/src/objects.h
index 35fed0a..0e352a2 100644 (file)
--- a/src/objects.h
+++ b/src/objects.h
@@ -1898,6 +1898,8 @@ class HeapObject: public Object {
   inline void IteratePointers(ObjectVisitor* v, int start, int end);
   // as above, for the single element at "offset"
   inline void IteratePointer(ObjectVisitor* v, int offset);
+  // as above, for the next code link of a code object.
+  inline void IterateNextCodeLink(ObjectVisitor* v, int offset);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(HeapObject);
@@ -5249,7 +5251,6 @@ class Code: public HeapObject {
   // the kind of the code object.
   //   FUNCTION           => type feedback information.
   //   STUB               => various things, e.g. a SMI
-  //   OPTIMIZED_FUNCTION => the next_code_link for optimized code list.
   DECL_ACCESSORS(raw_type_feedback_info, Object)
   inline Object* type_feedback_info();
   inline void set_type_feedback_info(
@@ -5582,8 +5583,8 @@ class Code: public HeapObject {
       kHandlerTableOffset + kPointerSize;
   static const int kTypeFeedbackInfoOffset =
       kDeoptimizationDataOffset + kPointerSize;
-  static const int kNextCodeLinkOffset = kTypeFeedbackInfoOffset;  // Shared.
-  static const int kGCMetadataOffset = kTypeFeedbackInfoOffset + kPointerSize;
+  static const int kNextCodeLinkOffset = kTypeFeedbackInfoOffset + kPointerSize;
+  static const int kGCMetadataOffset = kNextCodeLinkOffset + kPointerSize;
   static const int kICAgeOffset =
       kGCMetadataOffset + kPointerSize;
   static const int kFlagsOffset = kICAgeOffset + kIntSize;
@@ -10734,6 +10735,9 @@ class ObjectVisitor BASE_EMBEDDED {
   // Handy shorthand for visiting a single pointer.
   virtual void VisitPointer(Object** p) { VisitPointers(p, p + 1); }
 
+  // Visit weak next_code_link in Code object.
+  virtual void VisitNextCodeLink(Object** p) { VisitPointers(p, p + 1); }
+
   // To allow lazy clearing of inline caches the visitor has
   // a rich interface for iterating over Code objects..
 

diff --git a/test/cctest/test-heap.cc b/test/cctest/test-heap.cc
index d67f18d..55bb466 100644 (file)
--- a/test/cctest/test-heap.cc
+++ b/test/cctest/test-heap.cc
@@ -3752,6 +3752,102 @@ TEST(ObjectsInOptimizedCodeAreWeak) {
 }
 
 
+
+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 =
+      v8::Utils::OpenHandle(
+          *v8::Handle<v8::Function>::Cast(
+              CcTest::global()->Get(v8_str(name))));
+  return fun;
+}
+
+
+static int GetCodeChainLength(Code* code) {
+  int result = 0;
+  while (code->next_code_link()->IsCode()) {
+    result++;
+    code = Code::cast(code->next_code_link());
+  }
+  return result;
+}
+
+
+TEST(NextCodeLinkIsWeak) {
+  i::FLAG_allow_natives_syntax = true;
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  v8::internal::Heap* heap = CcTest::heap();
+
+  if (!isolate->use_crankshaft()) return;
+  HandleScope outer_scope(heap->isolate());
+  Handle<Code> code;
+  heap->CollectAllAvailableGarbage();
+  int code_chain_length_before, code_chain_length_after;
+  {
+    HandleScope scope(heap->isolate());
+    Handle<JSFunction> mortal = OptimizeDummyFunction("mortal");
+    Handle<JSFunction> immortal = OptimizeDummyFunction("immortal");
+    CHECK_EQ(immortal->code()->next_code_link(), mortal->code());
+    code_chain_length_before = GetCodeChainLength(immortal->code());
+    // Keep the immortal code and let the mortal code die.
+    code = scope.CloseAndEscape(Handle<Code>(immortal->code()));
+    CompileRun("mortal = null; immortal = null;");
+  }
+  heap->CollectAllAvailableGarbage();
+  // Now mortal code should be dead.
+  code_chain_length_after = GetCodeChainLength(*code);
+  CHECK_EQ(code_chain_length_before - 1, code_chain_length_after);
+}
+
+
+static Handle<Code> DummyOptimizedCode(Isolate* isolate) {
+  i::byte buffer[i::Assembler::kMinimalBufferSize];
+  MacroAssembler masm(isolate, buffer, sizeof(buffer));
+  CodeDesc desc;
+  masm.Prologue(BUILD_FUNCTION_FRAME);
+  masm.GetCode(&desc);
+  Handle<Object> undefined(isolate->heap()->undefined_value(), isolate);
+  Handle<Code> code = isolate->factory()->NewCode(
+      desc, Code::ComputeFlags(Code::OPTIMIZED_FUNCTION), undefined);
+  CHECK(code->IsCode());
+  return code;
+}
+
+
+TEST(NextCodeLinkIsWeak2) {
+  i::FLAG_allow_natives_syntax = true;
+  CcTest::InitializeVM();
+  Isolate* isolate = CcTest::i_isolate();
+  v8::internal::Heap* heap = CcTest::heap();
+
+  if (!isolate->use_crankshaft()) return;
+  HandleScope outer_scope(heap->isolate());
+  heap->CollectAllAvailableGarbage();
+  Handle<Context> context(Context::cast(heap->native_contexts_list()), isolate);
+  Handle<Code> new_head;
+  Handle<Object> old_head(context->get(Context::OPTIMIZED_CODE_LIST), isolate);
+  {
+    HandleScope scope(heap->isolate());
+    Handle<Code> immortal = DummyOptimizedCode(isolate);
+    Handle<Code> mortal = DummyOptimizedCode(isolate);
+    mortal->set_next_code_link(*old_head);
+    immortal->set_next_code_link(*mortal);
+    context->set(Context::OPTIMIZED_CODE_LIST, *immortal);
+    new_head = scope.CloseAndEscape(immortal);
+  }
+  heap->CollectAllAvailableGarbage();
+  // Now mortal code should be dead.
+  CHECK_EQ(*old_head, new_head->next_code_link());
+}
+
+
 #ifdef DEBUG
 TEST(AddInstructionChangesNewSpacePromotion) {
   i::FLAG_allow_natives_syntax = true;