2011-02-01 Sheriff Bot <webkit.review.bot@gmail.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Feb 2011 05:05:55 +0000 (05:05 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Feb 2011 05:05:55 +0000 (05:05 +0000)
        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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/runtime/Heap.cpp
Source/JavaScriptCore/runtime/Heap.h
Source/JavaScriptCore/runtime/JSGlobalData.cpp
Source/JavaScriptCore/runtime/JSGlobalData.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSGlobalObject.h
Source/JavaScriptCore/runtime/MarkedSpace.cpp
Source/JavaScriptCore/runtime/WeakGCMap.h

index 9109d4208f3893f5f85d844d522c1e94263bf66d..64b8e45cb59d464b118cb66e4f82e96e7b314b1d 100644 (file)
         * DerivedSources.make:
         * JavaScriptCore.xcodeproj/project.pbxproj:
 
+2011-02-01  Geoffrey Garen  <ggaren@apple.com>
+
+        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@apple.com>
 
         Reviewed by Oliver Hunt.
index b4173829e24f77536df981dd95dbf2062e79544e..5fba8bb73059b7d79a259cb08a4fe2c0a299bdba 100644 (file)
@@ -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)
index ad2efa668d0036bbffd7e5537e04b07dce52d197..f8498b4c6bbe481a759dfea5bca815e31e7a1334 100644 (file)
@@ -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;
     };
index 7060d6c398cfbb086bda393d40ed84cab71a2413..f032eca540072febc3b430d7109fb3e64bd12fcc 100644 (file)
@@ -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;
index 91a7e8849a8bd68dd632edf8b035a40284ac11b5..93e84cf1921c2e6ffc193e26dea1020bbda1c539 100644 (file)
@@ -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;
 
index a363377475e47961d1b5b4760ef94067207435e6..ba63bc040c7192247704a0345e9481964a79e7fc 100644 (file)
@@ -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)
index e8469ddabb28dc93c7c8186cf005f6ace6f1d058..7b6905581b00b00205b4c2564f9beb8f6453b690 100644 (file)
@@ -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;
index 715ff0d05cbceb10a61576e06b1040c81d668062..bd6c1f220322ae509c7b8c2ef28560ae3081d46d 100644 (file)
@@ -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);
index 11477cb673944be8417b33d388169a174fcbf76c..8dfb468dd4e576f5d9935fe473fd20d73de9e3cc 100644 (file)
@@ -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&);
         
index e814d84bf9551a90027ad954345008841204dd9b..6ff71f9db1b8684afd55df6fdc4b65cd21fda482 100644 (file)
@@ -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);
index 915ad0f6e20d791d8a77902e084114597115220a..68788d0544dd37128b450c387e1c39f40854b116 100644 (file)
@@ -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)