| Summary: | ThreadSanitizer: data race in WTF::StringImpl::deref() under WebKit::WebProcessPool::setMediaAccessibilityPreferences() | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
| Component: | WebKit2 | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | cdumez, cfleizach, kkinnunen, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 227617 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
Created attachment 435225 [details]
Patch v1
Comment on attachment 435225 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=435225&action=review > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:192 > - callOnMainRunLoop([weakProcess, captionDisplayMode, preferredLanguages] { > + callOnMainRunLoop([weakProcess, captionDisplayMode, preferredLanguages = preferredLanguages.isolatedCopy()] { I opted to make an isolatedCopy() here because the other use of WebCore::CaptionUserPreferencesMediaAF::platformPreferredLanguages() in Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm didn't involve separate threads: #if HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK) void WebProcessPool::mediaAccessibilityPreferencesChangedCallback(CFNotificationCenterRef, void *observer, CFStringRef name, const void *, CFDictionaryRef userInfo) { auto* pool = reinterpret_cast<WebProcessPool*>(observer); auto captionDisplayMode = WebCore::CaptionUserPreferencesMediaAF::platformCaptionDisplayMode(); auto preferredLanguages = WebCore::CaptionUserPreferencesMediaAF::platformPreferredLanguages(); pool->sendToAllProcesses(Messages::WebProcess::SetMediaAccessibilityPreferences(captionDisplayMode, preferredLanguages)); } #endif Comment on attachment 435225 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=435225&action=review r=me >> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:192 >> + callOnMainRunLoop([weakProcess, captionDisplayMode, preferredLanguages = preferredLanguages.isolatedCopy()] { > > I opted to make an isolatedCopy() here because the other use of WebCore::CaptionUserPreferencesMediaAF::platformPreferredLanguages() in Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm didn't involve separate threads: > > #if HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK) > void WebProcessPool::mediaAccessibilityPreferencesChangedCallback(CFNotificationCenterRef, void *observer, CFStringRef name, const void *, CFDictionaryRef userInfo) > { > auto* pool = reinterpret_cast<WebProcessPool*>(observer); > auto captionDisplayMode = WebCore::CaptionUserPreferencesMediaAF::platformCaptionDisplayMode(); > auto preferredLanguages = WebCore::CaptionUserPreferencesMediaAF::platformPreferredLanguages(); > pool->sendToAllProcesses(Messages::WebProcess::SetMediaAccessibilityPreferences(captionDisplayMode, preferredLanguages)); > } > #endif nit: WTFMove(preferredLanguages).isolatedCopy() may be more efficient in some cases because of this version of isolatedCopy(): template<typename U = T> Vector<U> isolatedCopy() &&; Created attachment 435227 [details]
Patch v2 for landing
Regressed in r279591: Collect Accessibility preferences on a background queue https://bugs.webkit.org/show_bug.cgi?id=227617 https://trac.webkit.org/changeset/279591/webkit Committed r280841 (240392@main): <https://commits.webkit.org/240392@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435227 [details]. |
ThreadSanitizer: data race in WTF::StringImpl::deref() under WebKit::WebProcessPool::setMediaAccessibilityPreferences(). In Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm, the following code sends a Vector<WTF::String> object from a background thread to the main thread without an isolated copy of the WTF::String objects: #if HAVE(MEDIA_ACCESSIBILITY_FRAMEWORK) void WebProcessPool::setMediaAccessibilityPreferences(WebProcessProxy& process) { dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), [weakProcess = makeWeakPtr(process)] { auto captionDisplayMode = WebCore::CaptionUserPreferencesMediaAF::platformCaptionDisplayMode(); auto preferredLanguages = WebCore::CaptionUserPreferencesMediaAF::platformPreferredLanguages(); callOnMainRunLoop([weakProcess, captionDisplayMode, preferredLanguages] { if (weakProcess) weakProcess->send(Messages::WebProcess::SetMediaAccessibilityPreferences(captionDisplayMode, preferredLanguages), 0); }); }); } #endif -- Thread sanitizer warning while running LayoutTests/crypto/crypto-random-values-types.html with WebKit r280760: WARNING: ThreadSanitizer: data race (pid=46345) Read of size 4 at 0x7b080000ea00 by main thread: #0 WTF::StringImpl::deref() <null> (WebKit:x86_64+0x7aca) #1 WTF::VectorDestructor<true, WTF::String>::destruct(WTF::String*, WTF::String*) <null> (WebKit:x86_64+0x9867) #2 WTF::VectorTypeOperations<WTF::String>::destruct(WTF::String*, WTF::String*) <null> (WebKit:x86_64+0x97b0) #3 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::~Vector() <null> (WebKit:x86_64+0x974f) #4 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::~Vector() <null> (WebKit:x86_64+0x6eb9) #5 WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const::'lambda'()::~() <null> (WebKit:x86_64+0x10cc72a) #6 WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const::'lambda'()::~() <null> (WebKit:x86_64+0x10cba49) #7 WTF::Detail::CallableWrapper<WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const::'lambda'(), void>::~CallableWrapper() <null> (WebKit:x86_64+0x10cbe00) #8 WTF::Detail::CallableWrapper<WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const::'lambda'(), void>::~CallableWrapper() <null> (WebKit:x86_64+0x10cbcc9) #9 WTF::Detail::CallableWrapper<WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const::'lambda'(), void>::~CallableWrapper() <null> (WebKit:x86_64+0x10cbcf9) #10 std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> >::operator()(WTF::Detail::CallableWrapperBase<void>*) const <null> (JavaScriptCore:x86_64+0x149c7) #11 std::__1::unique_ptr<WTF::Detail::CallableWrapperBase<void>, std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> > >::reset(WTF::Detail::CallableWrapperBase<void>*) <null> (JavaScriptCore:x86_64+0x1492d) #12 std::__1::unique_ptr<WTF::Detail::CallableWrapperBase<void>, std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> > >::~unique_ptr() <null> (JavaScriptCore:x86_64+0x148bb) #13 std::__1::unique_ptr<WTF::Detail::CallableWrapperBase<void>, std::__1::default_delete<WTF::Detail::CallableWrapperBase<void> > >::~unique_ptr() <null> (JavaScriptCore:x86_64+0x14889) #14 WTF::Function<void ()>::~Function() <null> (JavaScriptCore:x86_64+0x14859) #15 WTF::Function<void ()>::~Function() <null> (JavaScriptCore:x86_64+0x134a9) #16 WTF::RunLoop::performWork() <null> (JavaScriptCore:x86_64+0x8d9aa) #17 WTF::RunLoop::performWork(void*) <null> (JavaScriptCore:x86_64+0x9072a) #18 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ <null> (CoreFoundation:x86_64+0x81354) #19 WTR::TestController::runUntil(bool&, WTF::Seconds) <null> (WebKitTestRunner:x86_64+0x100054a1b) #20 WTR::TestController::resetStateToConsistentValues(WTR::TestOptions const&, WTR::TestController::ResetStage)::$_4::operator()() const <null> (WebKitTestRunner:x86_64+0x10005a0b8) #21 WTR::TestController::resetStateToConsistentValues(WTR::TestOptions const&, WTR::TestController::ResetStage) <null> (WebKitTestRunner:x86_64+0x100059032) #22 WTR::TestController::ensureViewSupportsOptionsForTest(WTR::TestInvocation const&) <null> (WebKitTestRunner:x86_64+0x1000587ae) #23 WTR::TestController::configureViewForTest(WTR::TestInvocation const&) <null> (WebKitTestRunner:x86_64+0x10005b480) #24 WTR::TestInvocation::invoke() <null> (WebKitTestRunner:x86_64+0x10009784c) #25 WTR::TestController::runTest(char const*) <null> (WebKitTestRunner:x86_64+0x10005b66e) #26 WTR::TestController::runTestingServerLoop() <null> (WebKitTestRunner:x86_64+0x10005ba17) #27 WTR::TestController::run() <null> (WebKitTestRunner:x86_64+0x10005523d) #28 WTR::TestController::TestController(int, char const**) <null> (WebKitTestRunner:x86_64+0x100054d4b) #29 WTR::TestController::TestController(int, char const**) <null> (WebKitTestRunner:x86_64+0x1000552e9) #30 main <null> (WebKitTestRunner:x86_64+0x10000804a) Previous write of size 4 at 0x7b080000ea00 by thread T3: #0 WTF::StringImpl::deref() <null> (WebKit:x86_64+0x7ada) #1 WTF::VectorDestructor<true, WTF::String>::destruct(WTF::String*, WTF::String*) <null> (WebKit:x86_64+0x9867) #2 WTF::VectorTypeOperations<WTF::String>::destruct(WTF::String*, WTF::String*) <null> (WebKit:x86_64+0x97b0) #3 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::~Vector() <null> (WebKit:x86_64+0x974f) #4 WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::~Vector() <null> (WebKit:x86_64+0x6eb9) #5 WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const <null> (WebKit:x86_64+0x10988c3) #6 invocation function for block in WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&) <null> (WebKit:x86_64+0x109882d) #7 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x7377b) #8 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x34ff) Location is heap block of size 24 at 0x7b080000ea00 allocated by thread T3: #0 __sanitizer_mz_malloc <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x5168a) #1 _malloc_zone_malloc <null> (libsystem_malloc.dylib:x86_64+0x1cf80) #2 bmalloc::Cache::allocateSlowCaseNullCache(bmalloc::HeapKind, unsigned long) <null> (JavaScriptCore:x86_64+0x11d240) #3 bmalloc::Cache::allocate(bmalloc::HeapKind, unsigned long) <null> (JavaScriptCore:x86_64+0x37629) #4 WTF::fastMalloc(unsigned long) <null> (JavaScriptCore:x86_64+0x36e5b) #5 WTF::StringImpl::operator new(unsigned long) <null> (JavaScriptCore:x86_64+0x34b69) #6 WTF::StringImpl::adopt(WTF::StringBuffer<unsigned char>&&) <null> (JavaScriptCore:x86_64+0xa6374) #7 WTF::String::String(__CFString const*) <null> (JavaScriptCore:x86_64+0x9ca9e) #8 WTF::String::String(__CFString const*) <null> (JavaScriptCore:x86_64+0x9cd20) #9 WebCore::CaptionUserPreferencesMediaAF::platformPreferredLanguages() <null> (WebCore:x86_64+0x2de4689) #10 WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&)::$_3::operator()() const <null> (WebKit:x86_64+0x1098876) #11 invocation function for block in WebKit::WebProcessPool::setMediaAccessibilityPreferences(WebKit::WebProcessProxy&) <null> (WebKit:x86_64+0x109882d) #12 __tsan::invoke_and_release_block(void*) <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x7377b) #13 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x34ff) Thread T3 (tid=12850431, running) is a GCD worker thread SUMMARY: ThreadSanitizer: data race (WebKitBuild/WebKit.framework/Versions/A/WebKit:x86_64+0x7aca) in WTF::StringImpl::deref()+0x1a