FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

deps: V8: cherry-pick 163d360 from upstream · nodejs/node@1c0ae10 · GitHub

/ node Public

Commit 1c0ae10

Browse files
ofrobots
authored and
MylesBorins
committed
deps: V8: cherry-pick 163d360 from upstream
Original commit message [heap] Fix memory leak in the remembered set. Empty slot set buckets can leak in the following scenarios. Scenario 1 (large object space): 1) A large array is allocated in the large object space. 2) The array is filled with old->new references, which allocates new slot set buckets. 3) The references are overwritten with smis or old space pointers, which make the slots set buckets empty. 4) Garbage collection (scavenge or mark-compact) iterates the slots set of the array and pre-frees the empty buckets. 5) Steps 2-4 repeated many times and leak arbitary many empty buckets. The fix to free empty buckets for large object space in mark-compact. Scenario 2 (no mark-compact): 1) A small array is allocated in the old space. 2) The array is filled with old->new references, which allocates new slot set buckets. 3) The references are overwritten with smis or old space pointers, which make the slots set buckets empty. 4) Scavenge iterates the slots set of the array and pre-frees the empty buckets. 5) Steps 2-4 repeated many times and leak arbitary many empty buckets. The fix to free empty buckets for swept pages in scavenger. Bug: v8:6800 TBR: mlippautz@chromium.org Change-Id: I48d94870f5acf4f6208858271886911c895a9126 Reviewed-on: https://chromium-review.googlesource.com/668442 Reviewed-by: Ulan Degenbaev <ulan@chromium.org> Commit-Queue: Ulan Degenbaev <ulan@chromium.org> Cr-Commit-Position: refs/heads/master@{#48041} PR-URL: #15664 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benedikt Meurer <benedikt.meurer@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent f68f572 commit 1c0ae10

5 files changed

Lines changed: 105 additions & 10 deletions

File tree

‎deps/v8/src/heap/heap.cc‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1814,7 +1814,11 @@ void Heap::Scavenge() {
18141814
ArrayBufferTracker::FreeDeadInNewSpace(this);
18151815

18161816
RememberedSet<OLD_TO_NEW>::IterateMemoryChunks(this, [](MemoryChunk* chunk) {
1817-
RememberedSet<OLD_TO_NEW>::PreFreeEmptyBuckets(chunk);
1817+
if (chunk->SweepingDone()) {
1818+
RememberedSet<OLD_TO_NEW>::FreeEmptyBuckets(chunk);
1819+
} else {
1820+
RememberedSet<OLD_TO_NEW>::PreFreeEmptyBuckets(chunk);
1821+
}
18181822
});
18191823

18201824
// Update how much has survived scavenge.

‎deps/v8/src/heap/remembered-set.h‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,19 @@ class RememberedSet : public AllStatic {
150150
}
151151
}
152152

153+
static int NumberOfPreFreedEmptyBuckets(MemoryChunk* chunk) {
154+
DCHECK(type == OLD_TO_NEW);
155+
int result = 0;
156+
SlotSet* slots = chunk->slot_set<type>();
157+
if (slots != nullptr) {
158+
size_t pages = (chunk->size() + Page::kPageSize - 1) / Page::kPageSize;
159+
for (size_t page = 0; page < pages; page++) {
160+
result += slots[page].NumberOfPreFreedEmptyBuckets();
161+
}
162+
}
163+
return result;
164+
}
165+
153166
static void PreFreeEmptyBuckets(MemoryChunk* chunk) {
154167
DCHECK(type == OLD_TO_NEW);
155168
SlotSet* slots = chunk->slot_set<type>();
@@ -161,6 +174,18 @@ class RememberedSet : public AllStatic {
161174
}
162175
}
163176

177+
static void FreeEmptyBuckets(MemoryChunk* chunk) {
178+
DCHECK(type == OLD_TO_NEW);
179+
SlotSet* slots = chunk->slot_set<type>();
180+
if (slots != nullptr) {
181+
size_t pages = (chunk->size() + Page::kPageSize - 1) / Page::kPageSize;
182+
for (size_t page = 0; page < pages; page++) {
183+
slots[page].FreeEmptyBuckets();
184+
slots[page].FreeToBeFreedBuckets();
185+
}
186+
}
187+
}
188+
164189
// Given a page and a typed slot in that page, this function adds the slot
165190
// to the remembered set.
166191
static void InsertTyped(Page* page, Address host_addr, SlotType slot_type,

‎deps/v8/src/heap/slot-set.h‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -225,32 +225,41 @@ class SlotSet : public Malloced {
225225
return new_count;
226226
}
227227

228+
int NumberOfPreFreedEmptyBuckets() {
229+
base::LockGuard<base::Mutex> guard(&to_be_freed_buckets_mutex_);
230+
return static_cast<int>(to_be_freed_buckets_.size());
231+
}
232+
228233
void PreFreeEmptyBuckets() {
229234
for (int bucket_index = 0; bucket_index < kBuckets; bucket_index++) {
230235
Bucket bucket = LoadBucket(&buckets_[bucket_index]);
231236
if (bucket != nullptr) {
232-
bool found_non_empty_cell = false;
233-
int cell_offset = bucket_index * kBitsPerBucket;
234-
for (int i = 0; i < kCellsPerBucket; i++, cell_offset += kBitsPerCell) {
235-
if (LoadCell(&bucket[i])) {
236-
found_non_empty_cell = true;
237-
break;
238-
}
239-
}
240-
if (!found_non_empty_cell) {
237+
if (IsEmptyBucket(bucket)) {
241238
PreFreeEmptyBucket(bucket_index);
242239
}
243240
}
244241
}
245242
}
246243

244+
void FreeEmptyBuckets() {
245+
for (int bucket_index = 0; bucket_index < kBuckets; bucket_index++) {
246+
Bucket bucket = LoadBucket(&buckets_[bucket_index]);
247+
if (bucket != nullptr) {
248+
if (IsEmptyBucket(bucket)) {
249+
ReleaseBucket(bucket_index);
250+
}
251+
}
252+
}
253+
}
254+
247255
void FreeToBeFreedBuckets() {
248256
base::LockGuard<base::Mutex> guard(&to_be_freed_buckets_mutex_);
249257
while (!to_be_freed_buckets_.empty()) {
250258
Bucket top = to_be_freed_buckets_.top();
251259
to_be_freed_buckets_.pop();
252260
DeleteArray<uint32_t>(top);
253261
}
262+
DCHECK_EQ(0u, to_be_freed_buckets_.size());
254263
}
255264

256265
private:
@@ -313,6 +322,15 @@ class SlotSet : public Malloced {
313322
}
314323
}
315324

325+
bool IsEmptyBucket(Bucket bucket) {
326+
for (int i = 0; i < kCellsPerBucket; i++) {
327+
if (LoadCell(&bucket[i])) {
328+
return false;
329+
}
330+
}
331+
return true;
332+
}
333+
316334
template <AccessMode access_mode = AccessMode::ATOMIC>
317335
bool SwapInNewBucket(Bucket* bucket, Bucket value) {
318336
if (access_mode == AccessMode::ATOMIC) {

‎deps/v8/src/heap/spaces.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3308,6 +3308,7 @@ void LargeObjectSpace::ClearMarkingStateOfLiveObjects() {
33083308
Marking::MarkWhite(
33093309
ObjectMarking::MarkBitFrom(obj, MarkingState::Internal(obj)));
33103310
MemoryChunk* chunk = MemoryChunk::FromAddress(obj->address());
3311+
RememberedSet<OLD_TO_NEW>::FreeEmptyBuckets(chunk);
33113312
chunk->ResetProgressBar();
33123313
MarkingState::Internal(chunk).SetLiveBytes(0);
33133314
}

‎deps/v8/test/cctest/heap/test-heap.cc‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6237,6 +6237,53 @@ HEAP_TEST(Regress5831) {
62376237
CHECK(chunk->NeverEvacuate());
62386238
}
62396239

6240+
TEST(Regress6800) {
6241+
CcTest::InitializeVM();
6242+
Isolate* isolate = CcTest::i_isolate();
6243+
HandleScope handle_scope(isolate);
6244+
6245+
const int kRootLength = 1000;
6246+
Handle<FixedArray> root =
6247+
isolate->factory()->NewFixedArray(kRootLength, TENURED);
6248+
{
6249+
HandleScope inner_scope(isolate);
6250+
Handle<FixedArray> new_space_array = isolate->factory()->NewFixedArray(1);
6251+
for (int i = 0; i < kRootLength; i++) {
6252+
root->set(i, *new_space_array);
6253+
}
6254+
for (int i = 0; i < kRootLength; i++) {
6255+
root->set(i, CcTest::heap()->undefined_value());
6256+
}
6257+
}
6258+
CcTest::CollectGarbage(NEW_SPACE);
6259+
CHECK_EQ(0, RememberedSet<OLD_TO_NEW>::NumberOfPreFreedEmptyBuckets(
6260+
MemoryChunk::FromAddress(root->address())));
6261+
}
6262+
6263+
TEST(Regress6800LargeObject) {
6264+
CcTest::InitializeVM();
6265+
Isolate* isolate = CcTest::i_isolate();
6266+
HandleScope handle_scope(isolate);
6267+
6268+
const int kRootLength = i::kMaxRegularHeapObjectSize / kPointerSize;
6269+
Handle<FixedArray> root =
6270+
isolate->factory()->NewFixedArray(kRootLength, TENURED);
6271+
CcTest::heap()->lo_space()->Contains(*root);
6272+
{
6273+
HandleScope inner_scope(isolate);
6274+
Handle<FixedArray> new_space_array = isolate->factory()->NewFixedArray(1);
6275+
for (int i = 0; i < kRootLength; i++) {
6276+
root->set(i, *new_space_array);
6277+
}
6278+
for (int i = 0; i < kRootLength; i++) {
6279+
root->set(i, CcTest::heap()->undefined_value());
6280+
}
6281+
}
6282+
CcTest::CollectGarbage(OLD_SPACE);
6283+
CHECK_EQ(0, RememberedSet<OLD_TO_NEW>::NumberOfPreFreedEmptyBuckets(
6284+
MemoryChunk::FromAddress(root->address())));
6285+
}
6286+
62406287
HEAP_TEST(RegressMissingWriteBarrierInAllocate) {
62416288
if (!FLAG_incremental_marking) return;
62426289
FLAG_black_allocation = true;

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL