[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198

ggaren at apple.com ggaren at apple.com
Mon Feb 21 00:36:49 UTC 2011


The following commit has been merged in the webkit-1.3 branch:
commit 79ab2a9842295a53e36a9ab37e3f1e17809488f5
Author: ggaren at apple.com <ggaren at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Wed Feb 2 05:05:55 2011 +0000

    2011-02-01  Sheriff Bot  <webkit.review.bot at gmail.com>
    
            Unreviewed, rolling out r77297.
            http://trac.webkit.org/changeset/77297
            https://bugs.webkit.org/show_bug.cgi?id=53538
    
            caused leopard crashes (Requested by paroga on #webkit).
    
            * JavaScriptCore.vcproj/JavaScriptCore/JavaScriptCore.def:
            * wtf/text/AtomicString.cpp:
            (WTF::AtomicString::fromUTF8):
            * wtf/unicode/UTF8.cpp:
            (WTF::Unicode::calculateStringHashFromUTF8):
            * wtf/unicode/UTF8.h:
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77360 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index 9109d42..64b8e45 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -62,6 +62,51 @@
 
         Reviewed by Oliver Hunt.
 
+        Refactor JSGlobalObject-related tear-down
+        https://bugs.webkit.org/show_bug.cgi?id=53478
+        
+        While investigating crashes caused by r77082, I noticed some strange
+        destructor-time behaviors. This patch makes them less strange.
+
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::CodeBlock):
+        (JSC::CodeBlock::markAggregate):
+        * bytecode/CodeBlock.h:
+        (JSC::CodeBlock::globalObject):
+        (JSC::GlobalCodeBlock::GlobalCodeBlock):
+        (JSC::GlobalCodeBlock::~GlobalCodeBlock): Store the set of global code
+        blocks on the Heap, instead of on independent global objects. The heap
+        is guaranteed to outlast any GC-owned data structure. The heap is also
+        a natural place to store objects that needs out-of-band marking, since
+        the heap is responsible for marking all roots.
+
+        * runtime/Heap.cpp:
+        (JSC::Heap::markRoots):
+        (JSC::Heap::globalObjectCount):
+        (JSC::Heap::protectedGlobalObjectCount):
+        * runtime/Heap.h:
+        (JSC::Heap::codeBlocks):
+        * runtime/JSGlobalData.cpp:
+        (JSC::JSGlobalData::JSGlobalData):
+        * runtime/JSGlobalData.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::~JSGlobalObject):
+        (JSC::JSGlobalObject::init):
+        (JSC::JSGlobalObject::markChildren):
+        * runtime/JSGlobalObject.h:
+        * runtime/MarkedSpace.cpp: Store the set of global objects in a weak map
+        owned by JSGlobalData, instead of an instrusive circular linked list.
+        This is simpler, and it avoids destructor-time access between garbage
+        collected objects, which is hard to get right.
+
+        (JSC::MarkedSpace::destroy): Make sure to clear mark bits before tearing
+        everything down. Otherwise, weak data structures will incorrectly report
+        that objects pending destruction are still alive.
+
+2011-02-01  Geoffrey Garen  <ggaren at apple.com>
+
+        Reviewed by Oliver Hunt.
+
         REGRESSION(77082): GC-related crashes seen: on WebKit2 bot; on GTK 32bit
         bot; loading trac pages; typing in search field
         https://bugs.webkit.org/show_bug.cgi?id=53519
diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
index b417382..5fba8bb 100644
--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
@@ -1361,6 +1361,7 @@ void CodeBlock::dumpStatistics()
 
 CodeBlock::CodeBlock(ScriptExecutable* ownerExecutable, CodeType codeType, JSGlobalObject *globalObject, PassRefPtr<SourceProvider> sourceProvider, unsigned sourceOffset, SymbolTable* symTab, bool isConstructor)
     : m_globalObject(globalObject)
+    , m_heap(&m_globalObject->globalData().heap)
     , m_numCalleeRegisters(0)
     , m_numVars(0)
     , m_numParameters(0)
@@ -1534,7 +1535,6 @@ void CodeBlock::markAggregate(MarkStack& markStack)
         m_functionExprs[i]->markAggregate(markStack);
     for (size_t i = 0; i < m_functionDecls.size(); ++i)
         m_functionDecls[i]->markAggregate(markStack);
-    markStack.append(&m_globalObject);
 }
 
 HandlerInfo* CodeBlock::handlerForBytecodeOffset(unsigned bytecodeOffset)
diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h
index ad2efa6..f8498b4 100644
--- a/Source/JavaScriptCore/bytecode/CodeBlock.h
+++ b/Source/JavaScriptCore/bytecode/CodeBlock.h
@@ -249,6 +249,7 @@ namespace JSC {
         CodeBlock(ScriptExecutable* ownerExecutable, CodeType, JSGlobalObject*, PassRefPtr<SourceProvider>, unsigned sourceOffset, SymbolTable* symbolTable, bool isConstructor);
 
         DeprecatedPtr<JSGlobalObject> m_globalObject;
+        Heap* m_heap;
 
     public:
         virtual ~CodeBlock();
@@ -616,17 +617,14 @@ namespace JSC {
         GlobalCodeBlock(ScriptExecutable* ownerExecutable, CodeType codeType, JSGlobalObject* globalObject, PassRefPtr<SourceProvider> sourceProvider, unsigned sourceOffset)
             : CodeBlock(ownerExecutable, codeType, globalObject, sourceProvider, sourceOffset, &m_unsharedSymbolTable, false)
         {
-            m_globalObject->codeBlocks().add(this);
+            m_heap->codeBlocks().add(this);
         }
 
         ~GlobalCodeBlock()
         {
-            if (m_globalObject)
-                m_globalObject->codeBlocks().remove(this);
+            m_heap->codeBlocks().remove(this);
         }
 
-        void clearGlobalObject() { m_globalObject = 0; }
-
     private:
         SymbolTable m_unsharedSymbolTable;
     };
diff --git a/Source/JavaScriptCore/runtime/Heap.cpp b/Source/JavaScriptCore/runtime/Heap.cpp
index 7060d6c..f032eca 100644
--- a/Source/JavaScriptCore/runtime/Heap.cpp
+++ b/Source/JavaScriptCore/runtime/Heap.cpp
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "Heap.h"
 
+#include "CodeBlock.h"
 #include "CollectorHeapIterator.h"
 #include "ConservativeSet.h"
 #include "GCActivityCallback.h"
@@ -250,6 +251,11 @@ void Heap::markRoots()
     // Mark temporary vector for Array sorting
     markTempSortVectors(markStack);
     markStack.drain();
+    
+    HashSet<GlobalCodeBlock*>::const_iterator end = m_codeBlocks.end();
+    for (HashSet<GlobalCodeBlock*>::const_iterator it = m_codeBlocks.begin(); it != end; ++it)
+        (*it)->markAggregate(markStack);
+    markStack.drain();
 
     // Mark misc. other roots.
     if (m_markListSet && m_markListSet->size())
@@ -289,27 +295,18 @@ size_t Heap::capacity() const
 
 size_t Heap::globalObjectCount()
 {
-    size_t count = 0;
-    if (JSGlobalObject* head = m_globalData->head) {
-        JSGlobalObject* o = head;
-        do {
-            ++count;
-            o = o->next();
-        } while (o != head);
-    }
-    return count;
+    return m_globalData->globalObjects.uncheckedSize();
 }
 
 size_t Heap::protectedGlobalObjectCount()
 {
     size_t count = 0;
-    if (JSGlobalObject* head = m_globalData->head) {
-        JSGlobalObject* o = head;
-        do {
-            if (m_protectedValues.contains(o))
-                ++count;
-            o = o->next();
-        } while (o != head);
+
+    GlobalObjectMap& map = m_globalData->globalObjects;
+    GlobalObjectMap::iterator end = map.uncheckedEnd();
+    for (GlobalObjectMap::iterator it = map.uncheckedBegin(); it != end; ++it) {
+        if (map.isValid(it) && m_protectedValues.contains(it->second.get()))
+            ++count;
     }
 
     return count;
diff --git a/Source/JavaScriptCore/runtime/Heap.h b/Source/JavaScriptCore/runtime/Heap.h
index 91a7e88..93e84cf 100644
--- a/Source/JavaScriptCore/runtime/Heap.h
+++ b/Source/JavaScriptCore/runtime/Heap.h
@@ -32,6 +32,7 @@
 namespace JSC {
 
     class GCActivityCallback;
+    class GlobalCodeBlock;
     class JSCell;
     class JSGlobalData;
     class JSValue;
@@ -91,8 +92,10 @@ namespace JSC {
 
         WeakGCHandle* addWeakGCHandle(JSCell*);
 
-        void pushTempSortVector(WTF::Vector<ValueStringPair>*);
-        void popTempSortVector(WTF::Vector<ValueStringPair>*);        
+        void pushTempSortVector(Vector<ValueStringPair>*);
+        void popTempSortVector(Vector<ValueStringPair>*);
+        
+        HashSet<GlobalCodeBlock*>& codeBlocks() { return m_codeBlocks; }
 
         HashSet<MarkedArgumentBuffer*>& markListSet() { if (!m_markListSet) m_markListSet = new HashSet<MarkedArgumentBuffer*>; return *m_markListSet; }
 
@@ -123,8 +126,9 @@ namespace JSC {
         OperationInProgress m_operationInProgress;
 
         ProtectCountSet m_protectedValues;
-        WTF::Vector<PageAllocationAligned> m_weakGCHandlePools;
-        WTF::Vector<WTF::Vector<ValueStringPair>* > m_tempSortingVectors;
+        Vector<PageAllocationAligned> m_weakGCHandlePools;
+        Vector<Vector<ValueStringPair>* > m_tempSortingVectors;
+        HashSet<GlobalCodeBlock*> m_codeBlocks;
 
         HashSet<MarkedArgumentBuffer*>* m_markListSet;
 
diff --git a/Source/JavaScriptCore/runtime/JSGlobalData.cpp b/Source/JavaScriptCore/runtime/JSGlobalData.cpp
index a363377..ba63bc0 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalData.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalData.cpp
@@ -140,7 +140,6 @@ JSGlobalData::JSGlobalData(GlobalDataType globalDataType, ThreadStackType thread
     , parser(new Parser)
     , interpreter(new Interpreter)
     , heap(this)
-    , head(0)
     , dynamicGlobalObject(0)
     , firstStringifierToMark(0)
     , cachedUTCOffset(NaN)
diff --git a/Source/JavaScriptCore/runtime/JSGlobalData.h b/Source/JavaScriptCore/runtime/JSGlobalData.h
index e8469dd..7b69055 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalData.h
+++ b/Source/JavaScriptCore/runtime/JSGlobalData.h
@@ -39,6 +39,7 @@
 #include "SmallStrings.h"
 #include "Terminator.h"
 #include "TimeoutChecker.h"
+#include "WeakGCMap.h"
 #include "WeakRandom.h"
 #include <wtf/BumpPointerAllocator.h>
 #include <wtf/Forward.h>
@@ -72,7 +73,9 @@ namespace JSC {
 #endif
 
     struct HashTable;
-    struct Instruction;    
+    struct Instruction;
+
+    typedef WeakGCMap<JSGlobalObject*, JSGlobalObject> GlobalObjectMap; // FIXME: Would be nice to use a WeakGCSet here.
 
     struct DSTOffsetCache {
         DSTOffsetCache()
@@ -210,7 +213,7 @@ namespace JSC {
 
         HashMap<OpaqueJSClass*, OpaqueJSClassContextData*> opaqueJSClassData;
 
-        JSGlobalObject* head;
+        GlobalObjectMap globalObjects;
         JSGlobalObject* dynamicGlobalObject;
 
         HashSet<JSObject*> stringRecursionCheckVisitedObjects;
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
index 715ff0d..bd6c1f2 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
@@ -104,18 +104,8 @@ JSGlobalObject::~JSGlobalObject()
         (*profiler)->stopProfiling(globalExec(), UString());
     }
 
-    d()->next->d()->prev = d()->prev;
-    d()->prev->d()->next = d()->next;
-    JSGlobalObject*& headObject = head();
-    if (headObject == this)
-        headObject = d()->next;
-    if (headObject == this)
-        headObject = 0;
-
-    HashSet<GlobalCodeBlock*>::const_iterator end = codeBlocks().end();
-    for (HashSet<GlobalCodeBlock*>::const_iterator it = codeBlocks().begin(); it != end; ++it)
-        (*it)->clearGlobalObject();
-        
+    d()->globalData->globalObjects.take(this);
+
     RegisterFile& registerFile = globalData().interpreter->registerFile();
     if (registerFile.clearGlobalObject(this))
         registerFile.setNumGlobals(0);
@@ -125,22 +115,15 @@ JSGlobalObject::~JSGlobalObject()
 void JSGlobalObject::init(JSObject* thisValue)
 {
     ASSERT(JSLock::currentThreadIsHoldingLock());
-
+    
     structure()->disableSpecificFunctionTracking();
 
     d()->globalData = Heap::heap(this)->globalData();
+    d()->globalData->globalObjects.set(this, this);
     d()->globalScopeChain = ScopeChain(this, d()->globalData.get(), this, thisValue);
 
     JSGlobalObject::globalExec()->init(0, 0, d()->globalScopeChain.node(), CallFrame::noCaller(), 0, 0);
 
-    if (JSGlobalObject*& headObject = head()) {
-        d()->prev = headObject;
-        d()->next = headObject->d()->next;
-        headObject->d()->next->d()->prev = this;
-        headObject->d()->next = this;
-    } else
-        headObject = d()->next = d()->prev = this;
-
     d()->debugger = 0;
 
     d()->profileGroup = 0;
@@ -345,10 +328,6 @@ void JSGlobalObject::markChildren(MarkStack& markStack)
 {
     JSVariableObject::markChildren(markStack);
     
-    HashSet<GlobalCodeBlock*>::const_iterator end = codeBlocks().end();
-    for (HashSet<GlobalCodeBlock*>::const_iterator it = codeBlocks().begin(); it != end; ++it)
-        (*it)->markAggregate(markStack);
-
     markIfNeeded(markStack, &d()->regExpConstructor);
     markIfNeeded(markStack, &d()->errorConstructor);
     markIfNeeded(markStack, &d()->evalErrorConstructor);
diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
index 11477cb..8dfb468 100644
--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
@@ -135,7 +135,6 @@ namespace JSC {
 
             RefPtr<JSGlobalData> globalData;
 
-            HashSet<GlobalCodeBlock*> codeBlocks;
             WeakMapSet weakMaps;
             WeakRandom weakRandom;
         };
@@ -182,10 +181,6 @@ namespace JSC {
         virtual void defineGetter(ExecState*, const Identifier& propertyName, JSObject* getterFunc, unsigned attributes);
         virtual void defineSetter(ExecState*, const Identifier& propertyName, JSObject* setterFunc, unsigned attributes);
 
-        // Linked list of all global objects that use the same JSGlobalData.
-        JSGlobalObject*& head() { return d()->globalData->head; }
-        JSGlobalObject* next() { return d()->next; }
-
         // The following accessors return pristine values, even if a script 
         // replaces the global object's associated property.
 
@@ -250,8 +245,6 @@ namespace JSC {
 
         virtual bool isDynamicScope(bool& requiresDynamicChecks) const;
 
-        HashSet<GlobalCodeBlock*>& codeBlocks() { return d()->codeBlocks; }
-
         void copyGlobalsFrom(RegisterFile&);
         void copyGlobalsTo(RegisterFile&);
         
diff --git a/Source/JavaScriptCore/runtime/MarkedSpace.cpp b/Source/JavaScriptCore/runtime/MarkedSpace.cpp
index e814d84..6ff71f9 100644
--- a/Source/JavaScriptCore/runtime/MarkedSpace.cpp
+++ b/Source/JavaScriptCore/runtime/MarkedSpace.cpp
@@ -50,6 +50,8 @@ MarkedSpace::MarkedSpace(JSGlobalData* globalData)
 
 void MarkedSpace::destroy()
 {
+    clearMarkBits(); // Make sure weak pointers appear dead during destruction.
+
     for (size_t block = 0; block < m_heap.usedBlocks; ++block)
         freeBlock(block);
     fastFree(m_heap.blocks);
diff --git a/Source/JavaScriptCore/runtime/WeakGCMap.h b/Source/JavaScriptCore/runtime/WeakGCMap.h
index 915ad0f..68788d0 100644
--- a/Source/JavaScriptCore/runtime/WeakGCMap.h
+++ b/Source/JavaScriptCore/runtime/WeakGCMap.h
@@ -59,6 +59,8 @@ public:
     // These unchecked functions provide access to a value even if the value's
     // mark bit is not set. This is used, among other things, to retrieve values
     // during the GC mark phase, which begins by clearing all mark bits.
+    
+    size_t uncheckedSize() { return m_map.size(); }
 
     MappedType* uncheckedGet(const KeyType& key) const { return m_map.get(key).get(); }
     DeprecatedPtr<MappedType>* uncheckedGetSlot(const KeyType& key)

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list