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

test/cctest/test-api.cc

Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index 5186e41f507ce7dee574ce03a8bdf6210da2430c..9fd7aacb7ef13e11739ce45c1ce66098c1873edb 100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -331,16 +331,14 @@ static uint16_t* AsciiToTwoByteString(const char* source) {
 
 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 {
@@ -353,23 +351,18 @@ class TestResource: public String::ExternalStringResource {
  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 {
@@ -382,20 +375,18 @@ class TestAsciiResource: public String::ExternalAsciiStringResource {
  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();
@@ -405,37 +396,46 @@ THREADED_TEST(ScriptUsingStringResource) {
     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;
@@ -444,23 +444,27 @@ THREADED_TEST(ScriptMakingExternalString) {
     // 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;
@@ -470,18 +474,21 @@ THREADED_TEST(ScriptMakingExternalAsciiString) {
     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);
 }
 
 
@@ -605,49 +612,52 @@ THREADED_TEST(UsingExternalAsciiString) {
 
 
 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() {
@@ -659,6 +669,7 @@ class TestAsciiResourceWithDisposeControl: public TestAsciiResource {
 };
 
 
+int TestAsciiResourceWithDisposeControl::dispose_count = 0;
 int TestAsciiResourceWithDisposeControl::dispose_calls = 0;
 
 
@@ -666,7 +677,7 @@ 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);
   {
@@ -678,15 +689,18 @@ TEST(ExternalStringWithDisposeHandling) {
     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);
@@ -699,12 +713,15 @@ TEST(ExternalStringWithDisposeHandling) {
     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);
 }