Bug 183066 – Crash under SchemeRegistry::shouldTreatURLSchemeAsLocal(WTF::String const&)

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

Crash under SchemeRegistry::shouldTreatURLSchemeAsLocal(WTF::String const&):
Thread 0 Crashed ↩:: Dispatch queue: com.apple.main-thread
0   com.apple.WebCore             	0x00007fff4dc91800 unsigned int WTF::StringHasher::computeHashAndMaskTop8Bits<unsigned short, WTF::ASCIICaseInsensitiveHash::FoldCase<unsigned short> >(unsigned short const*, unsigned int) + 32
1   com.apple.WebCore             	0x00007fff4dcef1ae WTF::String* WTF::HashTable<WTF::String, WTF::String, WTF::IdentityExtractor, WTF::ASCIICaseInsensitiveHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::String> >::lookup<WTF::IdentityHashTranslator<WTF::HashTraits<WTF::String>, WTF::ASCIICaseInsensitiveHash>, WTF::String>(WTF::String const&) + 46
2   com.apple.WebCore             	0x00007fff4daa7f0d WebCore::SchemeRegistry::shouldTreatURLSchemeAsLocal(WTF::String const&) + 77
3   com.apple.WebCore             	0x00007fff4ea71e19 WebCore::shouldTreatAsPotentiallyTrustworthy(WebCore::URL const&) + 217
4   com.apple.WebCore             	0x00007fff4ea72390 WebCore::SecurityOrigin::SecurityOrigin(WebCore::URL const&) + 672
5   com.apple.WebCore             	0x00007fff4ea7270d WebCore::SecurityOrigin::create(WebCore::URL const&) + 573
6   com.apple.WebCore             	0x00007fff4ea72acb WebCore::SecurityOrigin::canRequest(WebCore::URL const&) const + 315
7   com.apple.WebCore             	0x00007fff4e99836a WebCore::DocumentThreadableLoader::DocumentThreadableLoader(WebCore::Document&, WebCore::ThreadableLoaderClient&, WebCore::DocumentThreadableLoader::BlockingBehavior, WebCore::ResourceRequest&&, WebCore::ThreadableLoaderOptions const&, WTF::RefPtr<WebCore::SecurityOrigin, WTF::DumbPtrTraits<WebCore::SecurityOrigin> >&&, std::__1::unique_ptr<WebCore::ContentSecurityPolicy, std::__1::default_delete<WebCore::ContentSecurityPolicy> >&&, WTF::String&&, WebCore::DocumentThreadableLoader::ShouldLogError) + 266
8   com.apple.WebCore             	0x00007fff4e998089 WebCore::DocumentThreadableLoader::create(WebCore::Document&, WebCore::ThreadableLoaderClient&, WebCore::ResourceRequest&&, WebCore::ThreadableLoaderOptions const&, WTF::RefPtr<WebCore::SecurityOrigin, WTF::DumbPtrTraits<WebCore::SecurityOrigin> >&&, std::__1::unique_ptr<WebCore::ContentSecurityPolicy, std::__1::default_delete<WebCore::ContentSecurityPolicy> >&&, WTF::String&&, WebCore::DocumentThreadableLoader::ShouldLogError) + 89
9   com.apple.WebCore             	0x00007fff4e9d6832 WTF::Function<void (WebCore::ScriptExecutionContext&)>::CallableWrapper<WebCore::WorkerThreadableLoader::MainThreadBridge::MainThreadBridge(WebCore::ThreadableLoaderClientWrapper&, WebCore::WorkerLoaderProxy&, WTF::String const&, WebCore::ResourceRequest&&, WebCore::ThreadableLoaderOptions const&, WTF::String const&, WebCore::WorkerGlobalScope&)::$_5>::call(WebCore::ScriptExecutionContext&) + 98
10  com.apple.JavaScriptCore      	0x00007fff434a8160 WTF::dispatchFunctionsFromMainThread() + 176
11  com.apple.Foundation          	0x00007fff4255de75 __NSThreadPerformPerform + 334
12  com.apple.CoreFoundation      	0x00007fff40398271 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
13  com.apple.CoreFoundation      	0x00007fff40451c6c __CFRunLoopDoSource0 + 108
14  com.apple.CoreFoundation      	0x00007fff4037adb0 __CFRunLoopDoSources0 + 208
15  com.apple.CoreFoundation      	0x00007fff4037a22d __CFRunLoopRun + 1293
16  com.apple.CoreFoundation      	0x00007fff40379a93 CFRunLoopRunSpecific + 483
17  com.apple.HIToolbox           	0x00007fff3f66aef6 RunCurrentEventLoopInMode + 286
18  com.apple.HIToolbox           	0x00007fff3f66ac66 ReceiveNextEventCommon + 613
19  com.apple.HIToolbox           	0x00007fff3f66a9e4 _BlockUntilNextEventMatchingListInModeWithFilter + 64
20  com.apple.AppKit              	0x00007fff3d91e4db _DPSNextEvent + 2085
21  com.apple.AppKit              	0x00007fff3e0b46f8 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 3044
22  com.apple.AppKit              	0x00007fff3d9132ed -[NSApplication run] + 764
23  com.apple.AppKit              	0x00007fff3d8e24c6 NSApplicationMain + 804
24  libxpc.dylib                  	0x00007fff68c60f83 _xpc_objc_main + 580
25  libxpc.dylib                  	0x00007fff68c5fbd6 xpc_main + 417
26  com.apple.WebKit.WebContent   	0x10dfc36a1 main + 490 (~rc/Software/LoboElk/Projects/WebKit2/WebKit2-7605.1.25.1/Shared/EntryPointUtilities/mac/XPCService/XPCServiceMain.mm:148)
27  libdyld.dylib                 	0x00007fff68912015 start + 1

[Radar WebKit Bug Importer]

<rdar://problem/37804111>

[Chris Dumez]

<rdar://problem/37019849>

[Chris Dumez]

Created attachment 334485 [details]
Patch

[EWS Watchlist]

Comment on attachment 334485 [details]
Patch

Attachment 334485 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/6630815

New failing tests:
http/tests/media/media-stream/get-user-media-loopback-ip.html
http/tests/media/media-stream/get-user-media-localhost.html

[EWS Watchlist]

Created attachment 334489 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Chris Dumez]

Created attachment 334494 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 334494 [details]
Patch

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

> Source/WebCore/page/SecurityOrigin.cpp:223
> +bool SecurityOrigin::isPotentiallyTrustworthy() const
> +{

Are you sure this function is only called by a single thread at a time?
If this function can be called synchronously from multiple threads, we have mutate m_isPotentiallyTrustworthy from multiple threads.

[Chris Dumez]

Comment on attachment 334494 [details]
Patch

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

>> Source/WebCore/page/SecurityOrigin.cpp:223
>> +{
> 
> Are you sure this function is only called by a single thread at a time?
> If this function can be called synchronously from multiple threads, we have mutate m_isPotentiallyTrustworthy from multiple threads.

I don't think this is guaranteed :/ SecurityOrigin are frequently passed across threads. You are right that there is a bug here.

[Chris Dumez]

Created attachment 334551 [details]
Patch

[Chris Dumez]

Created attachment 334552 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 334552 [details]
Patch

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

> Source/WebCore/page/SecurityOrigin.cpp:127
> +    if (SchemeRegistry::shouldTreatURLSchemeAsSecure(protocol))
>          return true;

I think there is still a thread safety issue from the string hasher
but I think this patch still fixes the majority of thread safety issues over the status quo.
Please add a FIXME here.

> Source/WebCore/page/SecurityOrigin.cpp:224
> +    // This code is using an enum instead of an std::optional for thread-safety. Worse case scenario, several thread will read

Nit: "Worse case" -> "The worst case".

[Chris Dumez]

Created attachment 334559 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 334559 [details]
Patch

Clearing flags on attachment: 334559

Committed r228972: <https://trac.webkit.org/changeset/228972>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

This crash still occurs on WebKitGTK+, so I suspect the fix was not complete. See bug #183197.