Changeset 53460 in webkit for trunk/JavaScriptCore/runtime/Collector.cpp

Timestamp:

Jan 19, 2010, 12:39:04 AM (15 years ago)

Author:

[email protected]

Message:

JavaScriptCore: REGRESSION (52082): Crash on worker thread when reloading ​http://radnan.public.iastate.edu/procedural/
​https://bugs.webkit.org/show_bug.cgi?id=33826

Reviewed by Oliver Hunt.

This bug was caused by a GC-protected object being destroyed early by
Heap::destroy. Clients of the GC protect APIs (reasonably) expect pointers
to GC-protected memory to be valid.

The solution is to do two passes of tear-down in Heap::destroy. The first
pass tears down all unprotected objects. The second pass ASSERTs that all
previously protected objects are now unprotected, and then tears down
all perviously protected objects. These two passes simulate the two passes
that would have been required to free a protected object during normal GC.

• API/JSContextRef.cpp: Removed some ASSERTs that have moved into Heap.

• runtime/Collector.cpp:

(JSC::Heap::destroy): Moved ASSERTs to here.
(JSC::Heap::freeBlock): Tidied up the use of didShrink by moving its
setter to the function that does the shrinking.
(JSC::Heap::freeBlocks): Implemented above algorithm.
(JSC::Heap::shrinkBlocks): Tidied up the use of didShrink.

WebCore: REGRESSION (52082): Crash on worker thread when reloading ​http://radnan.public.iastate.edu/procedural/
​https://bugs.webkit.org/show_bug.cgi?id=33826

Reviewed by Oliver Hunt.

Test: fast/workers/worker-gc2.html

• bindings/js/WorkerScriptController.cpp:

(WebCore::WorkerScriptController::~WorkerScriptController): Removed some
ASSERTs that have moved to JavaScriptCore.

LayoutTests: REGRESSION (52082): Crash on worker thread when reloading ​http://radnan.public.iastate.edu/procedural/
​https://bugs.webkit.org/show_bug.cgi?id=33826

Reviewed by Oliver Hunt.

Added a test for this edge case.

• fast/workers/resources/worker-gc2.js: Added.

(Dummy):

• fast/workers/worker-gc2.html: Added.

File:

• trunk/JavaScriptCore/runtime/Collector.cpp (modified) (4 diffs)

trunk/JavaScriptCore/runtime/Collector.cpp

@@ -187 +187 @@
         return;
 
+    ASSERT(!m_globalData->dynamicGlobalObject);
+    ASSERT(!isBusy());
+    
     // The global object is not GC protected at this point, so sweeping may delete it
     // (and thus the global data) before other objects that may use the global data.
@@ -291 +294 @@
 NEVER_INLINE void Heap::freeBlock(size_t block)
 {
+    m_heap.didShrink = true;
+
     ObjectIterator it(m_heap, block);
     ObjectIterator end(m_heap, block + 1);
@@ -330 +335 @@
 void Heap::freeBlocks()
 {
-    while (m_heap.usedBlocks)
-        freeBlock(0);
+    ProtectCountSet protectedValuesCopy = m_protectedValues;
+
+    clearMarkBits();
+    markProtectedObjects(m_globalData->markStack);
+
+    m_heap.nextCell = 0;
+    m_heap.nextBlock = 0;
+    DeadObjectIterator it(m_heap, m_heap.nextBlock, m_heap.nextCell);
+    DeadObjectIterator end(m_heap, m_heap.usedBlocks);
+    for ( ; it != end; ++it)
+        (*it)->~JSCell();
+
+    ASSERT(!protectedObjectCount());
+
+    ProtectCountSet::iterator protectedValuesEnd = protectedValuesCopy.end();
+    for (ProtectCountSet::iterator protectedValuesIt = protectedValuesCopy.begin(); protectedValuesIt != protectedValuesEnd; ++protectedValuesIt)
+        protectedValuesIt->first->~JSCell();
+
+    for (size_t block = 0; block < m_heap.usedBlocks; ++block)
+        freeBlockPtr(m_heap.blocks[block]);
+
     fastFree(m_heap.blocks);
+
     memset(&m_heap, 0, sizeof(CollectorHeap));
 }
@@ -441 +466 @@
         if (m_heap.blocks[i]->marked.isEmpty()) {
             freeBlock(i);
-            m_heap.didShrink = true;
         } else
             ++i;