Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(364)

Unified Diff: src/heap/spaces.cc

Issue 2689683002: [heap] Fix address space leak in Unmapper (Closed)
Patch Set: Windows compile fixes Created 3 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/heap/spaces.h ('k') | test/unittests/BUILD.gn » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/heap/spaces.cc
diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc
index b6d41e03a1e1c276888f43b2b698e262f9307529..f05dae4f911c31a07a8e38e1c79e07b7b619a7c8 100644
--- a/src/heap/spaces.cc
+++ b/src/heap/spaces.cc
@@ -335,7 +335,7 @@ class MemoryAllocator::Unmapper::UnmapFreeMemoryTask : public v8::Task {
private:
// v8::Task overrides.
void Run() override {
- unmapper_->PerformFreeMemoryOnQueuedChunks();
+ unmapper_->PerformFreeMemoryOnQueuedChunks<FreeMode::kUncommitPooled>();
unmapper_->pending_unmapping_tasks_semaphore_.Signal();
}
@@ -350,7 +350,7 @@ void MemoryAllocator::Unmapper::FreeQueuedChunks() {
new UnmapFreeMemoryTask(this), v8::Platform::kShortRunningTask);
concurrent_unmapping_tasks_active_++;
} else {
- PerformFreeMemoryOnQueuedChunks();
+ PerformFreeMemoryOnQueuedChunks<FreeMode::kUncommitPooled>();
}
}
@@ -364,6 +364,7 @@ bool MemoryAllocator::Unmapper::WaitUntilCompleted() {
return waited;
}
+template <MemoryAllocator::Unmapper::FreeMode mode>
void MemoryAllocator::Unmapper::PerformFreeMemoryOnQueuedChunks() {
MemoryChunk* chunk = nullptr;
// Regular chunks.
@@ -372,6 +373,14 @@ void MemoryAllocator::Unmapper::PerformFreeMemoryOnQueuedChunks() {
allocator_->PerformFreeMemory(chunk);
if (pooled) AddMemoryChunkSafe<kPooled>(chunk);
}
+ if (mode == MemoryAllocator::Unmapper::FreeMode::kReleasePooled) {
+ // The previous loop uncommitted any pages marked as pooled and added them
+ // to the pooled list. In case of kReleasePooled we need to free them
+ // though.
+ while ((chunk = GetMemoryChunkSafe<kPooled>()) != nullptr) {
+ allocator_->Free<MemoryAllocator::kAlreadyPooled>(chunk);
+ }
+ }
// Non-regular chunks.
while ((chunk = GetMemoryChunkSafe<kNonRegular>()) != nullptr) {
allocator_->PerformFreeMemory(chunk);
@@ -382,7 +391,10 @@ void MemoryAllocator::Unmapper::TearDown() {
WaitUntilCompleted();
ReconsiderDelayedChunks();
CHECK(delayed_regular_chunks_.empty());
- PerformFreeMemoryOnQueuedChunks();
+ PerformFreeMemoryOnQueuedChunks<FreeMode::kReleasePooled>();
+ for (int i = 0; i < kNumberOfChunkQueues; i++) {
+ DCHECK(chunks_[i].empty());
+ }
}
void MemoryAllocator::Unmapper::ReconsiderDelayedChunks() {
@@ -909,6 +921,11 @@ void MemoryAllocator::Free(MemoryChunk* chunk) {
PreFreeMemory(chunk);
PerformFreeMemory(chunk);
break;
+ case kAlreadyPooled:
+ // Pooled pages cannot be touched anymore as their memory is uncommitted.
+ FreeMemory(chunk->address(), static_cast<size_t>(MemoryChunk::kPageSize),
+ Executability::NOT_EXECUTABLE);
+ break;
case kPooledAndQueue:
DCHECK_EQ(chunk->size(), static_cast<size_t>(MemoryChunk::kPageSize));
DCHECK_EQ(chunk->executable(), NOT_EXECUTABLE);
@@ -919,13 +936,14 @@ void MemoryAllocator::Free(MemoryChunk* chunk) {
// The chunks added to this queue will be freed by a concurrent thread.
unmapper()->AddMemoryChunkSafe(chunk);
break;
- default:
- UNREACHABLE();
}
}
template void MemoryAllocator::Free<MemoryAllocator::kFull>(MemoryChunk* chunk);
+template void MemoryAllocator::Free<MemoryAllocator::kAlreadyPooled>(
+ MemoryChunk* chunk);
+
template void MemoryAllocator::Free<MemoryAllocator::kPreFreeAndQueue>(
MemoryChunk* chunk);
« no previous file with comments | « src/heap/spaces.h ('k') | test/unittests/BUILD.gn » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698