Revision: 25032
Author: [email protected]
Date: Fri Oct 31 13:11:30 2014 UTC
Log: Clear old backing store of WeakCollection on updates.
Not clearing can lead to a crash under following conditions:
1. Backing store of a weak map is allocated in large object space.
2. The backing store is marked incrementaly via the weak map.
3. The weak map is updated and gets a new backing store.
4. The store buffer overflows and marks the chunk of the old backing store
as
"scan on scavenge."
5. Mark-compact collection kills some elements of the weak map. Note that
the
old backing store survives because it was marked incrementally, but its dead
elements are not cleared.
6. Scavenger iterates over the old backing store, tries to move a dead
object
and crashes.
BUG=v8:3631
LOG=N
TEST=cctest/test-heap/Regress3631
[email protected]
Review URL: https://codereview.chromium.org/686783003
https://code.google.com/p/v8/source/detail?r=25032
Modified:
/branches/bleeding_edge/src/runtime/runtime-collections.cc
/branches/bleeding_edge/test/cctest/test-heap.cc
=======================================
--- /branches/bleeding_edge/src/runtime/runtime-collections.cc Mon Oct 20
12:07:45 2014 UTC
+++ /branches/bleeding_edge/src/runtime/runtime-collections.cc Fri Oct 31
13:11:30 2014 UTC
@@ -288,6 +288,10 @@
Handle<ObjectHashTable> new_table =
ObjectHashTable::Remove(table, key, &was_present);
weak_collection->set_table(*new_table);
+ if (*table != *new_table) {
+ // Zap the old table since we didn't record slots for its elements.
+ table->FillWithHoles(0, table->length());
+ }
return isolate->heap()->ToBoolean(was_present);
}
@@ -304,6 +308,10 @@
RUNTIME_ASSERT(table->IsKey(*key));
Handle<ObjectHashTable> new_table = ObjectHashTable::Put(table, key,
value);
weak_collection->set_table(*new_table);
+ if (*table != *new_table) {
+ // Zap the old table since we didn't record slots for its elements.
+ table->FillWithHoles(0, table->length());
+ }
return *weak_collection;
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-heap.cc Mon Oct 27 11:57:58
2014 UTC
+++ /branches/bleeding_edge/test/cctest/test-heap.cc Fri Oct 31 13:11:30
2014 UTC
@@ -4606,6 +4606,46 @@
// when it calls heap->AdjustLiveBytes(...).
JSObject::MigrateToMap(o, map2);
}
+
+
+TEST(Regress3631) {
+ i::FLAG_expose_gc = true;
+ CcTest::InitializeVM();
+ v8::HandleScope scope(CcTest::isolate());
+ Isolate* isolate = CcTest::i_isolate();
+ Heap* heap = isolate->heap();
+ IncrementalMarking* marking = CcTest::heap()->incremental_marking();
+ v8::Local<v8::Value> result = CompileRun(
+ "var weak_map = new WeakMap();"
+ "var future_keys = [];"
+ "for (var i = 0; i < 50; i++) {"
+ " var key = {'k' : i + 0.1};"
+ " weak_map.set(key, 1);"
+ " future_keys.push({'x' : i + 0.2});"
+ "}"
+ "weak_map");
+ if (marking->IsStopped()) {
+ marking->Start();
+ }
+ // Incrementally mark the backing store.
+ Handle<JSObject> obj =
+ v8::Utils::OpenHandle(*v8::Handle<v8::Object>::Cast(result));
+ Handle<JSWeakCollection>
weak_map(reinterpret_cast<JSWeakCollection*>(*obj));
+ while (!Marking::IsBlack(
+ Marking::MarkBitFrom(HeapObject::cast(weak_map->table()))) &&
+ !marking->IsStopped()) {
+ marking->Step(MB, IncrementalMarking::NO_GC_VIA_STACK_GUARD);
+ }
+ // Stash the backing store in a handle.
+ Handle<Object> save(weak_map->table(), isolate);
+ // The following line will update the backing store.
+ CompileRun(
+ "for (var i = 0; i < 50; i++) {"
+ " weak_map.set(future_keys[i], i);"
+ "}");
+ heap->incremental_marking()->set_should_hurry(true);
+ heap->CollectGarbage(OLD_POINTER_SPACE);
+}
#ifdef DEBUG
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
