Summary: | [Soup] Deadlock in NetworkProcess | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tomas Popela <tpopela> | ||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | bugs-noreply, cgarcia | ||||
Priority: | P2 | ||||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugzilla.redhat.com/show_bug.cgi?id=1418413 | ||||||
Attachments: |
|
Description
Tomas Popela
2017-02-06 05:45:23 PST
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. webkitSoupRequestInputStreamReadAsync also takes the read lock. I don't know why we are using that read lock, I don't think it's needed, at least now everything should happen in the main thread. But I'm going to look at it in more detail. If the lock is needed, then the solution is to release it before calling webkitSoupRequestInputStreamPendingReadAsyncComplete According to myself in https://bugs.webkit.org/show_bug.cgi?id=85880#c9 the mutex is needed, because glib will use a thread to implement read_async, and webkitSoupRequestInputStreamAddData can be called from other thread. Before r210374, that thread was the message work queue, that's why this never happened. Now it's the main thread, so I guess we just need to release the lock. (In reply to comment #2) > According to myself in https://bugs.webkit.org/show_bug.cgi?id=85880#c9 the > mutex is needed, because glib will use a thread to implement read_async, and > webkitSoupRequestInputStreamAddData can be called from other thread. Before > r210374, that thread was the message work queue, that's why this never > happened. Now it's the main thread, so I guess we just need to release the > lock. But I was wrong, because we are overriding GInputStreamClass::read_async, claiming we know how to handle the async request, and therefore not using the fallback implementation in GInputStream that runs the thread. It makes sense, indeed, because we are actually a GMemoryInputStream, there's no io in the read function, so we can handle it in the main thread. Created attachment 300716 [details]
Patch
Committed r211734: <http://trac.webkit.org/changeset/211734> |