Commit a0c037ca authored by ap@webkit.org's avatar ap@webkit.org

Reviewed by Sam Weinig.

        https://bugs.webkit.org/show_bug.cgi?id=21213
        MessagePort crash when GC collects an object with a pending close event

        Test: fast/events/message-channel-gc-2.html
              fast/events/message-channel-listener-circular-ownership.html

        * bindings/DOMProtect.cpp: Added.
        (WebCore::gcProtectDOMObject):
        (WebCore::gcUnprotectDOMObject):
        * bindings/DOMProtect.h: Added.
        Added an abstraction for GC protection to avoid the need to call JS bindings code from
        DOM objects directly.

        * dom/MessagePort.cpp:
        (WebCore::CloseMessagePortTimer::fired):
        (WebCore::MessagePort::queueCloseEvent):
        GC protect MessagePort wrapper while there is a pending close event.
        This may be necessary for message events, too, but that case is not a crasher, and actually
        behaves to the letter of the current HTML5 text, so I'll consider it later.

        * xml/XMLHttpRequest.cpp:
        (WebCore::XMLHttpRequest::loadRequestAsynchronously):
        (WebCore::XMLHttpRequest::dropProtection):
        Use gcProtectDOMObject here, too. Unfortunately, XMLHttpRequest has more dependencies on JSC.

        * bindings/js/JSMessagePortCustom.cpp:
        (WebCore::JSMessagePort::addEventListener):
        (WebCore::JSMessagePort::removeEventListener):
        (WebCore::JSMessagePort::setOnmessage):
        (WebCore::JSMessagePort::setOnclose):
        Don't tell DOMWindowBase that MessagePort is a NodeEventTarget, this is not true. I do not
        know if this was causing any real issues, but we shouldn't lie to DOMWindowBase.

        * bindings/js/JSXMLHttpRequestUploadCustom.cpp:
        (WebCore::JSXMLHttpRequestUpload::mark):
        While at it, changed to use a typedef for event listeners from XMLHttpRequestUpload, not
        from XMLHttpRequest.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@37094 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent d227201a
2008-09-30 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Sam Weinig.
https://bugs.webkit.org/show_bug.cgi?id=21213
MessagePort crash when GC collects an object with a pending close event
* fast/events/message-channel-gc-2-expected.txt: Added.
* fast/events/message-channel-gc-2.html: Added.
* fast/events/message-channel-listener-circular-ownership-expected.txt: Added
* fast/events/message-channel-listener-circular-ownership.html: Added
Demonstrate why turning the listeners into JSEventListeners wouldn't be right.
2008-09-29 Geoffrey Garen <ggaren@apple.com>
Reviewed by Cameron Zwarich.
......
Test that MessagePort close event gets delivered (without crashing) even if the channel object is garbage collected.
onclose
close listener
<body>
<p>Test that MessagePort close event gets delivered (without crashing) even if the channel object is garbage collected.</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();
}
var channel = new MessageChannel;
channel.port1.onclose = function() { log("onclose"); setTimeout(function() { if (window.layoutTestController) layoutTestController.notifyDone(); gc(); }, 0) }
channel.port1.addEventListener("close", function() { log("close listener"); gc(); }, false);
gc();
channel.port1.start();
channel.port2.postMessage("msg");
gc();
channel.port1.close();
channel.port2.close();
gc();
channel = new MessageChannel;
gc();
</script>
</body>
Test that a leak is not created by assigning a MessagePort to a property of its own listener.
Only works with run-webkit-tests --leaks.
<body>
<p>Test that a leak is not created by assigning a MessagePort to a property of its own listener.</p>
<p>Only works with run-webkit-tests --leaks.</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");
}
}
if (window.layoutTestController)
layoutTestController.dumpAsText();
var channel = new MessageChannel;
channel.port1.onclose = function() {}
channel.port1.onclose.foo = channel;
channel.port1.onclose.bar = channel.port1;
channel.port1.onclose.baz = channel.port2;
channel = null;
gc();
</script>
</body>
2008-09-30 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Sam Weinig.
https://bugs.webkit.org/show_bug.cgi?id=21213
MessagePort crash when GC collects an object with a pending close event
Test: fast/events/message-channel-gc-2.html
fast/events/message-channel-listener-circular-ownership.html
* bindings/DOMProtect.cpp: Added.
(WebCore::gcProtectDOMObject):
(WebCore::gcUnprotectDOMObject):
* bindings/DOMProtect.h: Added.
Added an abstraction for GC protection to avoid the need to call JS bindings code from
DOM objects directly.
* dom/MessagePort.cpp:
(WebCore::CloseMessagePortTimer::fired):
(WebCore::MessagePort::queueCloseEvent):
GC protect MessagePort wrapper while there is a pending close event.
This may be necessary for message events, too, but that case is not a crasher, and actually
behaves to the letter of the current HTML5 text, so I'll consider it later.
* xml/XMLHttpRequest.cpp:
(WebCore::XMLHttpRequest::loadRequestAsynchronously):
(WebCore::XMLHttpRequest::dropProtection):
Use gcProtectDOMObject here, too. Unfortunately, XMLHttpRequest has more dependencies on JSC.
* bindings/js/JSMessagePortCustom.cpp:
(WebCore::JSMessagePort::addEventListener):
(WebCore::JSMessagePort::removeEventListener):
(WebCore::JSMessagePort::setOnmessage):
(WebCore::JSMessagePort::setOnclose):
Don't tell DOMWindowBase that MessagePort is a NodeEventTarget, this is not true. I do not
know if this was causing any real issues, but we shouldn't lie to DOMWindowBase.
* bindings/js/JSXMLHttpRequestUploadCustom.cpp:
(WebCore::JSXMLHttpRequestUpload::mark):
While at it, changed to use a typedef for event listeners from XMLHttpRequestUpload, not
from XMLHttpRequest.
2008-09-30 Adam Roben <aroben@apple.com>
Windows build fix
......
......@@ -4151,6 +4151,8 @@
E1ADEDDB0E76BD93004A1A5E /* JSMessagePort.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1ADEDD90E76BD93004A1A5E /* JSMessagePort.cpp */; };
E1BE512D0CF6C512002EA959 /* XSLTUnicodeSort.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1BE512B0CF6C512002EA959 /* XSLTUnicodeSort.cpp */; };
E1BE512E0CF6C512002EA959 /* XSLTUnicodeSort.h in Headers */ = {isa = PBXBuildFile; fileRef = E1BE512C0CF6C512002EA959 /* XSLTUnicodeSort.h */; };
E1CA84920E923BE900114251 /* DOMProtect.h in Headers */ = {isa = PBXBuildFile; fileRef = E1CA84910E923BE900114251 /* DOMProtect.h */; };
E1CA84940E923BEF00114251 /* DOMProtect.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1CA84930E923BEF00114251 /* DOMProtect.cpp */; };
E1E6EEA40B628DA8005F2F70 /* JSHTMLSelectElement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1E6EEA30B628DA8005F2F70 /* JSHTMLSelectElement.cpp */; };
E1E6EEA80B628DB3005F2F70 /* JSHTMLSelectElement.h in Headers */ = {isa = PBXBuildFile; fileRef = E1E6EEA70B628DB3005F2F70 /* JSHTMLSelectElement.h */; };
E1EBBBD40AAC9B87001FE8E2 /* CSSCharsetRule.cpp in Sources */ = {isa = PBXBuildFile; fileRef = E1EBBBD30AAC9B87001FE8E2 /* CSSCharsetRule.cpp */; };
......@@ -8713,6 +8715,8 @@
E1ADEDD90E76BD93004A1A5E /* JSMessagePort.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSMessagePort.cpp; sourceTree = "<group>"; };
E1BE512B0CF6C512002EA959 /* XSLTUnicodeSort.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = XSLTUnicodeSort.cpp; sourceTree = "<group>"; };
E1BE512C0CF6C512002EA959 /* XSLTUnicodeSort.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = XSLTUnicodeSort.h; sourceTree = "<group>"; };
E1CA84910E923BE900114251 /* DOMProtect.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DOMProtect.h; sourceTree = "<group>"; };
E1CA84930E923BEF00114251 /* DOMProtect.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = DOMProtect.cpp; sourceTree = "<group>"; };
E1E6EEA30B628DA8005F2F70 /* JSHTMLSelectElement.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSHTMLSelectElement.cpp; sourceTree = "<group>"; };
E1E6EEA70B628DB3005F2F70 /* JSHTMLSelectElement.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = JSHTMLSelectElement.h; sourceTree = "<group>"; };
E1EBBBD30AAC9B87001FE8E2 /* CSSCharsetRule.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CSSCharsetRule.cpp; sourceTree = "<group>"; };
......@@ -12681,6 +12685,8 @@
isa = PBXGroup;
children = (
93F8B3050A300FE100F61AB8 /* CodeGenerator.pm */,
E1CA84930E923BEF00114251 /* DOMProtect.cpp */,
E1CA84910E923BE900114251 /* DOMProtect.h */,
93F8B3070A300FEA00F61AB8 /* generate-bindings.pl */,
14813BF309EDF88E00F757E1 /* IDLParser.pm */,
93F8B3080A300FEA00F61AB8 /* IDLStructure.pm */,
......@@ -15955,6 +15961,7 @@
BC3B7AF40E919CA900D54065 /* JSEventTargetNode.h in Headers */,
BC60901F0E91B8EC000C68B5 /* JSEventTarget.h in Headers */,
BC3BC29C0E91AB0F00835588 /* HostWindow.h in Headers */,
E1CA84920E923BE900114251 /* DOMProtect.h in Headers */,
);
runOnlyForDeploymentPostprocessing = 0;
};
......@@ -17805,6 +17812,7 @@
BC3B7AF30E919CA900D54065 /* JSEventTargetNode.cpp in Sources */,
BC3B7B210E91AAF400D54065 /* JSEventTargetNodeCustom.cpp in Sources */,
BC6090200E91B8EC000C68B5 /* JSEventTarget.cpp in Sources */,
E1CA84940E923BEF00114251 /* DOMProtect.cpp in Sources */,
);
runOnlyForDeploymentPostprocessing = 0;
};
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
* OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
*/
#include "config.h"
#include "DOMProtect.h"
#include "JSDOMBinding.h"
#include <kjs/protect.h>
namespace WebCore {
void gcProtectDOMObject(void* objectHandle)
{
JSC::gcProtectNullTolerant(getCachedDOMObjectWrapper(objectHandle));
}
void gcUnprotectDOMObject(void* objectHandle)
{
JSC::gcUnprotectNullTolerant(getCachedDOMObjectWrapper(objectHandle));
}
} // namespace WebCore
/*
* Copyright (C) 2008 Apple Inc. All Rights Reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
* EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
* PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
* OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
*
*/
#ifndef DOMProtect_h
#define DOMProtect_h
namespace WebCore {
void gcProtectDOMObject(void*);
void gcUnprotectDOMObject(void*);
} // namespace WebCore
#endif // DOMProtect_h
......@@ -54,7 +54,7 @@ JSValue* JSMessagePort::addEventListener(ExecState* exec, const ArgList& args)
JSDOMWindow* window = toJSDOMWindow(document->frame());
if (!window)
return jsUndefined();
RefPtr<JSUnprotectedEventListener> listener = window->findOrCreateJSUnprotectedEventListener(exec, args.at(exec, 1), true);
RefPtr<JSUnprotectedEventListener> listener = window->findOrCreateJSUnprotectedEventListener(exec, args.at(exec, 1));
if (!listener)
return jsUndefined();
impl()->addEventListener(args.at(exec, 0)->toString(exec), listener.release(), args.at(exec, 2)->toBoolean(exec));
......@@ -69,7 +69,7 @@ JSValue* JSMessagePort::removeEventListener(ExecState* exec, const ArgList& args
JSDOMWindow* window = toJSDOMWindow(document->frame());
if (!window)
return jsUndefined();
JSUnprotectedEventListener* listener = window->findJSUnprotectedEventListener(exec, args.at(exec, 1), true);
JSUnprotectedEventListener* listener = window->findJSUnprotectedEventListener(exec, args.at(exec, 1));
if (!listener)
return jsUndefined();
impl()->removeEventListener(args.at(exec, 0)->toString(exec), listener, args.at(exec, 2)->toBoolean(exec));
......@@ -94,7 +94,7 @@ void JSMessagePort::setOnmessage(ExecState* exec, JSValue* value)
JSDOMWindow* window = toJSDOMWindow(document->frame());
if (!window)
return;
impl()->setOnMessageListener(window->findOrCreateJSUnprotectedEventListener(exec, value, true));
impl()->setOnMessageListener(window->findOrCreateJSUnprotectedEventListener(exec, value));
}
JSValue* JSMessagePort::onmessage(ExecState*) const
......@@ -113,7 +113,7 @@ void JSMessagePort::setOnclose(ExecState* exec, JSValue* value)
JSDOMWindow* window = toJSDOMWindow(document->frame());
if (!window)
return;
impl()->setOnCloseListener(window->findOrCreateJSUnprotectedEventListener(exec, value, true));
impl()->setOnCloseListener(window->findOrCreateJSUnprotectedEventListener(exec, value));
}
JSValue* JSMessagePort::onclose(ExecState*) const
......
......@@ -66,8 +66,8 @@ void JSXMLHttpRequestUpload::mark()
if (JSUnprotectedEventListener* onProgressListener = static_cast<JSUnprotectedEventListener*>(m_impl->onProgressListener()))
onProgressListener->mark();
typedef XMLHttpRequest::EventListenersMap EventListenersMap;
typedef XMLHttpRequest::ListenerVector ListenerVector;
typedef XMLHttpRequestUpload::EventListenersMap EventListenersMap;
typedef XMLHttpRequestUpload::ListenerVector ListenerVector;
EventListenersMap& eventListeners = m_impl->eventListeners();
for (EventListenersMap::iterator mapIter = eventListeners.begin(); mapIter != eventListeners.end(); ++mapIter) {
for (ListenerVector::iterator vecIter = mapIter->second.begin(); vecIter != mapIter->second.end(); ++vecIter) {
......
......@@ -28,8 +28,9 @@
#include "MessagePort.h"
#include "AtomicString.h"
#include "Document.h"
#include "DOMProtect.h"
#include "DOMWindow.h"
#include "Document.h"
#include "EventException.h"
#include "EventNames.h"
#include "MessageEvent.h"
......@@ -56,6 +57,7 @@ private:
m_port->dispatchMessages();
m_port->dispatchCloseEvent();
gcUnprotectDOMObject(m_port.get());
delete this;
}
......@@ -209,6 +211,9 @@ void MessagePort::dispatchMessages()
void MessagePort::queueCloseEvent()
{
// Need to keep listeners alive, and they are marked by the wrapper.
gcProtectDOMObject(this);
CloseMessagePortTimer* timer = new CloseMessagePortTimer(this);
timer->startOneShot(0);
}
......
......@@ -24,6 +24,7 @@
#include "CString.h"
#include "Console.h"
#include "DOMImplementation.h"
#include "DOMProtect.h"
#include "DOMWindow.h"
#include "Event.h"
#include "EventException.h"
......@@ -47,7 +48,6 @@
#include "XMLHttpRequestUpload.h"
#include "markup.h"
#include <kjs/JSLock.h>
#include <kjs/protect.h>
namespace WebCore {
......@@ -765,8 +765,7 @@ void XMLHttpRequest::loadRequestAsynchronously(ResourceRequest& request)
// a request is in progress because we need to keep the listeners alive,
// and they are referenced by the JavaScript wrapper.
ref();
JSC::gcProtectNullTolerant(getCachedDOMObjectWrapper(this));
gcProtectDOMObject(this);
}
}
......@@ -874,11 +873,10 @@ void XMLHttpRequest::dropProtection()
// report the extra cost at that point.
JSC::JSValue* wrapper = getCachedDOMObjectWrapper(this);
if (wrapper) {
JSC::gcUnprotect(wrapper);
if (wrapper)
JSC::Heap::heap(wrapper)->reportExtraMemoryCost(m_responseText.size() * 2);
}
gcUnprotectDOMObject(this);
deref();
}
......
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