Bug 1472124 - Fix memory leak in GC incremental marking validator r=anba
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 04 Jul 2018 09:48:02 +0100
changeset 424944 a355ad34b92b9b75bce1667db40a62063ead215f
parent 424943 ff25e66da36e4c95363e684880085f99d845da2a
child 424978 796893f4d2f5555c8e29658bae05b98cffad0fe1
push id104945
push userjcoppeard@mozilla.com
push dateWed, 04 Jul 2018 08:58:00 +0000
treeherdermozilla-inbound@a355ad34b92b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1472124
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1472124 - Fix memory leak in GC incremental marking validator r=anba
js/src/gc/GC.cpp
--- a/js/src/gc/GC.cpp
+++ b/js/src/gc/GC.cpp
@@ -4620,42 +4620,32 @@ struct GCChunkHasher {
         return k == l;
     }
 };
 
 class js::gc::MarkingValidator
 {
   public:
     explicit MarkingValidator(GCRuntime* gc);
-    ~MarkingValidator();
     void nonIncrementalMark(AutoGCSession& session);
     void validate();
 
   private:
     GCRuntime* gc;
     bool initialized;
 
-    typedef HashMap<Chunk*, ChunkBitmap*, GCChunkHasher, SystemAllocPolicy> BitmapMap;
+    using BitmapMap = HashMap<Chunk*, UniquePtr<ChunkBitmap>, GCChunkHasher, SystemAllocPolicy>;
     BitmapMap map;
 };
 
 js::gc::MarkingValidator::MarkingValidator(GCRuntime* gc)
   : gc(gc),
     initialized(false)
 {}
 
-js::gc::MarkingValidator::~MarkingValidator()
-{
-    if (!map.initialized())
-        return;
-
-    for (BitmapMap::Range r(map.all()); !r.empty(); r.popFront())
-        js_delete(r.front().value());
-}
-
 void
 js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session)
 {
     /*
      * Perform a non-incremental mark for all collecting zones and record
      * the results for later comparison.
      *
      * Currently this does not validate gray marking.
@@ -4672,22 +4662,23 @@ js::gc::MarkingValidator::nonIncremental
     /* Wait for off-thread parsing which can allocate. */
     HelperThreadState().waitForAllThreads();
 
     /* Save existing mark bits. */
     {
         AutoLockGC lock(runtime);
         for (auto chunk = gc->allNonEmptyChunks(lock); !chunk.done(); chunk.next()) {
             ChunkBitmap* bitmap = &chunk->bitmap;
-            ChunkBitmap* entry = js_new<ChunkBitmap>();
+            auto entry = MakeUnique<ChunkBitmap>();
             if (!entry)
                 return;
 
             memcpy((void*)entry->bitmap, (void*)bitmap->bitmap, sizeof(bitmap->bitmap));
-            if (!map.putNew(chunk, entry))
+
+            if (!map.putNew(chunk, std::move(entry)))
                 return;
         }
     }
 
     /*
      * Temporarily clear the weakmaps' mark flags for the compartments we are
      * collecting.
      */
@@ -4778,17 +4769,17 @@ js::gc::MarkingValidator::nonIncremental
         gc->marker.setMarkColorBlack();
     }
 
     /* Take a copy of the non-incremental mark state and restore the original. */
     {
         AutoLockGC lock(runtime);
         for (auto chunk = gc->allNonEmptyChunks(lock); !chunk.done(); chunk.next()) {
             ChunkBitmap* bitmap = &chunk->bitmap;
-            ChunkBitmap* entry = map.lookup(chunk)->value();
+            ChunkBitmap* entry = map.lookup(chunk)->value().get();
             Swap(*entry, *bitmap);
         }
     }
 
     for (GCZonesIter zone(runtime); !zone.done(); zone.next()) {
         WeakMapBase::unmarkZone(zone);
         AutoEnterOOMUnsafeRegion oomUnsafe;
         if (!zone->gcWeakKeys().clear())
@@ -4821,17 +4812,17 @@ js::gc::MarkingValidator::validate()
     gc->waitBackgroundSweepEnd();
 
     AutoLockGC lock(gc->rt);
     for (auto chunk = gc->allNonEmptyChunks(lock); !chunk.done(); chunk.next()) {
         BitmapMap::Ptr ptr = map.lookup(chunk);
         if (!ptr)
             continue;  /* Allocated after we did the non-incremental mark. */
 
-        ChunkBitmap* bitmap = ptr->value();
+        ChunkBitmap* bitmap = ptr->value().get();
         ChunkBitmap* incBitmap = &chunk->bitmap;
 
         for (size_t i = 0; i < ArenasPerChunk; i++) {
             if (chunk->decommittedArenas.get(i))
                 continue;
             Arena* arena = &chunk->arenas[i];
             if (!arena->allocated())
                 continue;