Bug 113256 – Crashes in NetworkProcess due to threading issues

• Component: Page Loading
• Status: RESOLVED FIXED

[Alexey Proskuryakov]

NetworkProcess happily uses WebCore code from secondary threads. A few examples I found:
1. WebCoreCredentialStorage is now used from secondary threads, and has no locking whatsoever.
2. AuthenticaitonManager is now used from secondary threads, and m_challenges set is not protected.
3. ResourceRequests are used from multiple threads, and they contain AtomicStrings.

ResourceHandle code in general was never expected ot run fom non-main threads, so there can be more issues.

<rdar://problem/13194263>

[Alexey Proskuryakov]

Created attachment 194956 [details]
try EWS

[WebKit Review Bot]

Attachment 194956 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/network/CredentialStorage.cpp', u'Source/WebCore/platform/network/ResourceHandle.cpp', u'Source/WebCore/platform/network/ResourceHandle.h', u'Source/WebCore/platform/network/ResourceHandleClient.cpp', u'Source/WebCore/platform/network/ResourceHandleClient.h', u'Source/WebCore/platform/network/ResourceHandleInternal.h', u'Source/WebCore/platform/network/mac/ResourceHandleMac.mm', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm', u'Source/WebKit2/DerivedSources.make', u'Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.h', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.messages.in', u'Source/WebKit2/Shared/Authentication/AuthenticationManager.cpp', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/Network/NetworkProcessConnection.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.h', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.messages.in']" exit_code: 1
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 2 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Early Warning System Bot]

Comment on attachment 194956 [details]
try EWS

Attachment 194956 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17319102

[EFL EWS Bot]

Comment on attachment 194956 [details]
try EWS

Attachment 194956 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17325104

[Early Warning System Bot]

Comment on attachment 194956 [details]
try EWS

Attachment 194956 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17297306

[kov's GTK+ EWS bot]

Comment on attachment 194956 [details]
try EWS

Attachment 194956 [details] did not pass gtk-ews (gtk):
Output: http://webkit-commit-queue.appspot.com/results/17297308

[Alexey Proskuryakov]

Created attachment 194966 [details]
more EWS testing

[WebKit Review Bot]

Attachment 194966 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/network/CredentialStorage.cpp', u'Source/WebCore/platform/network/ResourceHandle.cpp', u'Source/WebCore/platform/network/ResourceHandle.h', u'Source/WebCore/platform/network/ResourceHandleClient.cpp', u'Source/WebCore/platform/network/ResourceHandleClient.h', u'Source/WebCore/platform/network/ResourceHandleInternal.h', u'Source/WebCore/platform/network/mac/ResourceHandleMac.mm', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm', u'Source/WebKit2/DerivedSources.make', u'Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.h', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.messages.in', u'Source/WebKit2/Shared/Authentication/AuthenticationManager.cpp', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/Network/NetworkProcessConnection.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.h', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.messages.in']" exit_code: 1
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Peter Beverloo (cr-android ews)]

Comment on attachment 194966 [details]
more EWS testing

Attachment 194966 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17227574

[WebKit Review Bot]

Comment on attachment 194966 [details]
more EWS testing

Attachment 194966 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17328153

[WebKit Review Bot]

Comment on attachment 194966 [details]
more EWS testing

Attachment 194966 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17255432

[Alexey Proskuryakov]

Created attachment 195111 [details]
proposed fix

[WebKit Review Bot]

Attachment 195111 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/platform/network/CredentialStorage.cpp', u'Source/WebCore/platform/network/ResourceHandle.cpp', u'Source/WebCore/platform/network/ResourceHandle.h', u'Source/WebCore/platform/network/ResourceHandleClient.cpp', u'Source/WebCore/platform/network/ResourceHandleClient.h', u'Source/WebCore/platform/network/ResourceHandleInternal.h', u'Source/WebCore/platform/network/chromium/ResourceHandle.cpp', u'Source/WebCore/platform/network/mac/ResourceHandleMac.mm', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsDelegate.mm', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h', u'Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.mm', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/DerivedSources.make', u'Source/WebKit2/NetworkProcess/NetworkConnectionToWebProcess.cpp', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.h', u'Source/WebKit2/NetworkProcess/NetworkResourceLoader.messages.in', u'Source/WebKit2/Shared/Authentication/AuthenticationManager.cpp', u'Source/WebKit2/WebKit2.xcodeproj/project.pbxproj', u'Source/WebKit2/WebProcess/Network/NetworkProcessConnection.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.cpp', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.h', u'Source/WebKit2/WebProcess/Network/WebResourceLoader.messages.in']" exit_code: 1
Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 25 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Brady Eidson]

Comment on attachment 195111 [details]
proposed fix

View in context: https://bugs.webkit.org/attachment.cgi?id=195111&action=review

Scary

> Source/WebCore/platform/network/mac/WebCoreResourceHandleAsOperationQueueDelegate.h:36
>  namespace WebCore {
>      class ResourceHandle;
>  }

Indeed!  Don't care that it's old code, either.

#petpeeve

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/r146929>.

[Alexey Proskuryakov]

Some inconsequential cleanup in r146931.