Bug 113256 - Crashes in NetworkProcess due to threading issues
Summary: Crashes in NetworkProcess due to threading issues
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-03-25 17:14 PDT by Alexey Proskuryakov
Modified: 2013-03-26 14:45 PDT (History)
6 users (show)

See Also:


Attachments
try EWS (83.39 KB, patch)
2013-03-25 17:14 PDT, Alexey Proskuryakov
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
more EWS testing (83.43 KB, patch)
2013-03-25 17:57 PDT, Alexey Proskuryakov
peter+ews: commit-queue-
Details | Formatted Diff | Diff
proposed fix (94.21 KB, patch)
2013-03-26 10:31 PDT, Alexey Proskuryakov
beidson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.