Author: [email protected]
Date: Tue May 5 01:38:54 2009
New Revision: 1854
Modified:
branches/1.1/src/api.cc
branches/1.1/src/mark-compact.cc
branches/1.1/src/mark-compact.h
branches/1.1/test/cctest/test-strings.cc
Log:
Push the fix for http://crbug.com/9746 to the 1.1 branch.
Review URL: http://codereview.chromium.org/109006
Modified: branches/1.1/src/api.cc
==============================================================================
--- branches/1.1/src/api.cc (original)
+++ branches/1.1/src/api.cc Tue May 5 01:38:54 2009
@@ -2373,7 +2373,7 @@
const char* v8::V8::GetVersion() {
- return "1.1.10.6";
+ return "1.1.10.7";
}
Modified: branches/1.1/src/mark-compact.cc
==============================================================================
--- branches/1.1/src/mark-compact.cc (original)
+++ branches/1.1/src/mark-compact.cc Tue May 5 01:38:54 2009
@@ -562,18 +562,57 @@
}
-void MarkCompactCollector::ProcessRoots(RootMarkingVisitor* visitor) {
- // Mark the heap roots gray, including global variables, stack variables,
- // etc.
- Heap::IterateStrongRoots(visitor);
+class SymbolMarkingVisitor : public ObjectVisitor {
+ public:
+ void VisitPointers(Object** start, Object** end) {
+ MarkingVisitor marker;
+ for (Object** p = start; p < end; p++) {
+ if (!(*p)->IsHeapObject()) continue;
+
+ HeapObject* object = HeapObject::cast(*p);
+ // If the object is marked, we have marked or are in the process
+ // of marking subparts.
+ if (object->IsMarked()) continue;
+
+ // The object is unmarked, we do not need to unmark to use its
+ // map.
+ Map* map = object->map();
+ object->IterateBody(map->instance_type(),
+ object->SizeFromMap(map),
+ &marker);
+ }
+ }
+};
- // Take care of the symbol table specially.
+
+void MarkCompactCollector::MarkSymbolTable() {
+ // Objects reachable from symbols are marked as live so as to ensure
+ // that if the symbol itself remains alive after GC for any reason,
+ // and if it is a sliced string or a cons string backed by an
+ // external string (even indirectly), then the external string does
+ // not receive a weak reference callback.
SymbolTable* symbol_table = SymbolTable::cast(Heap::symbol_table());
- // 1. Mark the prefix of the symbol table gray.
- symbol_table->IteratePrefix(visitor);
- // 2. Mark the symbol table black (ie, do not push it on the marking
stack
- // or mark it overflowed).
+ // Mark the symbol table itself.
SetMark(symbol_table);
+ // Explicitly mark the prefix.
+ MarkingVisitor marker;
+ symbol_table->IteratePrefix(&marker);
+ ProcessMarkingStack(&marker);
+ // Mark subparts of the symbols but not the symbols themselves
+ // (unless reachable from another symbol).
+ SymbolMarkingVisitor symbol_marker;
+ symbol_table->IterateElements(&symbol_marker);
+ ProcessMarkingStack(&marker);
+}
+
+
+void MarkCompactCollector::MarkRoots(RootMarkingVisitor* visitor) {
+ // Mark the heap roots including global variables, stack variables,
+ // etc., and all objects reachable from them.
+ Heap::IterateStrongRoots(visitor);
+
+ // Handle the symbol table specially.
+ MarkSymbolTable();
// There may be overflowed objects in the heap. Visit them now.
while (marking_stack.overflowed()) {
@@ -715,7 +754,7 @@
ASSERT(!marking_stack.overflowed());
RootMarkingVisitor root_visitor;
- ProcessRoots(&root_visitor);
+ MarkRoots(&root_visitor);
// The objects reachable from the roots are marked black, unreachable
// objects are white. Mark objects reachable from object groups with at
@@ -762,21 +801,22 @@
#ifdef DEBUG
-void MarkCompactCollector::UpdateLiveObjectCount(HeapObject* obj) {
- live_bytes_ += obj->Size();
+void MarkCompactCollector::UpdateLiveObjectCount(HeapObject* obj, int
scale) {
+ ASSERT(scale == -1 || scale == 1);
+ live_bytes_ += obj->Size() * scale;
if (Heap::new_space()->Contains(obj)) {
- live_young_objects_++;
+ live_young_objects_ += scale;
} else if (Heap::map_space()->Contains(obj)) {
ASSERT(obj->IsMap());
- live_map_objects_++;
+ live_map_objects_ += scale;
} else if (Heap::old_pointer_space()->Contains(obj)) {
- live_old_pointer_objects_++;
+ live_old_pointer_objects_ += scale;
} else if (Heap::old_data_space()->Contains(obj)) {
- live_old_data_objects_++;
+ live_old_data_objects_ += scale;
} else if (Heap::code_space()->Contains(obj)) {
- live_code_objects_++;
+ live_code_objects_ += scale;
} else if (Heap::lo_space()->Contains(obj)) {
- live_lo_objects_++;
+ live_lo_objects_ +=scale;
} else {
UNREACHABLE();
}
Modified: branches/1.1/src/mark-compact.h
==============================================================================
--- branches/1.1/src/mark-compact.h (original)
+++ branches/1.1/src/mark-compact.h Tue May 5 01:38:54 2009
@@ -142,6 +142,7 @@
friend class RootMarkingVisitor;
friend class MarkingVisitor;
+ friend class UnmarkingVisitor;
// Marking operations for objects reachable from roots.
static void MarkLiveObjects();
@@ -155,7 +156,7 @@
static inline void SetMark(HeapObject* obj) {
tracer_->increment_marked_count();
#ifdef DEBUG
- UpdateLiveObjectCount(obj);
+ UpdateLiveObjectCount(obj, 1);
#endif
obj->SetMark();
}
@@ -172,7 +173,11 @@
static void MarkDescriptorArray(DescriptorArray* descriptors);
// Mark the heap roots and all objects reachable from them.
- static void ProcessRoots(RootMarkingVisitor* visitor);
+ static void MarkRoots(RootMarkingVisitor* visitor);
+
+ // Mark the symbol table specially. References to symbols from the
+ // symbol table are weak.
+ static void MarkSymbolTable();
// Mark objects in object groups that have at least one object in the
// group marked.
@@ -202,7 +207,10 @@
static bool MustBeMarked(Object** p);
#ifdef DEBUG
- static void UpdateLiveObjectCount(HeapObject* obj);
+ // The scale argument is positive 1 if we are marking an object and
+ // -1 if we are clearing the mark bit of an object that we didn't
+ // actually want marked.
+ static void UpdateLiveObjectCount(HeapObject* obj, int scale);
#endif
// We sweep the large object space in the same way whether we are
Modified: branches/1.1/test/cctest/test-strings.cc
==============================================================================
--- branches/1.1/test/cctest/test-strings.cc (original)
+++ branches/1.1/test/cctest/test-strings.cc Tue May 5 01:38:54 2009
@@ -9,6 +9,7 @@
#include "v8.h"
+#include "api.h"
#include "factory.h"
#include "cctest.h"
#include "zone-inl.h"
@@ -391,9 +392,19 @@
class TwoByteResource: public v8::String::ExternalStringResource {
public:
- explicit TwoByteResource(const uint16_t* data, size_t length)
- : data_(data), length_(length) { }
- virtual ~TwoByteResource() { }
+ TwoByteResource(const uint16_t* data, size_t length, bool* destructed)
+ : data_(data), length_(length), destructed_(destructed) {
+ if (destructed_ != NULL) {
+ *destructed_ = false;
+ }
+ }
+
+ virtual ~TwoByteResource() {
+ if (destructed_ != NULL) {
+ CHECK(!*destructed_);
+ *destructed_ = true;
+ }
+ }
const uint16_t* data() const { return data_; }
size_t length() const { return length_; }
@@ -401,6 +412,7 @@
private:
const uint16_t* data_;
size_t length_;
+ bool* destructed_;
};
@@ -420,7 +432,8 @@
// Allocate an external string resource and external string.
TwoByteResource* resource = new TwoByteResource(two_byte_data,
- two_byte_length);
+ two_byte_length,
+ NULL);
Handle<String> string = Factory::NewExternalStringFromTwoByte(resource);
Vector<const char> one_byte_vec = CStrVector(one_byte_data);
Handle<String> compare = Factory::NewStringFromAscii(one_byte_vec);
@@ -443,4 +456,88 @@
CHECK_EQ(false, compare->Equals(*string));
CHECK_EQ(false, string->Equals(Heap::empty_string()));
#endif // !defined(DEBUG)
+}
+
+
+// Regression test case for http://crbug.com/9746. The problem was
+// that when we marked objects reachable only through weak pointers,
+// we ended up keeping a sliced symbol alive, even though we already
+// invoked the weak callback on the underlying external string thus
+// deleting its resource.
+TEST(Regress9746) {
+ InitializeVM();
+
+ // Setup lengths that guarantee we'll get slices instead of simple
+ // flat strings.
+ static const int kFullStringLength = String::kMinNonFlatLength * 2;
+ static const int kSliceStringLength = String::kMinNonFlatLength + 1;
+
+ uint16_t* source = new uint16_t[kFullStringLength];
+ for (int i = 0; i < kFullStringLength; i++) source[i] = '1';
+ char* key = new char[kSliceStringLength];
+ for (int i = 0; i < kSliceStringLength; i++) key[i] = '1';
+
+ // Allocate an external string resource that keeps track of when it
+ // is destructed.
+ bool resource_destructed = false;
+ TwoByteResource* resource =
+ new TwoByteResource(source, kFullStringLength, &resource_destructed);
+
+ {
+ v8::HandleScope scope;
+
+ // Allocate an external string resource and external string. We
+ // have to go through the API to get the weak handle and the
+ // automatic destruction going.
+ Handle<String> string =
+ v8::Utils::OpenHandle(*v8::String::NewExternal(resource));
+
+ // Create a slice of the external string.
+ Handle<String> slice =
+ Factory::NewStringSlice(string, 0, kSliceStringLength);
+ CHECK_EQ(kSliceStringLength, slice->length());
+ CHECK(StringShape(*slice).IsSliced());
+
+ // Make sure the slice ends up in old space so we can morph it
+ // into a symbol.
+ while (Heap::InNewSpace(*slice)) {
+ Heap::PerformScavenge();
+ }
+
+ // Force the slice into the symbol table.
+ slice = Factory::SymbolFromString(slice);
+ CHECK(slice->IsSymbol());
+ CHECK(StringShape(*slice).IsSliced());
+
+ Handle<String> buffer(Handle<SlicedString>::cast(slice)->buffer());
+ CHECK(StringShape(*buffer).IsExternal());
+ CHECK(StringShape(*buffer).IsTwoByteRepresentation());
+
+ // Finally, base a script on the slice of the external string and
+ // get its wrapper. This allocated yet another weak handle that
+ // indirectly refers to the external string.
+ Handle<Script> script = Factory::NewScript(slice);
+ Handle<JSObject> wrapper = GetScriptWrapper(script);
+ }
+
+ // When we collect all garbage, we cannot get rid of the sliced
+ // symbol entry in the symbol table because it is used by the script
+ // kept alive by the weak wrapper. Make sure we don't destruct the
+ // external string.
+ Heap::CollectAllGarbage();
+ CHECK(!resource_destructed);
+
+ // Make sure the sliced symbol is still in the table.
+ v8::HandleScope scope;
+ Vector<const char> vector(key, kSliceStringLength);
+ Handle<String> symbol = Factory::LookupSymbol(vector);
+ CHECK(StringShape(*symbol).IsSliced());
+
+ // Make sure the buffer is still a two-byte external string.
+ Handle<String> buffer(Handle<SlicedString>::cast(symbol)->buffer());
+ CHECK(StringShape(*buffer).IsExternal());
+ CHECK(StringShape(*buffer).IsTwoByteRepresentation());
+
+ delete[] source;
+ delete[] key;
}
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
