Changeset 118483 in webkit for trunk/Source/JavaScriptCore/runtime/WeakGCMap.h

Timestamp:

May 24, 2012, 11:52:00 PM (13 years ago)

Author:

[email protected]

Message:

WebKit should be lazy-finalization-safe (esp. the DOM)
​https://bugs.webkit.org/show_bug.cgi?id=87456

Reviewed by Filip Pizlo.

../JavaScriptCore:

Lazy finalization adds one twist to weak pointer use:

A HashMap of weak pointers may contain logically null entries.
(Weak pointers behave as-if null once their payloads die.)
Insertion must not assume that a pre-existing entry is
necessarily valid, and iteration must not assume that all
entries can be dereferenced.

(Previously, I thought that it also added a second twist:

A demand-allocated weak pointer may replace a dead payload
before the payload's finalizer runs. In that case, when the
payload's finalizer runs, the payload has already been
overwritten, and the finalizer should not clear the payload,
which now points to something new.

But that's not the case here, since we cancel the old payload's
finalizer when we over-write it. I've added ASSERTs to verify this
assumption, in case it ever changes.)

• API/JSClassRef.cpp:

(OpaqueJSClass::prototype): No need to specify null; that's the default.

• API/JSWeakObjectMapRefPrivate.cpp: Use remove, since take() is gone.

• heap/PassWeak.h:

(WeakImplAccessor::was): This is no longer a debug-only function, since
it's required to reason about lazily finalized pointers.

• heap/Weak.h:

(JSC::weakAdd):
(JSC::weakRemove):
(JSC::weakClear): Added these helper functions for the common idioms of
what clients want to do in their weak pointer finalizers.

• jit/JITStubs.cpp:

(JSC::JITThunks::hostFunctionStub): Use the new idioms. Otherwise, we
would return NULL for a "zombie" executable weak pointer that was waiting
for finalization (item (2)), and finalizing a dead executable weak pointer
would potentially destroy a new, live one (item (1)).

• runtime/RegExpCache.cpp:

(JSC::RegExpCache::lookupOrCreate):
(JSC::RegExpCache::finalize): Ditto.

(JSC::RegExpCache::invalidateCode): Check for null while iterating. (See
item (2).)

• runtime/Structure.cpp:

(JSC::StructureTransitionTable::contains):
(JSC::StructureTransitionTable::add): Use get and set instead of add and
contains, since add and contains are not compatible with lazy finalization.

• runtime/WeakGCMap.h:

(WeakGCMap):
(JSC::WeakGCMap::clear):
(JSC::WeakGCMap::remove): Removed a bunch of code that was incompatible with
lazy finalization because I didn't feel like making it compatible, and I had
no way to test it.

../WebCore:

• bindings/js/DOMWrapperWorld.cpp:

(WebCore::JSStringOwner::finalize):

• bindings/js/JSDOMBinding.cpp:

(WebCore::jsStringSlowCase):

• bindings/js/JSDOMBinding.h:

(WebCore::cacheWrapper):
(WebCore::uncacheWrapper): Use the new idioms.

(WebCore::jsString): Use get instead of find because get is simpler in
the case of entries that are logically null.

(WebCore::domObjectWrapperMapFor): Removed, since it was unused.

• bindings/js/ScriptWrappable.h:

(WebCore::ScriptWrappable::clearWrapper): Use the new idioms.

• bridge/runtime_root.cpp:

(JSC::Bindings::RootObject::invalidate): Check for null while iterating,
since that's possible now.

(JSC::Bindings::RootObject::addRuntimeObject):
(JSC::Bindings::RootObject::removeRuntimeObject):
(JSC::Bindings::RootObject::finalize): Use the new idioms.

• bridge/runtime_root.h:

(RootObject): Clarified the word "need".

../WebKit2:

• WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp:

(WebKit::NPRuntimeObjectMap::getOrCreateJSObject): Use the new idioms.

(WebKit::NPRuntimeObjectMap::invalidate): Check for null while iterating,
since that's possible now.

(WebKit::NPRuntimeObjectMap::finalize): Use the new idioms.

File:

• trunk/Source/JavaScriptCore/runtime/WeakGCMap.h (modified) (3 diffs)

trunk/Source/JavaScriptCore/runtime/WeakGCMap.h

@@ -54 +54 @@
     typedef HashMap<KeyType, WeakImpl*, HashArg, KeyTraitsArg> MapType;
     typedef typename HandleTypes<MappedType>::ExternalType ExternalType;
-    typedef typename MapType::iterator map_iterator;
 
 public:
-
-    struct iterator {
-        friend class WeakGCMap;
-        iterator(map_iterator iter)
-            : m_iterator(iter)
-        {
-        }
-        
-        std::pair<KeyType, ExternalType> get() const { return std::make_pair(m_iterator->first, HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&m_iterator->second->jsValue()))); }
-        
-        iterator& operator++() { ++m_iterator; return *this; }
-        
-        // postfix ++ intentionally omitted
-        
-        // Comparison.
-        bool operator==(const iterator& other) const { return m_iterator == other.m_iterator; }
-        bool operator!=(const iterator& other) const { return m_iterator != other.m_iterator; }
-        
-    private:
-        map_iterator m_iterator;
-    };
-
-    typedef WTF::HashTableAddResult<iterator> AddResult;
-
     WeakGCMap()
     {
     }
 
-    bool isEmpty() { return m_map.isEmpty(); }
     void clear()
     {
-        map_iterator end = m_map.end();
-        for (map_iterator ptr = m_map.begin(); ptr != end; ++ptr)
+        typename MapType::iterator end = m_map.end();
+        for (typename MapType::iterator ptr = m_map.begin(); ptr != end; ++ptr)
             WeakSet::deallocate(ptr->second);
         m_map.clear();
-    }
-
-    bool contains(const KeyType& key) const
-    {
-        return m_map.contains(key);
-    }
-
-    iterator find(const KeyType& key)
-    {
-        return m_map.find(key);
-    }
-
-    void remove(iterator iter)
-    {
-        ASSERT(iter.m_iterator != m_map.end());
-        WeakImpl* impl = iter.m_iterator->second;
-        ASSERT(impl);
-        WeakSet::deallocate(impl);
-        m_map.remove(iter.m_iterator);
     }
 
@@ -116 +71 @@
     {
         return HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&m_map.get(key)->jsValue()));
-    }
-
-    AddResult add(JSGlobalData&, const KeyType& key, ExternalType value)
-    {
-        typename MapType::AddResult result = m_map.add(key, 0);
-        if (result.isNewEntry)
-            result.iterator->second = WeakSet::allocate(value, this, FinalizerCallback::finalizerContextFor(key));
-
-        // WeakGCMap exposes a different iterator, so we need to wrap it and create our own AddResult.
-        return AddResult(iterator(result.iterator), result.isNewEntry);
     }
 
@@ -136 +81 @@
     }
 
-    ExternalType take(const KeyType& key)
+    void remove(const KeyType& key)
     {
         WeakImpl* impl = m_map.take(key);
         if (!impl)
-            return HashTraits<ExternalType>::emptyValue();
-        ExternalType result = HandleTypes<MappedType>::getFromSlot(const_cast<JSValue*>(&impl->jsValue()));
+            return;
         WeakSet::deallocate(impl);
-        return result;
     }
 
-    size_t size() { return m_map.size(); }
-
-    iterator begin() { return iterator(m_map.begin()); }
-    iterator end() { return iterator(m_map.end()); }
-    
     ~WeakGCMap()
     {