Abort incremental marking before GCs when test assumes that an object is collected.

test/cctest/test-api.cc

 // Copyright 2011 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 313 matching lines @@
 static uint16_t* AsciiToTwoByteString(const char* source) {
   int array_length = i::StrLength(source) + 1;
   uint16_t* converted = i::NewArray<uint16_t>(array_length);
   for (int i = 0; i < array_length; i++) converted[i] = source[i];
   return converted;
 }
 
 
 class TestResource: public String::ExternalStringResource {
  public:
-  static int dispose_count;
-
-  explicit TestResource(uint16_t* data)
-      : data_(data), length_(0) {
+  explicit TestResource(uint16_t* data, int* counter = NULL)
+    : data_(data), length_(0), counter_(counter) {
     while (data[length_]) ++length_;
   }
 
   ~TestResource() {
     i::DeleteArray(data_);
-    ++dispose_count;
+    if (counter_ != NULL) ++*counter_;
   }
 
   const uint16_t* data() const {
     return data_;
   }
 
   size_t length() const {
     return length_;
   }
  private:
   uint16_t* data_;
   size_t length_;
+  int* counter_;
 };
 
 
-int TestResource::dispose_count = 0;
-
-
 class TestAsciiResource: public String::ExternalAsciiStringResource {
  public:
-  static int dispose_count;
-
-  explicit TestAsciiResource(const char* data)
-      : data_(data),
-        length_(strlen(data)) { }
+  explicit TestAsciiResource(const char* data, int* counter = NULL)
+    : data_(data), length_(strlen(data)), counter_(counter) { }
 
   ~TestAsciiResource() {
     i::DeleteArray(data_);
-    ++dispose_count;
+    if (counter_ != NULL) ++*counter_;
   }
 
   const char* data() const {
     return data_;
   }
 
   size_t length() const {
     return length_;
   }
  private:
   const char* data_;
   size_t length_;
+  int* counter_;
 };
 
 
-int TestAsciiResource::dispose_count = 0;
-
-
 THREADED_TEST(ScriptUsingStringResource) {
-  TestResource::dispose_count = 0;
+  int dispose_count = 0;
   const char* c_source = "1 + 2 * 3";
   uint16_t* two_byte_source = AsciiToTwoByteString(c_source);
   {
     v8::HandleScope scope;
     LocalContext env;
-    TestResource* resource = new TestResource(two_byte_source);
+    TestResource* resource = new TestResource(two_byte_source, &dispose_count);
     Local<String> source = String::NewExternal(resource);
     Local<Script> script = Script::Compile(source);
     Local<Value> value = script->Run();
     CHECK(value->IsNumber());
     CHECK_EQ(7, value->Int32Value());
     CHECK(source->IsExternal());
     CHECK_EQ(resource,
              static_cast<TestResource*>(source->GetExternalStringResource()));
     HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-    CHECK_EQ(0, TestResource::dispose_count);
+    CHECK_EQ(0, dispose_count);
   }
   i::Isolate::Current()->compilation_cache()->Clear();
+  if (HEAP->incremental_marking()->IsMarking()) {

[Vyacheslav Egorov (Chromium), 2011/08/11 11:37:24]

Maybe we should always abort incremental marking when we call
CollectAllAvailableGarbage.

It has this 'All' in the name after all.

We also have a kMakeHeapIterableMask GC flag that cancels incremental marking
internally.

I think we can have another flag that cancels marking but does not do precise
sweeping. 

[Lasse Reichstein, 2011/08/11 12:20:13]

Actaully CollectAllAvailableGarbage does use the kMakeHeapIterableMask flag, so
this call is safe.
On the other hand CollectAllGarbage doesn't do it (and it has "All" in the name
too (which is odd naming - the unqualified CollectAllGarbage collects less than
the qualified CollectAllAvailableGarbage, and "Available" seems odd to use about
garbage which, by definition, isn't reachable)).

I've changed the code to use CollectAllAvailableGarbage instead of
CollectAllGarbage instead of inserting these manual aborts.

+    // Incremental marking might already have marked the string.
+    // We need to start a GC from scratch to ensure that it will be collected.
+    HEAP->incremental_marking()->Abort();
+  }
   HEAP->CollectAllAvailableGarbage();
-  CHECK_EQ(1, TestResource::dispose_count);
+  CHECK_EQ(1, dispose_count);
 }
 
 
 THREADED_TEST(ScriptUsingAsciiStringResource) {
-  TestAsciiResource::dispose_count = 0;
+  int dispose_count = 0;
   const char* c_source = "1 + 2 * 3";
   {
     v8::HandleScope scope;
     LocalContext env;
     Local<String> source =
-        String::NewExternal(new TestAsciiResource(i::StrDup(c_source)));
+        String::NewExternal(new TestAsciiResource(i::StrDup(c_source),
+                                                  &dispose_count));
     Local<Script> script = Script::Compile(source);
     Local<Value> value = script->Run();
     CHECK(value->IsNumber());
     CHECK_EQ(7, value->Int32Value());
     HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-    CHECK_EQ(0, TestAsciiResource::dispose_count);
+    CHECK_EQ(0, dispose_count);
   }
   i::Isolate::Current()->compilation_cache()->Clear();
+  if (HEAP->incremental_marking()->IsMarking()) {
+    HEAP->incremental_marking()->Abort();
+  }
   HEAP->CollectAllGarbage(i::Heap::kMakeHeapIterableMask);
-  CHECK_EQ(1, TestAsciiResource::dispose_count);
+  CHECK_EQ(1, dispose_count);
 }
 
 
 THREADED_TEST(ScriptMakingExternalString) {
-  TestResource::dispose_count = 0;
+  int dispose_count = 0;
   uint16_t* two_byte_source = AsciiToTwoByteString("1 + 2 * 3");
   {
     v8::HandleScope scope;
     LocalContext env;
     Local<String> source = String::New(two_byte_source);
     // Trigger GCs so that the newly allocated string moves to old gen.
     HEAP->CollectGarbage(i::NEW_SPACE);  // in survivor space now
     HEAP->CollectGarbage(i::NEW_SPACE);  // in old gen now
-    bool success = source->MakeExternal(new TestResource(two_byte_source));
+    bool success = source->MakeExternal(new TestResource(two_byte_source,
+                                                         &dispose_count));
     CHECK(success);
     Local<Script> script = Script::Compile(source);
     Local<Value> value = script->Run();
     CHECK(value->IsNumber());
     CHECK_EQ(7, value->Int32Value());
     HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-    CHECK_EQ(0, TestResource::dispose_count);
+    CHECK_EQ(0, dispose_count);
   }
   i::Isolate::Current()->compilation_cache()->Clear();
+  if (HEAP->incremental_marking()->IsMarking()) {
+    HEAP->incremental_marking()->Abort();
+  }
   HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-  CHECK_EQ(1, TestResource::dispose_count);
+  CHECK_EQ(1, dispose_count);
 }
 
 
 THREADED_TEST(ScriptMakingExternalAsciiString) {
-  TestAsciiResource::dispose_count = 0;
+  int dispose_count = 0;
   const char* c_source = "1 + 2 * 3";
   {
     v8::HandleScope scope;
     LocalContext env;
     Local<String> source = v8_str(c_source);
     // Trigger GCs so that the newly allocated string moves to old gen.
     HEAP->CollectGarbage(i::NEW_SPACE);  // in survivor space now
     HEAP->CollectGarbage(i::NEW_SPACE);  // in old gen now
     bool success = source->MakeExternal(
-        new TestAsciiResource(i::StrDup(c_source)));
+        new TestAsciiResource(i::StrDup(c_source), &dispose_count));
     CHECK(success);
     Local<Script> script = Script::Compile(source);
     Local<Value> value = script->Run();
     CHECK(value->IsNumber());
     CHECK_EQ(7, value->Int32Value());
     HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-    CHECK_EQ(0, TestAsciiResource::dispose_count);
+    CHECK_EQ(0, dispose_count);
   }
   i::Isolate::Current()->compilation_cache()->Clear();
+  if (HEAP->incremental_marking()->IsMarking()) {
+    HEAP->incremental_marking()->Abort();
+  }
   HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-  CHECK_EQ(1, TestAsciiResource::dispose_count);
+  CHECK_EQ(1, dispose_count);
 }
 
 
 TEST(MakingExternalStringConditions) {
   v8::HandleScope scope;
   LocalContext env;
 
   // Free some space in the new space so that we can check freshness.
   HEAP->CollectGarbage(i::NEW_SPACE);
   HEAP->CollectGarbage(i::NEW_SPACE);
@@ skipping 103 matching lines @@
     HEAP->CollectGarbage(i::NEW_SPACE);  // in old gen now
     i::Handle<i::String> isymbol = FACTORY->SymbolFromString(istring);
     CHECK(isymbol->IsSymbol());
   }
   HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
   HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
 }
 
 
 THREADED_TEST(ScavengeExternalString) {
-  TestResource::dispose_count = 0;
+  int dispose_count = 0;
   bool in_new_space = false;
   {
     v8::HandleScope scope;
     uint16_t* two_byte_string = AsciiToTwoByteString("test string");
     Local<String> string =
-        String::NewExternal(new TestResource(two_byte_string));
+      String::NewExternal(new TestResource(two_byte_string,
+                                           &dispose_count));
     i::Handle<i::String> istring = v8::Utils::OpenHandle(*string);
     HEAP->CollectGarbage(i::NEW_SPACE);
     in_new_space = HEAP->InNewSpace(*istring);
     CHECK(in_new_space || HEAP->old_data_space()->Contains(*istring));
-    CHECK_EQ(0, TestResource::dispose_count);
+    CHECK_EQ(0, dispose_count);
   }
   HEAP->CollectGarbage(in_new_space ? i::NEW_SPACE : i::OLD_DATA_SPACE);
-  CHECK_EQ(1, TestResource::dispose_count);
+  CHECK_EQ(1, dispose_count);
 }
 
 
 THREADED_TEST(ScavengeExternalAsciiString) {
-  TestAsciiResource::dispose_count = 0;
+  int dispose_count = 0;
   bool in_new_space = false;
   {
     v8::HandleScope scope;
     const char* one_byte_string = "test string";
     Local<String> string = String::NewExternal(
-        new TestAsciiResource(i::StrDup(one_byte_string)));
+        new TestAsciiResource(i::StrDup(one_byte_string), &dispose_count));
     i::Handle<i::String> istring = v8::Utils::OpenHandle(*string);
     HEAP->CollectGarbage(i::NEW_SPACE);
     in_new_space = HEAP->InNewSpace(*istring);
     CHECK(in_new_space || HEAP->old_data_space()->Contains(*istring));
-    CHECK_EQ(0, TestAsciiResource::dispose_count);
+    CHECK_EQ(0, dispose_count);
   }
   HEAP->CollectGarbage(in_new_space ? i::NEW_SPACE : i::OLD_DATA_SPACE);
-  CHECK_EQ(1, TestAsciiResource::dispose_count);
+  CHECK_EQ(1, dispose_count);
 }
 
 
 class TestAsciiResourceWithDisposeControl: public TestAsciiResource {
  public:
+  // Only used by non-threaded tests, so it can use static fields.
   static int dispose_calls;
+  static int dispose_count;
 
   TestAsciiResourceWithDisposeControl(const char* data, bool dispose)
-      : TestAsciiResource(data),
+      : TestAsciiResource(data, &dispose_count),
         dispose_(dispose) { }
 
   void Dispose() {
     ++dispose_calls;
     if (dispose_) delete this;
   }
  private:
   bool dispose_;
 };
 
 
+int TestAsciiResourceWithDisposeControl::dispose_count = 0;
 int TestAsciiResourceWithDisposeControl::dispose_calls = 0;
 
 
 TEST(ExternalStringWithDisposeHandling) {
   const char* c_source = "1 + 2 * 3";
 
   // Use a stack allocated external string resource allocated object.
-  TestAsciiResource::dispose_count = 0;
+  TestAsciiResourceWithDisposeControl::dispose_count = 0;
   TestAsciiResourceWithDisposeControl::dispose_calls = 0;
   TestAsciiResourceWithDisposeControl res_stack(i::StrDup(c_source), false);
   {
     v8::HandleScope scope;
     LocalContext env;
     Local<String> source =  String::NewExternal(&res_stack);
     Local<Script> script = Script::Compile(source);
     Local<Value> value = script->Run();
     CHECK(value->IsNumber());
     CHECK_EQ(7, value->Int32Value());
     HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-    CHECK_EQ(0, TestAsciiResource::dispose_count);
+    CHECK_EQ(0, TestAsciiResourceWithDisposeControl::dispose_count);
   }
   i::Isolate::Current()->compilation_cache()->Clear();
+  if (HEAP->incremental_marking()->IsMarking()) {
+    HEAP->incremental_marking()->Abort();
+  }
   HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
   CHECK_EQ(1, TestAsciiResourceWithDisposeControl::dispose_calls);
-  CHECK_EQ(0, TestAsciiResource::dispose_count);
+  CHECK_EQ(0, TestAsciiResourceWithDisposeControl::dispose_count);
 
   // Use a heap allocated external string resource allocated object.
-  TestAsciiResource::dispose_count = 0;
+  TestAsciiResourceWithDisposeControl::dispose_count = 0;
   TestAsciiResourceWithDisposeControl::dispose_calls = 0;
   TestAsciiResource* res_heap =
       new TestAsciiResourceWithDisposeControl(i::StrDup(c_source), true);
   {
     v8::HandleScope scope;
     LocalContext env;
     Local<String> source =  String::NewExternal(res_heap);
     Local<Script> script = Script::Compile(source);
     Local<Value> value = script->Run();
     CHECK(value->IsNumber());
     CHECK_EQ(7, value->Int32Value());
     HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
-    CHECK_EQ(0, TestAsciiResource::dispose_count);
+    CHECK_EQ(0, TestAsciiResourceWithDisposeControl::dispose_count);
   }
   i::Isolate::Current()->compilation_cache()->Clear();
+  if (HEAP->incremental_marking()->IsMarking()) {
+    HEAP->incremental_marking()->Abort();
+  }
   HEAP->CollectAllGarbage(i::Heap::kNoGCFlags);
   CHECK_EQ(1, TestAsciiResourceWithDisposeControl::dispose_calls);
-  CHECK_EQ(1, TestAsciiResource::dispose_count);
+  CHECK_EQ(1, TestAsciiResourceWithDisposeControl::dispose_count);
 }
 
 
 THREADED_TEST(StringConcat) {
   {
     v8::HandleScope scope;
     LocalContext env;
     const char* one_byte_string_1 = "function a_times_t";
     const char* two_byte_string_1 = "wo_plus_b(a, b) {return ";
     const char* one_byte_extern_1 = "a * 2 + b;} a_times_two_plus_b(4, 8) + ";
@@ skipping 14117 matching lines @@
   }
 
   i::Isolate::Current()->heap()->CollectAllGarbage(true);
   { i::Object* raw_map_cache = i::Isolate::Current()->context()->map_cache();
     if (raw_map_cache != i::Isolate::Current()->heap()->undefined_value()) {
       i::MapCache* map_cache = i::MapCache::cast(raw_map_cache);
       CHECK_GT(elements, map_cache->NumberOfElements());
     }
   }
 }