Summary: | Crash under SchemeRegistry::shouldTreatURLSchemeAsLocal(WTF::String const&) | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||||||||||||
Component: | WebCore Misc. | Assignee: | Chris Dumez <cdumez> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | achristensen, aestes, ap, beidson, commit-queue, dbates, ews-watchlist, koivisto, mcatanzaro, rniwa, webkit-bug-importer, youennf | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=183197 | ||||||||||||||||
Bug Depends on: | 184024 | ||||||||||||||||
Bug Blocks: | |||||||||||||||||
Attachments: |
|
Description
Chris Dumez
2018-02-22 15:09:41 PST
Created attachment 334485 [details]
Patch
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 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
Created attachment 334494 [details]
Patch
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. 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. Created attachment 334551 [details]
Patch
Created attachment 334552 [details]
Patch
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". Created attachment 334559 [details]
Patch
Comment on attachment 334559 [details] Patch Clearing flags on attachment: 334559 Committed r228972: <https://trac.webkit.org/changeset/228972> All reviewed patches have been landed. Closing bug. This crash still occurs on WebKitGTK+, so I suspect the fix was not complete. See bug #183197. |