Changeset 211734 in webkit

Timestamp:

Feb 6, 2017 10:00:52 AM (7 years ago)

Author:

Carlos Garcia Campos

Message:

[Soup] Deadlock in NetworkProcess
​https://bugs.webkit.org/show_bug.cgi?id=167876

Reviewed by Michael Catanzaro.

WebKitSoupRequestInputStream uses a read lock. What is happening is that webkitSoupRequestInputStreamAddData
takes the lock, and it calls webkitSoupRequestInputStreamPendingReadAsyncComplete with the lock help. That
causes webkitSoupRequestInputStreamReadAsync to be called again to read the next chunk, but in the same run loop
operation. We don't really need the read lock because both webkitSoupRequestInputStreamAddData and
webkitSoupRequestInputStreamReadAsync shoudl always be called from the main thread.

• WebProcess/soup/WebKitSoupRequestInputStream.cpp:

(webkitSoupRequestInputStreamReadAsync): Remove the read lock and assert if called from a secondary thread.
(webkitSoupRequestInputStreamAddData): Ditto.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• WebProcess/soup/WebKitSoupRequestInputStream.cpp (modified) (4 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2017-02-06  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [Soup] Deadlock in NetworkProcess
+        https://bugs.webkit.org/show_bug.cgi?id=167876
+
+        Reviewed by Michael Catanzaro.
+
+        WebKitSoupRequestInputStream uses a read lock. What is happening is that webkitSoupRequestInputStreamAddData
+        takes the lock, and it calls webkitSoupRequestInputStreamPendingReadAsyncComplete with the lock help. That
+        causes webkitSoupRequestInputStreamReadAsync to be called again to read the next chunk, but in the same run loop
+        operation. We don't really need the read lock because both webkitSoupRequestInputStreamAddData and
+        webkitSoupRequestInputStreamReadAsync shoudl always be called from the main thread.
+
+        * WebProcess/soup/WebKitSoupRequestInputStream.cpp:
+        (webkitSoupRequestInputStreamReadAsync): Remove the read lock and assert if called from a secondary thread.
+        (webkitSoupRequestInputStreamAddData): Ditto.
+
 2017-02-03  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/WebProcess/soup/WebKitSoupRequestInputStream.cpp

@@ -21 +21 @@
 #include "WebKitSoupRequestInputStream.h"
 
-#include <wtf/Lock.h>
-#include <wtf/Threading.h>
+#include <wtf/MainThread.h>
 #include <wtf/glib/GRefPtr.h>
 #include <wtf/glib/GUniquePtr.h>
@@ -46 +45 @@
     GUniquePtr<GError> error;
 
-    Lock readLock;
     std::unique_ptr<AsyncReadData> pendingAsyncRead;
 };
@@ -86 +84 @@
 static void webkitSoupRequestInputStreamReadAsync(GInputStream* inputStream, void* buffer, gsize count, int /*priority*/, GCancellable* cancellable, GAsyncReadyCallback callback, gpointer userData)
 {
+    ASSERT(isMainThread());
     WebKitSoupRequestInputStream* stream = WEBKIT_SOUP_REQUEST_INPUT_STREAM(inputStream);
     GRefPtr<GTask> task = adoptGRef(g_task_new(stream, cancellable, callback, userData));
-
-    LockHolder locker(stream->priv->readLock);
 
     if (!webkitSoupRequestInputStreamHasDataToRead(stream) && !webkitSoupRequestInputStreamIsWaitingForData(stream)) {
@@ -150 +147 @@
 void webkitSoupRequestInputStreamAddData(WebKitSoupRequestInputStream* stream, const void* data, size_t dataLength)
 {
+    ASSERT(isMainThread());
+
     if (webkitSoupRequestInputStreamFinished(stream))
         return;
-
-    LockHolder locker(stream->priv->readLock);
 
     if (dataLength) {