Reviewed by Darin Adler.
authorap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Oct 2008 08:00:53 +0000 (08:00 +0000)
committerap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 16 Oct 2008 08:00:53 +0000 (08:00 +0000)
        https://bugs.webkit.org/show_bug.cgi?id=21609
        Make MessagePorts protect their peers across heaps

JavaScriptCore:
        * JavaScriptCore.exp:
        * kjs/JSGlobalObject.cpp:
        (JSC::JSGlobalObject::markCrossHeapDependentObjects):
        * kjs/JSGlobalObject.h:
        * kjs/collector.cpp:
        (JSC::Heap::collect):
        Before GC sweep phase, a function supplied by global object is now called for all global
        objects in the heap, making it possible to implement cross-heap dependencies.

WebCore:
        * dom/MessagePort.cpp:
        (WebCore::MessagePort::MessagePort):
        * dom/MessagePort.h:
        (WebCore::MessagePort::setJSWrapperIsKnownToBeInaccessible):
        (WebCore::MessagePort::jsWrapperIsKnownToBeInaccessible):
        Track objects whose JS wrappers are no longer reachable in MessagePort. Unfortunately, this
        means that the implementation object knows about JS bindings - but it is not possible to
        access JS wrappers from another heap/thread.

        * bindings/js/JSDOMBinding.cpp:
        (WebCore::markCrossHeapDependentObjectsForDocument):
        * bindings/js/JSDOMBinding.h:
        * bindings/js/JSDOMWindowBase.cpp:
        (WebCore::JSDOMWindowBase::markCrossHeapDependentObjects):
        * bindings/js/JSDOMWindowBase.h:
        Implement cross-heap dependency tracking for entangled MessagePorts. If a wrapper object
        hasn't been marked normally, it is marked as inaccessible. It is then marked manually,
        as long as its entangled port is accessible itself.

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

12 files changed:
JavaScriptCore/ChangeLog
JavaScriptCore/JavaScriptCore.exp
JavaScriptCore/kjs/JSGlobalObject.cpp
JavaScriptCore/kjs/JSGlobalObject.h
JavaScriptCore/kjs/collector.cpp
WebCore/ChangeLog
WebCore/bindings/js/JSDOMBinding.cpp
WebCore/bindings/js/JSDOMBinding.h
WebCore/bindings/js/JSDOMWindowBase.cpp
WebCore/bindings/js/JSDOMWindowBase.h
WebCore/dom/MessagePort.cpp
WebCore/dom/MessagePort.h

index 02bb1321a4570bce3fc48f703392c0c413d6db00..040e37afe5f2b896c7fb1ec0fdebec8383018bcb 100644 (file)
@@ -1,3 +1,19 @@
+2008-10-15  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=21609
+        Make MessagePorts protect their peers across heaps
+
+        * JavaScriptCore.exp:
+        * kjs/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::markCrossHeapDependentObjects):
+        * kjs/JSGlobalObject.h:
+        * kjs/collector.cpp:
+        (JSC::Heap::collect):
+        Before GC sweep phase, a function supplied by global object is now called for all global
+        objects in the heap, making it possible to implement cross-heap dependencies.
+
 2008-10-15  Alexey Proskuryakov  <ap@webkit.org>
 
         Reviewed by Darin Adler.
index a6a84499f88b312cb26f01ff7e124fafb34797e2..389e5e4eb98dadd9a9a35c643f5fb72070f4daa8 100644 (file)
@@ -147,6 +147,7 @@ __ZN3JSC14JSGlobalObject14setTimeoutTimeEj
 __ZN3JSC14JSGlobalObject16stopTimeoutCheckEv
 __ZN3JSC14JSGlobalObject17putWithAttributesEPNS_9ExecStateERKNS_10IdentifierEPNS_7JSValueEj
 __ZN3JSC14JSGlobalObject17startTimeoutCheckEv
+__ZN3JSC14JSGlobalObject29markCrossHeapDependentObjectsEv
 __ZN3JSC14JSGlobalObject3putEPNS_9ExecStateERKNS_10IdentifierEPNS_7JSValueERNS_15PutPropertySlotE
 __ZN3JSC14JSGlobalObject4initEPNS_8JSObjectE
 __ZN3JSC14JSGlobalObject4markEv
index c2f815a328b3a2c131833870928f4fb13ff3f620..385e8a610d6c1c2192739152ed666797002fc11d 100644 (file)
@@ -399,6 +399,11 @@ void JSGlobalObject::mark()
     }
 }
 
+void JSGlobalObject::markCrossHeapDependentObjects()
+{
+    // Overridden by subclasses.
+}
+
 JSGlobalObject* JSGlobalObject::toGlobalObject(ExecState*) const
 {
     return const_cast<JSGlobalObject*>(this);
index f9f525743773e1a6879a290d6e40c932bea0baa0..22ae6fe0b5cbf08cd46271d7c9b9509f1b29a7af 100644 (file)
@@ -158,6 +158,7 @@ namespace JSC {
         virtual ~JSGlobalObject();
 
         virtual void mark();
+        virtual void markCrossHeapDependentObjects();
 
         virtual bool getOwnPropertySlot(ExecState*, const Identifier&, PropertySlot&);
         virtual bool getOwnPropertySlot(ExecState*, const Identifier&, PropertySlot&, bool& slotIsWriteable);
index 52fd370310139747ec1f53f132b4bb93a5c69291..20be74c5685ff60ee3e8b8fbef665916df91ba62 100644 (file)
@@ -969,6 +969,12 @@ bool Heap::collect()
     m_globalData->machine->registerFile().markCallFrames(this);
     m_globalData->smallStrings.mark();
 
+    JSGlobalObject* globalObject = m_globalData->head;
+    do {
+        globalObject->markCrossHeapDependentObjects();
+        globalObject = globalObject->next();
+    } while (globalObject != m_globalData->head);
+
     JAVASCRIPTCORE_GC_MARKED();
 
     size_t originalLiveObjects = primaryHeap.numLiveObjects + numberHeap.numLiveObjects;
index e6a053066fd5b4279f3a8b845d9876254866c9dc..408629e17a1fd21e84e0ae557994a4b57180dc1d 100644 (file)
@@ -1,3 +1,29 @@
+2008-10-15  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        https://bugs.webkit.org/show_bug.cgi?id=21609
+        Make MessagePorts protect their peers across heaps
+
+        * dom/MessagePort.cpp:
+        (WebCore::MessagePort::MessagePort):
+        * dom/MessagePort.h:
+        (WebCore::MessagePort::setJSWrapperIsKnownToBeInaccessible):
+        (WebCore::MessagePort::jsWrapperIsKnownToBeInaccessible):
+        Track objects whose JS wrappers are no longer reachable in MessagePort. Unfortunately, this
+        means that the implementation object knows about JS bindings - but it is not possible to
+        access JS wrappers from another heap/thread.
+
+        * bindings/js/JSDOMBinding.cpp:
+        (WebCore::markCrossHeapDependentObjectsForDocument):
+        * bindings/js/JSDOMBinding.h:
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::markCrossHeapDependentObjects):
+        * bindings/js/JSDOMWindowBase.h:
+        Implement cross-heap dependency tracking for entangled MessagePorts. If a wrapper object
+        hasn't been marked normally, it is marked as inaccessible. It is then marked manually,
+        as long as its entangled port is accessible itself.
+
 2008-10-15  Jon Honeycutt  <jhoneycutt@apple.com>
 
         Remove unneeded check of whether a Page defers loading before running it
index ee5e7272399ee04141fe5ace4e14f4b0913be461..9910450952184defef54509d9a1eee9a362b07b4 100644 (file)
@@ -268,6 +268,33 @@ void markActiveObjectsForDocument(JSGlobalData& globalData, Document* doc)
     }
 }
 
+void markCrossHeapDependentObjectsForDocument(JSGlobalData& globalData, Document* document)
+{
+    const HashSet<MessagePort*>& messagePorts = document->messagePorts();
+    HashSet<MessagePort*>::const_iterator portsEnd = messagePorts.end();
+    for (HashSet<MessagePort*>::const_iterator iter = messagePorts.begin(); iter != portsEnd; ++iter) {
+        MessagePort* port = *iter;
+        RefPtr<MessagePort> entangledPort = port->entangledPort();
+        if (entangledPort) {
+            // No wrapper, or wrapper is already marked - no need to examine cross-heap dependencies.
+            DOMObject* wrapper = getCachedDOMObjectWrapper(globalData, port);
+            if (!wrapper || wrapper->marked())
+                continue;
+
+            // If the wrapper hasn't been marked during the mark phase of GC, then the port shouldn't protect its entangled one.
+            // It's important not to call this when there is no wrapper. E.g., if GC is triggered after a MessageChannel is created, but before its ports are used from JS,
+            // irreversibly telling the object that its (not yet existing) wrapper is inaccessible would be wrong. Similarly, ports posted via postMessage() may not
+            // have wrappers until delivered.
+            port->setJSWrapperIsInaccessible();
+
+            // If the port is protected by its entangled one, mark it.
+            // This is an atomic read of a boolean value, no synchronization between threads is required (at least on platforms that guarantee cache coherency).
+            if (!entangledPort->jsWrapperIsInaccessible())
+                wrapper->mark();
+        }
+    }
+}
+
 void updateDOMNodeDocument(Node* node, Document* oldDocument, Document* newDocument)
 {
     ASSERT(oldDocument != newDocument);
index bcb886f27acf319efd42a94227c329e75d9d33b2..569f92d675cb6208ecb78c69bb902df685c7e729 100644 (file)
@@ -69,6 +69,7 @@ namespace WebCore {
     void updateDOMNodeDocument(Node*, Document* oldDocument, Document* newDocument);
     void markDOMNodesForDocument(Document*);
     void markActiveObjectsForDocument(JSC::JSGlobalData&, Document*);
+    void markCrossHeapDependentObjectsForDocument(JSC::JSGlobalData&, Document*);
 
     JSC::StructureID* getCachedDOMStructure(JSC::ExecState*, const JSC::ClassInfo*);
     JSC::StructureID* cacheDOMStructure(JSC::ExecState*, PassRefPtr<JSC::StructureID>, const JSC::ClassInfo*);
index c37a862dbde7b3a3bcb051a441d18c0fd91d6bba..bcf22df18c39f46d025c3f9eeeb899cf9de092f8 100644 (file)
@@ -507,6 +507,15 @@ JSValue* JSDOMWindowBase::namedItemGetter(ExecState* exec, const Identifier& pro
     return toJS(exec, collection.get());
 }
 
+void JSDOMWindowBase::markCrossHeapDependentObjects()
+{
+    Document* document = impl()->document();
+    if (!document)
+        return;
+
+    markCrossHeapDependentObjectsForDocument(*globalData(), document);
+}
+
 bool JSDOMWindowBase::getOwnPropertySlot(ExecState* exec, const Identifier& propertyName, PropertySlot& slot)
 {
     // Check for child frames by name before built-in properties to
index df3daf863fca8e119bc2f1efa218c413c1408d3c..9170c63d115816a0b772dd2d1cbe772702ebc45f 100644 (file)
@@ -64,6 +64,8 @@ namespace WebCore {
 
         void disconnectFrame();
 
+        virtual void markCrossHeapDependentObjects();
+
         virtual bool getOwnPropertySlot(JSC::ExecState*, const JSC::Identifier&, JSC::PropertySlot&);
         virtual void put(JSC::ExecState*, const JSC::Identifier& propertyName, JSC::JSValue*, JSC::PutPropertySlot&);
 
index 6da0b54c27c234699af6bc57ac331ac06b5079c1..ee98befb3b6f728ef40cdcb50a164ed59e0419d2 100644 (file)
@@ -68,6 +68,7 @@ MessagePort::MessagePort(Document* document)
     , m_queueIsOpen(false)
     , m_document(document)
     , m_pendingActivity(0)
+    , m_jsWrapperIsInaccessible(false)
 {
     document->createdMessagePort(this);
 }
index 100bf79aedd4bfb354597434b18b7fa6d26ce886..ed7f9925b13fbfbca23ecf8d3be6d31d54d02d52 100644 (file)
@@ -33,8 +33,8 @@
 #include <wtf/HashMap.h>
 #include <wtf/MessageQueue.h>
 #include <wtf/PassRefPtr.h>
-#include <wtf/RefCounted.h>
 #include <wtf/RefPtr.h>
+#include <wtf/Threading.h>
 #include <wtf/Vector.h>
 
 namespace WebCore {
@@ -45,7 +45,7 @@ namespace WebCore {
     class Frame;
     class String;
 
-    class MessagePort : public RefCounted<MessagePort>, public EventTarget {
+    class MessagePort : public ThreadSafeShared<MessagePort>, public EventTarget {
     public:
         static PassRefPtr<MessagePort> create(Document* document) { return adoptRef(new MessagePort(document)); }
         ~MessagePort();
@@ -83,8 +83,8 @@ namespace WebCore {
         typedef HashMap<AtomicStringImpl*, ListenerVector> EventListenersMap;
         EventListenersMap& eventListeners() { return m_eventListeners; }
 
-        using RefCounted<MessagePort>::ref;
-        using RefCounted<MessagePort>::deref;
+        using ThreadSafeShared<MessagePort>::ref;
+        using ThreadSafeShared<MessagePort>::deref;
 
         bool hasPendingActivity() { return m_pendingActivity; }
 
@@ -94,6 +94,9 @@ namespace WebCore {
         void setOnclose(PassRefPtr<EventListener> eventListener) { m_onCloseListener = eventListener; }
         EventListener* onclose() const { return m_onCloseListener.get(); }
 
+        void setJSWrapperIsInaccessible() { m_jsWrapperIsInaccessible = true; }
+        bool jsWrapperIsInaccessible() const { return m_jsWrapperIsInaccessible; }
+
     private:
         friend class CloseMessagePortTimer;
 
@@ -119,6 +122,7 @@ namespace WebCore {
         EventListenersMap m_eventListeners;
 
         unsigned m_pendingActivity;
+        bool m_jsWrapperIsInaccessible;
     };
 
 } // namespace WebCore