[heap] Fix address space leak in Unmapper

src/heap/spaces.cc

Index: src/heap/spaces.cc
diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc
index b6d41e03a1e1c276888f43b2b698e262f9307529..20a1a685e271f462f3fdcf37d33ed0897350b4e5 100644
--- a/src/heap/spaces.cc
+++ b/src/heap/spaces.cc
@@ -382,7 +382,20 @@ void MemoryAllocator::Unmapper::TearDown() {
   WaitUntilCompleted();
   ReconsiderDelayedChunks();
   CHECK(delayed_regular_chunks_.empty());
-  PerformFreeMemoryOnQueuedChunks();
+  // Free any queued chunks. Note that we need to clear the pooled flag for
+  // regular chunks as they otherwise only get uncommitted.
+  MemoryChunk* chunk = nullptr;

[Hannes Payer (out of office), 2017/02/10 12:29:22]

Let's wrap that into a function with a proper name to immediately see the
difference to PerformFreeMemoryOnQueuedChunks.

[Michael Lippautz, 2017/02/10 13:34:12]

Done. Unified into a single function.

+  while ((chunk = GetMemoryChunkSafe<kRegular>()) != nullptr) {
+    chunk->ClearFlag(MemoryChunk::POOLED);
+    allocator_->PerformFreeMemory(chunk);
+  }
+  while ((chunk = GetMemoryChunkSafe<kPooled>()) != nullptr) {
+    // Already uncommitted memory.
+    allocator_->Free<MemoryAllocator::kAlreadyPooled>(chunk);
+  }
+  while ((chunk = GetMemoryChunkSafe<kNonRegular>()) != nullptr) {
+    allocator_->PerformFreeMemory(chunk);
+  }

[Hannes Payer (out of office), 2017/02/10 12:29:22]

Let's verify here that all lists are empty.

[Michael Lippautz, 2017/02/10 13:34:12]

Done.

 }
 
 void MemoryAllocator::Unmapper::ReconsiderDelayedChunks() {
@@ -909,6 +922,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,45 +937,20 @@ 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);
 
 template void MemoryAllocator::Free<MemoryAllocator::kPooledAndQueue>(
     MemoryChunk* chunk);
 
-template <MemoryAllocator::AllocationMode alloc_mode, typename SpaceType>
-Page* MemoryAllocator::AllocatePage(size_t size, SpaceType* owner,
-                                    Executability executable) {
-  MemoryChunk* chunk = nullptr;
-  if (alloc_mode == kPooled) {
-    DCHECK_EQ(size, static_cast<size_t>(MemoryChunk::kAllocatableMemory));
-    DCHECK_EQ(executable, NOT_EXECUTABLE);
-    chunk = AllocatePagePooled(owner);
-  }
-  if (chunk == nullptr) {
-    chunk = AllocateChunk(size, size, executable, owner);
-  }
-  if (chunk == nullptr) return nullptr;
-  return Page::Initialize(isolate_->heap(), chunk, executable, owner);
-}
-
-template Page*
-MemoryAllocator::AllocatePage<MemoryAllocator::kRegular, PagedSpace>(
-    size_t size, PagedSpace* owner, Executability executable);
-template Page*
-MemoryAllocator::AllocatePage<MemoryAllocator::kRegular, SemiSpace>(
-    size_t size, SemiSpace* owner, Executability executable);
-template Page*
-MemoryAllocator::AllocatePage<MemoryAllocator::kPooled, SemiSpace>(
-    size_t size, SemiSpace* owner, Executability executable);
-
 LargePage* MemoryAllocator::AllocateLargePage(size_t size,
                                               LargeObjectSpace* owner,
                                               Executability executable) {