DelayedReleaseScope is in the wrong place
authormhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Dec 2013 04:30:02 +0000 (04:30 +0000)
committermhahnenberg@apple.com <mhahnenberg@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 Dec 2013 04:30:02 +0000 (04:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=125876

Reviewed by Geoffrey Garen.

The DelayedReleaseScope needs to be around the free list sweeping in MarkedAllocator::tryAllocateHelper.
This location gives us a good safe point between getting ready to allocate  (i.e. identifying a non-empty
free list) and doing the actual allocation (popping the free list).

* heap/MarkedAllocator.cpp:
(JSC::MarkedAllocator::tryAllocateHelper):
(JSC::MarkedAllocator::allocateSlowCase):
(JSC::MarkedAllocator::addBlock):
* runtime/JSCellInlines.h:
(JSC::allocateCell):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/MarkedAllocator.cpp
Source/JavaScriptCore/runtime/JSCellInlines.h

index 1ec220a..2771bd2 100644 (file)
@@ -1,3 +1,21 @@
+2013-12-18  Mark Hahnenberg  <mhahnenberg@apple.com>
+
+        DelayedReleaseScope is in the wrong place
+        https://bugs.webkit.org/show_bug.cgi?id=125876
+
+        Reviewed by Geoffrey Garen.
+
+        The DelayedReleaseScope needs to be around the free list sweeping in MarkedAllocator::tryAllocateHelper. 
+        This location gives us a good safe point between getting ready to allocate  (i.e. identifying a non-empty 
+        free list) and doing the actual allocation (popping the free list).
+
+        * heap/MarkedAllocator.cpp:
+        (JSC::MarkedAllocator::tryAllocateHelper):
+        (JSC::MarkedAllocator::allocateSlowCase):
+        (JSC::MarkedAllocator::addBlock):
+        * runtime/JSCellInlines.h:
+        (JSC::allocateCell):
+
 2013-12-18  Gustavo Noronha Silva  <gns@gnome.org>
 
         [GTK][CMake] make libjavascriptcoregtk a public shared library again
index 02de791..7440208 100644 (file)
@@ -30,7 +30,11 @@ bool MarkedAllocator::isPagedOut(double deadline)
 
 inline void* MarkedAllocator::tryAllocateHelper(size_t bytes)
 {
-    if (!m_freeList.head) {
+    // We need a while loop to check the free list because the DelayedReleaseScope 
+    // could cause arbitrary code to execute and exhaust the free list that we 
+    // thought had elements in it.
+    while (!m_freeList.head) {
+        DelayedReleaseScope delayedReleaseScope(*m_markedSpace);
         if (m_currentBlock) {
             ASSERT(m_currentBlock == m_blocksToSweep);
             m_currentBlock->didConsumeFreeList();
@@ -59,7 +63,8 @@ inline void* MarkedAllocator::tryAllocateHelper(size_t bytes)
             return 0;
         }
     }
-    
+
+    ASSERT(m_freeList.head);
     MarkedBlock::FreeCell* head = m_freeList.head;
     m_freeList.head = head->next;
     ASSERT(head);
@@ -78,7 +83,6 @@ inline void* MarkedAllocator::tryAllocate(size_t bytes)
 void* MarkedAllocator::allocateSlowCase(size_t bytes)
 {
     ASSERT(m_heap->vm()->currentThreadIsHoldingAPILock());
-    DelayedReleaseScope delayedReleaseScope(*m_markedSpace);
 #if COLLECT_ON_EVERY_ALLOCATION
     if (!m_heap->isDeferred())
         m_heap->collectAllGarbage();
@@ -126,6 +130,8 @@ MarkedBlock* MarkedAllocator::allocateBlock(size_t bytes)
 
 void MarkedAllocator::addBlock(MarkedBlock* block)
 {
+    // Satisfy the ASSERT in MarkedBlock::sweep.
+    DelayedReleaseScope delayedReleaseScope(*m_markedSpace);
     ASSERT(!m_currentBlock);
     ASSERT(!m_freeList.head);
     
index 139b862..f7c8446 100644 (file)
@@ -88,10 +88,6 @@ void* allocateCell(Heap& heap, size_t size)
 {
     ASSERT(!DisallowGC::isGCDisallowedOnCurrentThread());
     ASSERT(size >= sizeof(T));
-#if ENABLE(GC_VALIDATION)
-    ASSERT(!heap.vm()->isInitializingObject());
-    heap.vm()->setInitializingObjectClass(T::info());
-#endif
     JSCell* result = 0;
     if (T::needsDestruction && T::hasImmortalStructure)
         result = static_cast<JSCell*>(heap.allocateWithImmortalStructureDestructor(size));
@@ -99,6 +95,10 @@ void* allocateCell(Heap& heap, size_t size)
         result = static_cast<JSCell*>(heap.allocateWithNormalDestructor(size));
     else 
         result = static_cast<JSCell*>(heap.allocateWithoutDestructor(size));
+#if ENABLE(GC_VALIDATION)
+    ASSERT(!heap.vm()->isInitializingObject());
+    heap.vm()->setInitializingObjectClass(T::info());
+#endif
     result->clearStructure();
     return result;
 }