Merge r268906 - [SOUP] Fix crash in WebSocketTask

https://bugs.webkit.org/show_bug.cgi?id=217892 Patch by Michael Catanzaro <mcatanzaro@gnome.org> on 2020-10-23 Reviewed by Carlos Garcia Campos. The WebSocketTask connects to the "starting" signal of its SoupMessage and never disconnects this signal, which is only safe if it is guaranteed to outlive its SoupMessage. However, it is not. We crash when the signal is emitted after the WebSocketTask is destroyed. To solve this, we just need to disconnect the signal when required. Normally that would be done in the destructor, but the WebSocketTask drops its ownership of the SoupMessage prior to that point, so we need to disconnect on each possible paths. * NetworkProcess/soup/WebSocketTaskSoup.cpp: (WebKit::WebSocketTask::~WebSocketTask): (WebKit::WebSocketTask::didConnect): (WebKit::WebSocketTask::didFail):
WebKit · Oct 23, 2020 · 1f6b01d · 1f6b01d
1 parent d7de385
commit 1f6b01d
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 0 deletions.
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
@@ -1,3 +1,22 @@
+2020-10-23  Michael Catanzaro  <mcatanzaro@gnome.org>
+
+        [SOUP] Fix crash in WebSocketTask
+        https://bugs.webkit.org/show_bug.cgi?id=217892
+
+        Reviewed by Carlos Garcia Campos.
+
+        The WebSocketTask connects to the "starting" signal of its SoupMessage and never disconnects
+        this signal, which is only safe if it is guaranteed to outlive its SoupMessage. However, it
+        is not. We crash when the signal is emitted after the WebSocketTask is destroyed. To solve
+        this, we just need to disconnect the signal when required. Normally that would be done in
+        the destructor, but the WebSocketTask drops its ownership of the SoupMessage prior to that
+        point, so we need to disconnect on each possible paths.
+
+        * NetworkProcess/soup/WebSocketTaskSoup.cpp:
+        (WebKit::WebSocketTask::~WebSocketTask):
+        (WebKit::WebSocketTask::didConnect):
+        (WebKit::WebSocketTask::didFail):
+
 2020-10-10  Adrian Perez de Castro  <aperez@igalia.com>
 
         [GTK] Build broken with ENABLE_GAMEPAD enabled

diff --git a/Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp b/Source/WebKit/NetworkProcess/soup/WebSocketTaskSoup.cpp
@@ -89,6 +89,9 @@ WebSocketTask::WebSocketTask(NetworkSocketChannel& channel, SoupSession* session
 
 WebSocketTask::~WebSocketTask()
 {
+    if (m_handshakeMessage)
+        g_signal_handlers_disconnect_by_data(m_handshakeMessage.get(), this);
+
     cancel();
 }
 
@@ -133,6 +136,7 @@ void WebSocketTask::didConnect(GRefPtr<SoupWebsocketConnection>&& connection)
     WebCore::ResourceResponse response;
     response.updateFromSoupMessage(m_handshakeMessage.get());
     m_channel.didReceiveHandshakeResponse(WTFMove(response));
+    g_signal_handlers_disconnect_by_data(m_handshakeMessage.get(), this);
     m_handshakeMessage = nullptr;
 }
 
@@ -172,6 +176,7 @@ void WebSocketTask::didFail(const String& errorMessage)
         WebCore::ResourceResponse response;
         response.updateFromSoupMessage(m_handshakeMessage.get());
         m_channel.didReceiveHandshakeResponse(WTFMove(response));
+        g_signal_handlers_disconnect_by_data(m_handshakeMessage.get(), this);
         m_handshakeMessage = nullptr;
     }
     m_channel.didReceiveMessageError(errorMessage);