Bug 113256

Summary: Crashes in NetworkProcess due to threading issues
Product: WebKit Reporter: Alexey Proskuryakov <ap>
Component: Page LoadingAssignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, gtk-ews, peter+ews, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
try EWS
webkit-ews: commit-queue-
more EWS testing
peter+ews: commit-queue-
proposed fix beidson: review+

Description Alexey Proskuryakov 2013-03-25 17:14:01 PDT 
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>
Comment 1 Alexey Proskuryakov 2013-03-25 17:14:48 PDT 
Created attachment 194956 [details]
try EWS
Comment 2 WebKit Review Bot 2013-03-25 17:18:02 PDT 
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.
Comment 3 Early Warning System Bot 2013-03-25 17:40:40 PDT 
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
Comment 4 EFL EWS Bot 2013-03-25 17:48:37 PDT 
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
Comment 5 Early Warning System Bot 2013-03-25 17:51:35 PDT 
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
Comment 6 kov's GTK+ EWS bot 2013-03-25 17:54:56 PDT 
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
Comment 7 Alexey Proskuryakov 2013-03-25 17:57:17 PDT 
Created attachment 194966 [details]
more EWS testing
Comment 8 WebKit Review Bot 2013-03-25 18:01:21 PDT 
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.
Comment 9 Peter Beverloo (cr-android ews) 2013-03-25 19:20:00 PDT 
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
Comment 10 WebKit Review Bot 2013-03-25 19:29:47 PDT 
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
Comment 11 WebKit Review Bot 2013-03-25 19:30:22 PDT 
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
Comment 12 Alexey Proskuryakov 2013-03-26 10:31:56 PDT 
Created attachment 195111 [details]
proposed fix
Comment 13 WebKit Review Bot 2013-03-26 10:35:09 PDT 
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.
Comment 14 Brady Eidson 2013-03-26 14:03:21 PDT 
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
Comment 15 Alexey Proskuryakov 2013-03-26 14:30:24 PDT 
Committed <http://trac.webkit.org/r146929>.
Comment 16 Alexey Proskuryakov 2013-03-26 14:45:25 PDT 
Some inconsequential cleanup in r146931.