Skip to content

Commit

Permalink
Merge r173848 - WebSocket crash when a connection is closed from serv…
Browse files Browse the repository at this point in the history
…er side

https://bugs.webkit.org/show_bug.cgi?id=137009
rdar://problem/18333977
rdar://problem/12708225

Reviewed by Brady Eidson.

I don't think that this can be tested with our test server.

* Modules/websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::WebSocketChannel): Added logging.
(WebCore::WebSocketChannel::~WebSocketChannel): Ditto.
(WebCore::WebSocketChannel::close): Protect self, because startClosingHandshake
can release the last reference.
(WebCore::WebSocketChannel::fail): Added an assertion that the channel is always
closed after this function.
(WebCore::WebSocketChannel::startClosingHandshake): Protect self, and don't change
the stack from closed back to closing if after failing to send closing handshake.
(WebCore::WebSocketChannel::processOutgoingFrameQueue): Protect self.

Canonical link: https://commits.webkit.org/154760.23@webkitgtk/2.6
git-svn-id: https://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.6@173917 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
aproskuryakov authored and carlosgcampos committed Sep 24, 2014
1 parent 0639290 commit ab57f23
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
22 changes: 22 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,25 @@
2014-09-22 Alexey Proskuryakov <ap@apple.com>

WebSocket crash when a connection is closed from server side
https://bugs.webkit.org/show_bug.cgi?id=137009
rdar://problem/18333977
rdar://problem/12708225

Reviewed by Brady Eidson.

I don't think that this can be tested with our test server.

* Modules/websockets/WebSocketChannel.cpp:
(WebCore::WebSocketChannel::WebSocketChannel): Added logging.
(WebCore::WebSocketChannel::~WebSocketChannel): Ditto.
(WebCore::WebSocketChannel::close): Protect self, because startClosingHandshake
can release the last reference.
(WebCore::WebSocketChannel::fail): Added an assertion that the channel is always
closed after this function.
(WebCore::WebSocketChannel::startClosingHandshake): Protect self, and don't change
the stack from closed back to closing if after failing to send closing handshake.
(WebCore::WebSocketChannel::processOutgoingFrameQueue): Protect self.

2014-09-22 David Hyatt <hyatt@apple.com>

Bad cast in isValidColumnSpanner.
Expand Down
15 changes: 15 additions & 0 deletions Source/WebCore/Modules/websockets/WebSocketChannel.cpp
Expand Up @@ -85,10 +85,13 @@ WebSocketChannel::WebSocketChannel(Document* document, WebSocketChannelClient* c
{
if (Page* page = m_document->page())
m_identifier = page->progress().createUniqueIdentifier();

LOG(Network, "WebSocketChannel %p ctor, identifier %lu", this, m_identifier);
}

WebSocketChannel::~WebSocketChannel()
{
LOG(Network, "WebSocketChannel %p dtor", this);
}

void WebSocketChannel::connect(const URL& url, const String& protocol)
Expand Down Expand Up @@ -181,6 +184,7 @@ void WebSocketChannel::close(int code, const String& reason)
ASSERT(!m_suspended);
if (!m_handle)
return;
Ref<WebSocketChannel> protect(*this); // An attempt to send closing handshake may fail, which will get the channel closed and dereferenced.
startClosingHandshake(code, reason);
if (m_closing && !m_closingTimer.isActive())
m_closingTimer.startOneShot(2 * TCPMaximumSegmentLifetime);
Expand Down Expand Up @@ -208,6 +212,8 @@ void WebSocketChannel::fail(const String& reason)

if (m_handle && !m_closed)
m_handle->disconnect(); // Will call didClose().

ASSERT(m_closed);
}

void WebSocketChannel::disconnect()
Expand Down Expand Up @@ -458,6 +464,7 @@ void WebSocketChannel::resumeTimerFired(Timer<WebSocketChannel>* timer)
void WebSocketChannel::startClosingHandshake(int code, const String& reason)
{
LOG(Network, "WebSocketChannel %p startClosingHandshake() code=%d m_receivedClosingHandshake=%d", this, m_closing, m_receivedClosingHandshake);
ASSERT(!m_closed);
if (m_closing)
return;
ASSERT(m_handle);
Expand All @@ -471,8 +478,14 @@ void WebSocketChannel::startClosingHandshake(int code, const String& reason)
buf.append(reason.utf8().data(), reason.utf8().length());
}
enqueueRawFrame(WebSocketFrame::OpCodeClose, buf.data(), buf.size());
Ref<WebSocketChannel> protect(*this); // An attempt to send closing handshake may fail, which will get the channel closed and dereferenced.
processOutgoingFrameQueue();

if (m_closed) {
// The channel got closed because processOutgoingFrameQueue() failed.
return;
}

m_closing = true;
if (m_client)
m_client->didStartClosingHandshake();
Expand Down Expand Up @@ -706,6 +719,8 @@ void WebSocketChannel::processOutgoingFrameQueue()
if (m_outgoingFrameQueueStatus == OutgoingFrameQueueClosed)
return;

Ref<WebSocketChannel> protect(*this); // Any call to fail() will get the channel closed and dereferenced.

while (!m_outgoingFrameQueue.isEmpty()) {
OwnPtr<QueuedFrame> frame = m_outgoingFrameQueue.takeFirst();
switch (frame->frameType) {
Expand Down

0 comments on commit ab57f23

Please sign in to comment.