Patch by Darin and me, reviewed by Maciej.

        Fixed <rdar://problem/4518397> REGRESSION(?): Oft-seen but unrepro crash
              in JavaScript garbage collection (KJS::Collector::collect())
               <rdar://problem/4752492> Crash in KJS::collect

        The issue here was allocating one garbage-collected object in the midst
        of allocating a second garbage-collected object. In such a case, the
        zeroIfFree word lies.

        * kjs/collector.cpp:
        (KJS::Collector::allocate):
        (KJS::Collector::collect):

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

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index e5560afe3edc677a9576373a0674088f1fd16f8e..7383113ea9288fdff94e11e28a844eebc1ab475d 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2006-10-04  Geoffrey Garen  <ggaren@apple.com>
+
+        Patch by Darin and me, reviewed by Maciej.
+
+        Fixed <rdar://problem/4518397> REGRESSION(?): Oft-seen but unrepro crash 
+              in JavaScript garbage collection (KJS::Collector::collect())
+               <rdar://problem/4752492> Crash in KJS::collect
+              
+        The issue here was allocating one garbage-collected object in the midst 
+        of allocating a second garbage-collected object. In such a case, the
+        zeroIfFree word lies.
+
+        * kjs/collector.cpp:
+        (KJS::Collector::allocate):
+        (KJS::Collector::collect):
+
 2006-10-04  Kevin McCullough  <KMcCullough@apple.com>
 
         Reviewed by Adam.

diff --git a/JavaScriptCore/kjs/collector.cpp b/JavaScriptCore/kjs/collector.cpp
index 8deeb15b1072573f01553954d5dff6f69cf06597..748480eae2896b7cd0e23d9d2bee49b5b50f3b4e 100644 (file)
--- a/JavaScriptCore/kjs/collector.cpp
+++ b/JavaScriptCore/kjs/collector.cpp
@@ -118,6 +118,7 @@ void* Collector::allocate(size_t s)
   size_t numLiveObjects = heap.numLiveObjects;
   size_t numLiveObjectsAtLastCollect = heap.numLiveObjectsAtLastCollect;
   size_t numNewObjects = numLiveObjects - numLiveObjectsAtLastCollect;
+
   if (numNewObjects >= ALLOCATIONS_PER_COLLECTION && numNewObjects >= numLiveObjectsAtLastCollect) {
     collect();
     numLiveObjects = heap.numLiveObjects;
@@ -492,6 +493,11 @@ bool Collector::collect()
         if (imp->m_marked) {
           imp->m_marked = false;
         } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
+          // special case for allocated but uninitialized object
+          // (We don't need this check earlier because nothing prior this point assumes the object has a valid vptr.)
+          if (cell->u.freeCell.zeroIfFree == 0)
+            continue;
+
           imp->~JSCell();
           --usedCells;
           --numLiveObjects;
@@ -504,7 +510,7 @@ bool Collector::collect()
       }
     } else {
       size_t minimumCellsToProcess = usedCells;
-      for (size_t i = 0; i < minimumCellsToProcess; i++) {
+      for (size_t i = 0; (i < minimumCellsToProcess) & (i < CELLS_PER_BLOCK); i++) {
         CollectorCell *cell = curBlock->cells + i;
         if (cell->u.freeCell.zeroIfFree == 0) {
           ++minimumCellsToProcess;