[bmalloc] Merging of XLargeRanges can leak the upper range
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Aug 2016 18:43:15 +0000 (18:43 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 3 Aug 2016 18:43:15 +0000 (18:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=160403

Reviewed by Michael Saboff.

* bmalloc/Heap.cpp:
(bmalloc::Heap::scavengeLargeObjects): Don't use removePhysical().
Recorded physical size is a performance optimization. It is not the
truth. So it might be zero even if a range contains physical pages.

Instead, iterate each range in the map unconditionally.

The map can shrink when we release the lock, so we must clamp our
iterator each time through the loop.

The map can grow when we release the lock, but we don't care because
growth restarts the scavenger from the beginning.

* bmalloc/XLargeMap.cpp:
(bmalloc::XLargeMap::removePhysical): Deleted. Not used anymore.

* bmalloc/XLargeMap.h:
(bmalloc::XLargeMap::ranges): Added direct access for the sake of
scavengeLargeObjects. (This violates our naming conventions -- I'll do
a rename in a follow-up patch.)

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@204091 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/bmalloc/ChangeLog
Source/bmalloc/bmalloc/Heap.cpp
Source/bmalloc/bmalloc/XLargeMap.cpp
Source/bmalloc/bmalloc/XLargeMap.h

index b576f93..d24263f 100644 (file)
@@ -1,3 +1,31 @@
+2016-08-03  Geoffrey Garen  <ggaren@apple.com>
+
+        [bmalloc] Merging of XLargeRanges can leak the upper range
+        https://bugs.webkit.org/show_bug.cgi?id=160403
+
+        Reviewed by Michael Saboff.
+
+        * bmalloc/Heap.cpp:
+        (bmalloc::Heap::scavengeLargeObjects): Don't use removePhysical().
+        Recorded physical size is a performance optimization. It is not the
+        truth. So it might be zero even if a range contains physical pages.
+
+        Instead, iterate each range in the map unconditionally.
+
+        The map can shrink when we release the lock, so we must clamp our
+        iterator each time through the loop.
+
+        The map can grow when we release the lock, but we don't care because
+        growth restarts the scavenger from the beginning.
+
+        * bmalloc/XLargeMap.cpp:
+        (bmalloc::XLargeMap::removePhysical): Deleted. Not used anymore.
+
+        * bmalloc/XLargeMap.h:
+        (bmalloc::XLargeMap::ranges): Added direct access for the sake of
+        scavengeLargeObjects. (This violates our naming conventions -- I'll do
+        a rename in a follow-up patch.)
+
 2016-07-13  Enrica Casucci  <enrica@apple.com>
 
         Update supported platforms in xcconfig files to match the sdk names.
index 3917c95..ac0d7de 100644 (file)
@@ -131,13 +131,16 @@ void Heap::scavengeSmallPages(std::unique_lock<StaticMutex>& lock, std::chrono::
 
 void Heap::scavengeLargeObjects(std::unique_lock<StaticMutex>& lock, std::chrono::milliseconds sleepDuration)
 {
-    while (XLargeRange range = m_largeFree.removePhysical()) {
+    auto& ranges = m_largeFree.ranges();
+    for (size_t i = ranges.size(); i-- > 0; i = std::min(i, ranges.size())) {
+        auto range = ranges.pop(i);
+
         lock.unlock();
         vmDeallocatePhysicalPagesSloppy(range.begin(), range.size());
         lock.lock();
-        
+
         range.setPhysicalSize(0);
-        m_largeFree.add(range);
+        ranges.push(range);
 
         waitUntilFalse(lock, sleepDuration, m_isAllocatingPages);
     }
index ab38ffe..99b8b79 100644 (file)
@@ -76,16 +76,4 @@ void XLargeMap::add(const XLargeRange& range)
     m_free.push(merged);
 }
 
-XLargeRange XLargeMap::removePhysical()
-{
-    auto it = std::find_if(m_free.begin(), m_free.end(), [](const XLargeRange& range) {
-        return range.physicalSize();
-    });
-
-    if (it == m_free.end())
-        return XLargeRange();
-
-    return m_free.pop(it);
-}
-
 } // namespace bmalloc
index 4f168e9..cd2f16d 100644 (file)
@@ -36,7 +36,7 @@ class XLargeMap {
 public:
     void add(const XLargeRange&);
     XLargeRange remove(size_t alignment, size_t);
-    XLargeRange removePhysical();
+    Vector<XLargeRange>& ranges() { return m_free; }
 
 private:
     Vector<XLargeRange> m_free;