Commit 4c610c0e authored by ap@webkit.org's avatar ap@webkit.org

Reviewed by Darin Adler.

        Test: fast/events/message-channel-gc-3.html

        https://bugs.webkit.org/show_bug.cgi?id=21769
        MessagePort should be GC protected if there are messages to be delivered

        * dom/MessagePort.h: Removed pending activity count. Now we track if a close event is
        pending, and check if the queue is non-empty.
        (WebCore::MessagePort::workerContext): Added a stub implementation for a cross-heap GC bug
        fix (below).

        * dom/MessagePort.cpp:
        (WebCore::CloseMessagePortTimer::fired):
        (WebCore::MessagePort::MessagePort):
        (WebCore::MessagePort::queueCloseEvent):
        (WebCore::MessagePort::dispatchCloseEvent):
        (WebCore::MessagePort::hasPendingActivity):
        Track message and close event activity separately.

        * bindings/js/JSDOMBinding.cpp:
        (WebCore::markCrossHeapDependentObjectsForDocument): Fixed a bug in cross-heap GC that was
        causing same-heap ports to never be deleted.

        * wtf/MessageQueue.h:
        (WTF::::isEmpty): Added. Also added a warning for methods that return a snapshot of queue
        state, thus likely to cause race conditions.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@37771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 90a34533
2008-10-21 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Darin Adler.
https://bugs.webkit.org/show_bug.cgi?id=21769
MessagePort should be GC protected if there are messages to be delivered
* wtf/MessageQueue.h:
(WTF::::isEmpty): Added. Also added a warning for methods that return a snapshot of queue
state, thus likely to cause race conditions.
2008-10-21 Darin Adler <darin@apple.com>
Reviewed by Maciej Stachowiak.
......
......@@ -44,10 +44,14 @@ namespace WTF {
void append(const DataType&);
void prepend(const DataType&);
bool waitForMessage(DataType&);
bool tryGetMessage(DataType&);
void kill();
bool tryGetMessage(DataType&);
bool killed() const;
// The result of isEmpty() is only valid if no other thread is manipulating the queue at the same time.
bool isEmpty();
private:
mutable Mutex m_mutex;
ThreadCondition m_condition;
......@@ -102,6 +106,15 @@ namespace WTF {
return true;
}
template<typename DataType>
inline bool MessageQueue<DataType>::isEmpty()
{
MutexLocker lock(m_mutex);
if (m_killed)
return true;
return m_queue.isEmpty();
}
template<typename DataType>
inline void MessageQueue<DataType>::kill()
{
......
2008-10-21 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Darin Adler.
https://bugs.webkit.org/show_bug.cgi?id=21769
MessagePort should be GC protected if there are messages to be delivered
* fast/events/message-channel-gc-3-expected.txt: Added.
* fast/events/message-channel-gc-3.html: Added.
2008-10-20 Chris Fleizach <cfleizach@apple.com>
Reviewed by Jon Honeycutt.
......
Test that MessagePort messages are delivered even if both ports are inaccessible from JS any more.
Should say PASS twice.
PASS: message delivered. Port: [object MessagePort]
PASS: close message delivered. Port: [object MessagePort]
<body>
<p>Test that MessagePort messages are delivered even if both ports are inaccessible from JS any more.</p>
<p>Should say PASS twice.</p>
<pre id=log></pre>
<script>
function gc()
{
if (window.GCController)
return GCController.collect();
for (var i = 0; i < 10000; i++) { // > force garbage collection (FF requires about 9K allocations before a collect)
var s = new String("abc");
}
}
function log(message)
{
document.getElementById("log").innerHTML += message + "<br>";
}
if (window.layoutTestController) {
layoutTestController.dumpAsText();
layoutTestController.waitUntilDone();
}
function test1()
{
var channel = new MessageChannel;
channel.port1.onmessage = function(evt) { log("PASS: message delivered. Port: " + evt.target); test2(); }
channel.port1.start();
channel.port2.postMessage("msg");
channel = 0;
gc();
}
function test2()
{
var channel = new MessageChannel;
channel.port1.onclose = function(evt) { log("PASS: close message delivered. Port: " + evt.target); if (window.layoutTestController) layoutTestController.notifyDone(); }
channel.port2.close();
channel = 0;
gc();
}
test1();
</script>
</body>
2008-10-21 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Darin Adler.
Test: fast/events/message-channel-gc-3.html
https://bugs.webkit.org/show_bug.cgi?id=21769
MessagePort should be GC protected if there are messages to be delivered
* dom/MessagePort.h: Removed pending activity count. Now we track if a close event is
pending, and check if the queue is non-empty.
(WebCore::MessagePort::workerContext): Added a stub implementation for a cross-heap GC bug
fix (below).
* dom/MessagePort.cpp:
(WebCore::CloseMessagePortTimer::fired):
(WebCore::MessagePort::MessagePort):
(WebCore::MessagePort::queueCloseEvent):
(WebCore::MessagePort::dispatchCloseEvent):
(WebCore::MessagePort::hasPendingActivity):
Track message and close event activity separately.
* bindings/js/JSDOMBinding.cpp:
(WebCore::markCrossHeapDependentObjectsForDocument): Fixed a bug in cross-heap GC that was
causing same-heap ports to never be deleted.
2008-10-21 Dan Bernstein <mitz@apple.com>
Reviewed by Sam Weinig.
......
......@@ -281,6 +281,11 @@ void markCrossHeapDependentObjectsForDocument(JSGlobalData& globalData, Document
if (!wrapper || wrapper->marked())
continue;
// Don't use cross-heap model of marking on same-heap pairs. Otherwise, they will never be destroyed, because a port will mark its entangled one,
// and it will never get a chance to be marked as inaccessible. So, the port will keep getting marked in this function.
if ((port->document() && entangledPort->document()) || (port->workerContext() == entangledPort->workerContext()))
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
......
......@@ -56,7 +56,6 @@ private:
m_port->dispatchMessages();
m_port->dispatchCloseEvent();
m_port->unsetPendingActivity();
delete this;
}
......@@ -67,7 +66,7 @@ MessagePort::MessagePort(Document* document)
: m_entangledPort(0)
, m_queueIsOpen(false)
, m_document(document)
, m_pendingActivity(0)
, m_pendingCloseEvent(false)
, m_jsWrapperIsInaccessible(false)
{
document->createdMessagePort(this);
......@@ -230,8 +229,8 @@ void MessagePort::dispatchMessages()
void MessagePort::queueCloseEvent()
{
// Need to keep listeners alive, and they are marked by the wrapper.
setPendingActivity();
ASSERT(!m_pendingCloseEvent);
m_pendingCloseEvent = true;
CloseMessagePortTimer* timer = new CloseMessagePortTimer(this);
timer->startOneShot(0);
......@@ -239,6 +238,9 @@ void MessagePort::queueCloseEvent()
void MessagePort::dispatchCloseEvent()
{
ASSERT(m_pendingCloseEvent);
m_pendingCloseEvent = false;
RefPtr<Event> evt = Event::create(EventNames::closeEvent, false, true);
if (m_onCloseListener) {
evt->setTarget(this);
......@@ -302,17 +304,11 @@ bool MessagePort::dispatchEvent(PassRefPtr<Event> event, ExceptionCode& ec)
return !event->defaultPrevented();
}
void MessagePort::setPendingActivity()
{
ref();
++m_pendingActivity;
}
void MessagePort::unsetPendingActivity()
bool MessagePort::hasPendingActivity()
{
ASSERT(m_pendingActivity > 0);
--m_pendingActivity;
deref();
// We only care about the result of this function when there is no entangled port, or it is inaccessible, so no more messages can be added to the queue.
// Thus, using MessageQueue::isEmpty() does not cause a race condition here.
return m_pendingCloseEvent || (m_queueIsOpen && !m_messageQueue.isEmpty());
}
} // namespace WebCore
......@@ -44,6 +44,7 @@ namespace WebCore {
class Event;
class Frame;
class String;
class WorkerContext;
class MessagePort : public ThreadSafeShared<MessagePort>, public EventTarget {
public:
......@@ -67,6 +68,7 @@ namespace WebCore {
void contextDestroyed();
Document* document() { return m_document; }
WorkerContext* workerContext() { return 0; } // Not implemented yet.
virtual MessagePort* toMessagePort() { return this; }
......@@ -86,7 +88,7 @@ namespace WebCore {
using ThreadSafeShared<MessagePort>::ref;
using ThreadSafeShared<MessagePort>::deref;
bool hasPendingActivity() { return m_pendingActivity; }
bool hasPendingActivity();
void setOnmessage(PassRefPtr<EventListener> eventListener) { m_onMessageListener = eventListener; }
EventListener* onmessage() const { return m_onMessageListener.get(); }
......@@ -107,9 +109,6 @@ namespace WebCore {
void dispatchCloseEvent();
void setPendingActivity();
void unsetPendingActivity();
MessagePort* m_entangledPort;
MessageQueue<RefPtr<Event> > m_messageQueue;
bool m_queueIsOpen;
......@@ -121,7 +120,7 @@ namespace WebCore {
EventListenersMap m_eventListeners;
unsigned m_pendingActivity;
bool m_pendingCloseEvent;
bool m_jsWrapperIsInaccessible;
};
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment