2009-10-27 Fumitoshi Ukai <ukai@chromium.org>
authoreric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Oct 2009 17:00:52 +0000 (17:00 +0000)
committereric@webkit.org <eric@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 27 Oct 2009 17:00:52 +0000 (17:00 +0000)
        Reviewed by Dimitri Glazkov.

        Fix crash found in chromium test_shell.
        https://bugs.webkit.org/show_bug.cgi?id=30808

        When WebSocket is deleted without close, webkit would crash
        when it handles didClose.

        Check scriptExecutionContext before post task for event.
        Use WebSocketChannel::disconnect() instead of close() in WebSocket
        destructor, so that WebSocketChannel should not call deleted WebSocket
        back in didClose().
        To make sure WebSocketChannel alive while it is processing WebSocket
        protocol over SocketStreamHandle, ref() in connect() and deref() in
        didClose().

        * websockets/WebSocket.cpp:
        (WebCore::WebSocket::~WebSocket):
        (WebCore::WebSocket::didConnect):
        (WebCore::WebSocket::didReceiveMessage):
        (WebCore::WebSocket::didClose):
        * websockets/WebSocketChannel.cpp:
        (WebCore::WebSocketChannel::connect):
        (WebCore::WebSocketChannel::disconnect):
        (WebCore::WebSocketChannel::didClose):
        (WebCore::WebSocketChannel::didReceiveData):
        * websockets/WebSocketChannel.h:

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

WebCore/ChangeLog
WebCore/websockets/WebSocket.cpp
WebCore/websockets/WebSocketChannel.cpp
WebCore/websockets/WebSocketChannel.h

index bdba721..73042d1 100644 (file)
@@ -1,3 +1,33 @@
+2009-10-27  Fumitoshi Ukai  <ukai@chromium.org>
+
+        Reviewed by Dimitri Glazkov.
+
+        Fix crash found in chromium test_shell.
+        https://bugs.webkit.org/show_bug.cgi?id=30808
+
+        When WebSocket is deleted without close, webkit would crash
+        when it handles didClose.
+
+        Check scriptExecutionContext before post task for event.
+        Use WebSocketChannel::disconnect() instead of close() in WebSocket
+        destructor, so that WebSocketChannel should not call deleted WebSocket
+        back in didClose().
+        To make sure WebSocketChannel alive while it is processing WebSocket
+        protocol over SocketStreamHandle, ref() in connect() and deref() in
+        didClose().
+
+        * websockets/WebSocket.cpp:
+        (WebCore::WebSocket::~WebSocket):
+        (WebCore::WebSocket::didConnect):
+        (WebCore::WebSocket::didReceiveMessage):
+        (WebCore::WebSocket::didClose):
+        * websockets/WebSocketChannel.cpp:
+        (WebCore::WebSocketChannel::connect):
+        (WebCore::WebSocketChannel::disconnect):
+        (WebCore::WebSocketChannel::didClose):
+        (WebCore::WebSocketChannel::didReceiveData):
+        * websockets/WebSocketChannel.h:
+
 2009-10-27  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Darin Adler.
index f3bbdd7..32e0559 100644 (file)
@@ -110,7 +110,8 @@ WebSocket::WebSocket(ScriptExecutionContext* context)
 
 WebSocket::~WebSocket()
 {
-    close();
+    if (m_channel.get())
+        m_channel->disconnect();
 }
 
 void WebSocket::connect(const KURL& url, ExceptionCode& ec)
@@ -190,7 +191,7 @@ ScriptExecutionContext* WebSocket::scriptExecutionContext() const
 void WebSocket::didConnect()
 {
     LOG(Network, "WebSocket %p didConnect", this);
-    if (m_state != CONNECTING) {
+    if (m_state != CONNECTING || !scriptExecutionContext()) {
         didClose();
         return;
     }
@@ -201,7 +202,7 @@ void WebSocket::didConnect()
 void WebSocket::didReceiveMessage(const String& msg)
 {
     LOG(Network, "WebSocket %p didReceiveMessage %s", this, msg.utf8().data());
-    if (m_state != OPEN)
+    if (m_state != OPEN || !scriptExecutionContext())
         return;
     RefPtr<MessageEvent> evt = MessageEvent::create();
     // FIXME: origin, lastEventId, source, messagePort.
@@ -213,7 +214,8 @@ void WebSocket::didClose()
 {
     LOG(Network, "WebSocket %p didClose", this);
     m_state = CLOSED;
-    scriptExecutionContext()->postTask(ProcessWebSocketEventTask::create(this, Event::create(eventNames().closeEvent, false, false)));
+    if (scriptExecutionContext())
+        scriptExecutionContext()->postTask(ProcessWebSocketEventTask::create(this, Event::create(eventNames().closeEvent, false, false)));
 }
 
 EventTargetData* WebSocket::eventTargetData()
index 5cbbd24..be388b4 100644 (file)
@@ -71,6 +71,7 @@ void WebSocketChannel::connect()
     LOG(Network, "WebSocketChannel %p connect", this);
     ASSERT(!m_handle.get());
     m_handshake.reset();
+    ref();
     m_handle = SocketStreamHandle::create(m_handshake.url(), this);
 }
 
@@ -103,6 +104,14 @@ void WebSocketChannel::close()
         m_handle->close();  // will call didClose()
 }
 
+void WebSocketChannel::disconnect()
+{
+    LOG(Network, "WebSocketChannel %p disconnect", this);
+    m_client = 0;
+    if (m_handle.get())
+        m_handle->close();
+}
+
 void WebSocketChannel::willOpenStream(SocketStreamHandle*, const KURL&)
 {
 }
@@ -126,13 +135,15 @@ void WebSocketChannel::didClose(SocketStreamHandle* handle)
 {
     LOG(Network, "WebSocketChannel %p didClose", this);
     ASSERT(handle == m_handle.get() || !m_handle.get());
-    if (!m_handle.get())
-        return;
-    m_unhandledBufferSize = handle->bufferedAmount();
-    WebSocketChannelClient* client = m_client;
-    m_client = 0;
-    m_handle = 0;
-    client->didClose();
+    if (m_handle.get()) {
+        m_unhandledBufferSize = handle->bufferedAmount();
+        WebSocketChannelClient* client = m_client;
+        m_client = 0;
+        m_handle = 0;
+        if (client)
+            client->didClose();
+    }
+    deref();
 }
 
 void WebSocketChannel::didReceiveData(SocketStreamHandle* handle, const char* data, int len)
@@ -143,6 +154,10 @@ void WebSocketChannel::didReceiveData(SocketStreamHandle* handle, const char* da
         handle->close();
         return;
     }
+    if (!m_client) {
+        handle->close();
+        return;
+    }
     if (m_handshake.mode() != WebSocketHandshake::Connected) {
         int headerLength = m_handshake.readServerHandshake(m_buffer, m_bufferSize);
         if (headerLength <= 0)
index 268d113..ad38163 100644 (file)
@@ -57,6 +57,8 @@ namespace WebCore {
 
         virtual void close();
 
+        virtual void disconnect();
+
         virtual void willOpenStream(SocketStreamHandle*, const KURL&);
         virtual void willSendData(SocketStreamHandle*, const char*, int);
         virtual void didOpen(SocketStreamHandle*);