Bug 229014

Summary:

ThreadSanitizer: data race in WebCore::CARingBufferStorageVector::setCurrentFrameBounds() / getCurrentFrameBounds()

Product:

WebKit

Reporter:

David Kilzer (:ddkilzer) <ddkilzer>

Component:

Media

Assignee:

David Kilzer (:ddkilzer) <ddkilzer>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, eric.carlson, ews-watchlist, ggaren, glenn, gsnedders, jer.noble, philipj, sergio, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch v1	ews-feeder: commit-queue-
Patch v2	jer.noble: review-
Patch v3	none
Patch v4	none

David Kilzer (:ddkilzer)

Reported 2021-08-11 15:47:40 PDT

ThreadSanitizer: data race in WebCore::CARingBufferStorageVector::setCurrentFrameBounds() / getCurrentFrameBounds(). The issue is that setCurrentFrameBounds() uses a Locker, but getCurrentFrameBounds(). Both currentStartFrame() and currentEndFrame() are also missing a Locker. I did not catch explicit races there, but they exist based on source code inspection. WARNING: ThreadSanitizer: data race (pid=55586) Write of size 8 at 0x7b58000622f8 by thread T17: #0 WebCore::CARingBufferStorageVector::setCurrentFrameBounds(unsigned long long, unsigned long long) <null> (WebCore:x86_64+0x2f166ca) #1 WebCore::CARingBuffer::setCurrentFrameBounds(unsigned long long, unsigned long long) <null> (WebCore:x86_64+0x2f1641d) #2 WebCore::CARingBuffer::store(AudioBufferList const*, unsigned long, unsigned long long) <null> (WebCore:x86_64+0x2f16386) #3 WebCore::AudioSampleDataSource::pushSamplesInternal(AudioBufferList const&, WTF::MediaTime const&, unsigned long) <null> (WebCore:x86_64+0x1151119) #4 WebCore::AudioSampleDataSource::pushSamples(WTF::MediaTime const&, WebCore::PlatformAudioData const&, unsigned long) <null> (WebCore:x86_64+0x11513d0) #5 WebCore::WebAudioSourceProviderCocoa::receivedNewAudioSamples(WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x16f17b1) #6 WebCore::MediaStreamTrackAudioSourceProviderCocoa::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x32b7d85) #7 non-virtual thunk to WebCore::MediaStreamTrackAudioSourceProviderCocoa::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x32b7dd7) #8 WebCore::RealtimeMediaSource::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x32581aa) #9 WebKit::RemoteRealtimeAudioSource::remoteAudioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebKit:x86_64+0x1c724a9) #10 WebKit::RemoteCaptureSampleManager::RemoteAudio::startThread()::$_3::operator()() <null> (WebKit:x86_64+0x1c8e38c) #11 WTF::Detail::CallableWrapper<WebKit::RemoteCaptureSampleManager::RemoteAudio::startThread()::$_3, void>::call() <null> (WebKit:x86_64+0x1c8e1ad) #12 WTF::Function<void ()>::operator()() const <null> (JavaScriptCore:x86_64+0x2620d) #13 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) <null> (JavaScriptCore:x86_64+0xbccf0) #14 WTF::wtfThreadEntryPoint(void*) <null> (JavaScriptCore:x86_64+0xc5719) Previous read of size 8 at 0x7b58000622f8 by thread T16: #0 WebCore::CARingBufferStorageVector::getCurrentFrameBounds(unsigned long long&, unsigned long long&) <null> (WebCore:x86_64+0x2f168e7) #1 WebCore::CARingBuffer::getCurrentFrameBoundsWithoutUpdate(unsigned long long&, unsigned long long&) <null> (WebCore:x86_64+0x2f1688d) #2 WebCore::CARingBuffer::getCurrentFrameBounds(unsigned long long&, unsigned long long&) <null> (WebCore:x86_64+0x2f167f1) #3 WebCore::AudioSampleDataSource::pullSamplesInternal(AudioBufferList&, unsigned long, unsigned long long, double, WebCore::AudioSampleDataSource::PullMode) <null> (WebCore:x86_64+0x1151549) #4 WebCore::AudioSampleDataSource::pullSamples(AudioBufferList&, unsigned long, unsigned long long, double, WebCore::AudioSampleDataSource::PullMode) <null> (WebCore:x86_64+0x1151ef9) #5 WebCore::WebAudioSourceProviderCocoa::provideInput(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x16f0ff5) #6 WebCore::MediaStreamAudioSourceNode::provideInput(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x1949f03) #7 WebCore::MediaStreamAudioSourceNode::process(unsigned long) <null> (WebCore:x86_64+0x194a08d) #8 WebCore::AudioNode::processIfNecessary(unsigned long) <null> (WebCore:x86_64+0x18aa734) #9 WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9c92) #10 WebCore::AudioNodeInput::pull(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9d35) #11 WebCore::AudioBasicInspectorNode::pullInputs(unsigned long) <null> (WebCore:x86_64+0x189e407) #12 WebCore::AudioNode::processIfNecessary(unsigned long) <null> (WebCore:x86_64+0x18aa660) #13 WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9c92) #14 WebCore::AudioNodeInput::pull(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9d35) #15 WebCore::AudioNode::pullInputs(unsigned long) <null> (WebCore:x86_64+0x18aa984) #16 WebCore::AudioNode::processIfNecessary(unsigned long) <null> (WebCore:x86_64+0x18aa660) #17 WebCore::AudioNodeOutput::pull(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9c92) #18 WebCore::AudioNodeInput::sumAllConnections(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9b64) #19 WebCore::AudioNodeInput::pull(WebCore::AudioBus*, unsigned long) <null> (WebCore:x86_64+0x18c9d11) #20 WebCore::AudioDestinationNode::renderQuantum(WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) <null> (WebCore:x86_64+0x18a6dee) #21 WebCore::DefaultAudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) <null> (WebCore:x86_64+0x19206f0) #22 non-virtual thunk to WebCore::DefaultAudioDestinationNode::render(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) <null> (WebCore:x86_64+0x19207c7) #23 WebCore::AudioDestination::callRenderCallback(WebCore::AudioBus*, WebCore::AudioBus*, unsigned long, WebCore::AudioIOPosition const&) <null> (WebCore:x86_64+0x2f04614) #24 WebCore::AudioDestinationCocoa::renderOnRenderingThead(unsigned long) <null> (WebCore:x86_64+0x2f0450e) #25 WebCore::AudioDestinationCocoa::renderOnRenderingTheadIfPlaying(unsigned long) <null> (WebCore:x86_64+0x2f0438d) #26 WebCore::AudioDestinationCocoa::render(double, unsigned long long, unsigned int, AudioBufferList*) <null> (WebCore:x86_64+0x2f041e2) #27 WebKit::RemoteAudioDestinationProxy::renderQuantum() <null> (WebKit:x86_64+0x1979fc1) #28 WebKit::RemoteAudioDestinationProxy::startRenderingThread()::$_10::operator()() <null> (WebKit:x86_64+0x19a14cb) #29 WTF::Detail::CallableWrapper<WebKit::RemoteAudioDestinationProxy::startRenderingThread()::$_10, void>::call() <null> (WebKit:x86_64+0x19a143d) #30 WTF::Function<void ()>::operator()() const <null> (JavaScriptCore:x86_64+0x2620d) #31 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) <null> (JavaScriptCore:x86_64+0xbccf0) #32 WTF::wtfThreadEntryPoint(void*) <null> (JavaScriptCore:x86_64+0xc5719) Location is heap block of size 768 at 0x7b5800062100 allocated by thread T17: #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::FastMalloc::malloc(unsigned long) <null> (WebCore:x86_64+0x1a059) #6 bool WTF::VectorBufferBase<WebCore::CARingBufferStorageVector::TimeBounds, WTF::FastMalloc>::allocateBuffer<(WTF::FailureAction)0>(unsigned long) <null> (WebCore:x86_64+0x2f1b4d1) #7 WTF::VectorBuffer<WebCore::CARingBufferStorageVector::TimeBounds, 0ul, WTF::FastMalloc>::VectorBuffer(unsigned long, unsigned long) <null> (WebCore:x86_64+0x2f1b3f0) #8 WTF::Vector<WebCore::CARingBufferStorageVector::TimeBounds, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::Vector(unsigned long) <null> (WebCore:x86_64+0x2f1b363) #9 WTF::Vector<WebCore::CARingBufferStorageVector::TimeBounds, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>::Vector(unsigned long) <null> (WebCore:x86_64+0x2f154b0) #10 WebCore::CARingBufferStorageVector::CARingBufferStorageVector() <null> (WebCore:x86_64+0x2f15426) #11 WebCore::CARingBufferStorageVector::CARingBufferStorageVector() <null> (WebCore:x86_64+0x2f154d9) #12 WTF::UniqueRef<WebCore::CARingBufferStorageVector> WTF::makeUniqueRefWithoutFastMallocCheck<WebCore::CARingBufferStorageVector>() <null> (WebCore:x86_64+0x2f1b617) #13 WTF::UniqueRef<WebCore::CARingBufferStorageVector> WTF::makeUniqueRef<WebCore::CARingBufferStorageVector>() <null> (WebCore:x86_64+0x2f155a9) #14 WebCore::CARingBuffer::CARingBuffer() <null> (WebCore:x86_64+0x2f15512) #15 WebCore::CARingBuffer::CARingBuffer() <null> (WebCore:x86_64+0x2f15669) #16 WTF::UniqueRef<WebCore::CARingBuffer> WTF::makeUniqueRefWithoutFastMallocCheck<WebCore::CARingBuffer>() <null> (WebCore:x86_64+0x353f7) #17 WTF::UniqueRef<WebCore::CARingBuffer> WTF::makeUniqueRef<WebCore::CARingBuffer>() <null> (WebCore:x86_64+0x353a9) #18 WebCore::AudioSampleDataSource::AudioSampleDataSource(unsigned long, WTF::LoggerHelper&, unsigned long) <null> (WebCore:x86_64+0x11503aa) #19 WebCore::AudioSampleDataSource::AudioSampleDataSource(unsigned long, WTF::LoggerHelper&, unsigned long) <null> (WebCore:x86_64+0x1150280) #20 WebCore::AudioSampleDataSource::create(unsigned long, WTF::LoggerHelper&, unsigned long) <null> (WebCore:x86_64+0x11501c0) #21 WebCore::WebAudioSourceProviderCocoa::prepare(AudioStreamBasicDescription const&) <null> (WebCore:x86_64+0x16f12ac) #22 WebCore::WebAudioSourceProviderCocoa::receivedNewAudioSamples(WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x16f173b) #23 WebCore::MediaStreamTrackAudioSourceProviderCocoa::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x32b7d85) #24 non-virtual thunk to WebCore::MediaStreamTrackAudioSourceProviderCocoa::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x32b7dd7) #25 WebCore::RealtimeMediaSource::audioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebCore:x86_64+0x32581aa) #26 WebKit::RemoteRealtimeAudioSource::remoteAudioSamplesAvailable(WTF::MediaTime const&, WebCore::PlatformAudioData const&, WebCore::AudioStreamDescription const&, unsigned long) <null> (WebKit:x86_64+0x1c724a9) #27 WebKit::RemoteCaptureSampleManager::RemoteAudio::startThread()::$_3::operator()() <null> (WebKit:x86_64+0x1c8e38c) #28 WTF::Detail::CallableWrapper<WebKit::RemoteCaptureSampleManager::RemoteAudio::startThread()::$_3, void>::call() <null> (WebKit:x86_64+0x1c8e1ad) #29 WTF::Function<void ()>::operator()() const <null> (JavaScriptCore:x86_64+0x2620d) #30 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*) <null> (JavaScriptCore:x86_64+0xbccf0) #31 WTF::wtfThreadEntryPoint(void*) <null> (JavaScriptCore:x86_64+0xc5719) Thread T17 (tid=13416394, running) created by thread T5 at: #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x2bffd) #1 WTF::Thread::establishHandle(WTF::Thread::NewThreadContext*, std::__1::optional<unsigned long>, WTF::Thread::QOS) <null> (JavaScriptCore:x86_64+0xc565e) #2 WTF::Thread::create(char const*, WTF::Function<void ()>&&, WTF::ThreadType, WTF::Thread::QOS) <null> (JavaScriptCore:x86_64+0xbcea7) #3 WebKit::RemoteCaptureSampleManager::RemoteAudio::startThread() <null> (WebKit:x86_64+0x1c718e8) #4 WebKit::RemoteCaptureSampleManager::RemoteAudio::setStorage(WebKit::SharedMemory::Handle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long) <null> (WebKit:x86_64+0x1c7125d) #5 WebKit::RemoteCaptureSampleManager::audioStorageChanged(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long) <null> (WebKit:x86_64+0x1c70f15) #6 void IPC::callMemberFunctionImpl<WebKit::RemoteCaptureSampleManager, void (WebKit::RemoteCaptureSampleManager::*)(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long), std::__1::tuple<WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle, WebCore::CAAudioStreamDescription, unsigned long long, IPC::Semaphore, WTF::MediaTime, unsigned long>, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul>(WebKit::RemoteCaptureSampleManager*, void (WebKit::RemoteCaptureSampleManager::*)(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long), std::__1::tuple<WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle, WebCore::CAAudioStreamDescription, unsigned long long, IPC::Semaphore, WTF::MediaTime, unsigned long>&&, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul>) <null> (WebKit:x86_64+0x494848) #7 void IPC::callMemberFunction<WebKit::RemoteCaptureSampleManager, void (WebKit::RemoteCaptureSampleManager::*)(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long), std::__1::tuple<WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle, WebCore::CAAudioStreamDescription, unsigned long long, IPC::Semaphore, WTF::MediaTime, unsigned long>, std::__1::integer_sequence<unsigned long, 0ul, 1ul, 2ul, 3ul, 4ul, 5ul, 6ul> >(std::__1::tuple<WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle, WebCore::CAAudioStreamDescription, unsigned long long, IPC::Semaphore, WTF::MediaTime, unsigned long>&&, WebKit::RemoteCaptureSampleManager*, void (WebKit::RemoteCaptureSampleManager::*)(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long)) <null> (WebKit:x86_64+0x48ab48) #8 void IPC::handleMessage<Messages::RemoteCaptureSampleManager::AudioStorageChanged, WebKit::RemoteCaptureSampleManager, void (WebKit::RemoteCaptureSampleManager::*)(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long)>(IPC::Decoder&, WebKit::RemoteCaptureSampleManager*, void (WebKit::RemoteCaptureSampleManager::*)(WTF::ObjectIdentifier<WebCore::RealtimeMediaSourceIdentifierType>, WebKit::SharedMemory::IPCHandle const&, WebCore::CAAudioStreamDescription const&, unsigned long long, IPC::Semaphore&&, WTF::MediaTime const&, unsigned long)) <null> (WebKit:x86_64+0x48a97b) #9 WebKit::RemoteCaptureSampleManager::didReceiveMessage(IPC::Connection&, IPC::Decoder&) <null> (WebKit:x86_64+0x48a8b0) #10 IPC::Connection::dispatchMessageReceiverMessage(IPC::MessageReceiver&, std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >&&) <null> (WebKit:x86_64+0x8d271) #11 IPC::ThreadMessageReceiverQueue::enqueueMessage(IPC::Connection&, std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >&&)::'lambda'()::operator()() <null> (WebKit:x86_64+0x9632e) #12 WTF::Detail::CallableWrapper<IPC::ThreadMessageReceiverQueue::enqueueMessage(IPC::Connection&, std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >&&)::'lambda'(), void>::call() <null> (WebKit:x86_64+0x9610d) #13 WTF::Function<void ()>::operator()() const <null> (JavaScriptCore:x86_64+0x2620d) #14 WTF::(anonymous namespace)::DispatchWorkItem::operator()() <null> (JavaScriptCore:x86_64+0x11285d) #15 void WTF::dispatchWorkItem<WTF::(anonymous namespace)::DispatchWorkItem>(void*) <null> (JavaScriptCore:x86_64+0x111849) #16 __tsan::dispatch_callback_wrap(void*) <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x734d1) #17 _dispatch_client_callout <null> (libdispatch.dylib:x86_64+0x34ff) Thread T16 (tid=13416360, running) created by main thread at: #0 pthread_create <null> (libclang_rt.tsan_osx_dynamic.dylib:x86_64+0x2bffd) #1 WTF::Thread::establishHandle(WTF::Thread::NewThreadContext*, std::__1::optional<unsigned long>, WTF::Thread::QOS) <null> (JavaScriptCore:x86_64+0xc565e) #2 WTF::Thread::create(char const*, WTF::Function<void ()>&&, WTF::ThreadType, WTF::Thread::QOS) <null> (JavaScriptCore:x86_64+0xbcea7) #3 WebKit::RemoteAudioDestinationProxy::startRenderingThread() <null> (WebKit:x86_64+0x1978f4a) #4 WebKit::RemoteAudioDestinationProxy::connection() <null> (WebKit:x86_64+0x1979246) #5 WebKit::RemoteAudioDestinationProxy::startRendering(WTF::CompletionHandler<void (bool)>&&) <null> (WebKit:x86_64+0x1979a65) #6 WebCore::AudioDestinationCocoa::start(WTF::Function<void (WTF::Function<void ()>&&)>&&, WTF::CompletionHandler<void (bool)>&&) <null> (WebCore:x86_64+0x2f03b7b) #7 WebCore::DefaultAudioDestinationNode::startRendering(WTF::CompletionHandler<void (std::__1::optional<WebCore::Exception>&&)>&&) <null> (WebCore:x86_64+0x191fdc7) #8 WebCore::AudioContext::startRendering() <null> (WebCore:x86_64+0x18a59dd) #9 WebCore::AudioContext::lazyInitialize() <null> (WebCore:x86_64+0x18a4e35) #10 WebCore::AudioContext::AudioContext(WebCore::Document&, WebCore::AudioContextOptions const&)::$_1::operator()() const <null> (WebCore:x86_64+0x18c0a30) #11 WTF::Detail::CallableWrapper<WebCore::AudioContext::AudioContext(WebCore::Document&, WebCore::AudioContextOptions const&)::$_1, void>::call() <null> (WebCore:x86_64+0x18c087d) #12 WTF::Function<void ()>::operator()() const <null> (WebCore:x86_64+0x1c7dd) #13 void WebCore::ActiveDOMObject::queueTaskKeepingObjectAlive<WebCore::BaseAudioContext>(WebCore::BaseAudioContext&, WebCore::TaskSource, WTF::Function<void ()>&&)::'lambda'()::operator()() const <null> (WebCore:x86_64+0x19126ed) #14 WTF::Detail::CallableWrapper<void WebCore::ActiveDOMObject::queueTaskKeepingObjectAlive<WebCore::BaseAudioContext>(WebCore::BaseAudioContext&, WebCore::TaskSource, WTF::Function<void ()>&&)::'lambda'(), void>::call() <null> (WebCore:x86_64+0x19124fd) #15 WTF::Function<void ()>::operator()() const <null> (WebCore:x86_64+0x1c7dd) #16 WebCore::EventLoopFunctionDispatchTask::execute() <null> (WebCore:x86_64+0x21e4e9d) #17 WebCore::EventLoop::run() <null> (WebCore:x86_64+0x21dac58) #18 WebCore::WindowEventLoop::didReachTimeToRun() <null> (WebCore:x86_64+0x22f302d) #19 decltype(*(std::__1::forward<WebCore::WindowEventLoop*&>(fp0)).*fp()) std::__1::__invoke<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&, void>(void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*&) <null> (WebCore:x86_64+0x22f4f0d) #20 std::__1::__bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<> >::value>::type std::__1::__apply_functor<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, 0ul, std::__1::tuple<> >(void (WebCore::WindowEventLoop::*&)(), std::__1::tuple<WebCore::WindowEventLoop*>&, std::__1::__tuple_indices<0ul>, std::__1::tuple<>&&) <null> (WebCore:x86_64+0x22f4e89) #21 std::__1::__bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<>, __is_valid_bind_return<void (WebCore::WindowEventLoop::*)(), std::__1::tuple<WebCore::WindowEventLoop*>, std::__1::tuple<> >::value>::type std::__1::__bind<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*>::operator()<>() <null> (WebCore:x86_64+0x22f4e41) #22 WTF::Detail::CallableWrapper<std::__1::__bind<void (WebCore::WindowEventLoop::*&)(), WebCore::WindowEventLoop*>, void>::call() <null> (WebCore:x86_64+0x22f4d7d) #23 WTF::Function<void ()>::operator()() const <null> (WebCore:x86_64+0x1c7dd) #24 WebCore::Timer::fired() <null> (WebCore:x86_64+0x5d35d) #25 WebCore::ThreadTimers::sharedTimerFiredInternal() <null> (WebCore:x86_64+0x2eb8247) #26 WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0::operator()() const <null> (WebCore:x86_64+0x2ebfb11) #27 WTF::Detail::CallableWrapper<WebCore::ThreadTimers::setSharedTimer(WebCore::SharedTimer*)::$_0, void>::call() <null> (WebCore:x86_64+0x2ebfab1) #28 WTF::Function<void ()>::operator()() const <null> (WebCore:x86_64+0x1c7dd) #29 WebCore::MainThreadSharedTimer::fired() <null> (WebCore:x86_64+0x2e7ad6d) #30 WebCore::timerFired(__CFRunLoopTimer*, void*) <null> (WebCore:x86_64+0x2f22677) #31 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ <null> (CoreFoundation:x86_64+0x99f28) #32 WKXPCServiceMain <null> (WebKit:x86_64+0x22543fe) #33 main <null> (com.apple.WebKit.WebContent.Development:x86_64+0x100003e3e) SUMMARY: ThreadSanitizer: data race (WebKitBuild/WebCore.framework/Versions/A/WebCore:x86_64+0x2f166ca) in WebCore::CARingBufferStorageVector::setCurrentFrameBounds(unsigned long long, unsigned long long)+0x6a

Attachments
Patch v1 (3.38 KB, patch) 2021-08-11 15:53 PDT, David Kilzer (:ddkilzer)	ews-feeder: commit-queue-	Details Formatted Diff Diff
Patch v2 (4.08 KB, patch) 2021-08-11 16:47 PDT, David Kilzer (:ddkilzer)	jer.noble: review-	Details Formatted Diff Diff
Patch v3 (4.85 KB, patch) 2021-08-11 18:01 PDT, David Kilzer (:ddkilzer)	no flags	Details Formatted Diff Diff
Patch v4 (4.26 KB, patch) 2021-08-12 11:03 PDT, David Kilzer (:ddkilzer)	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2021-08-11 15:48:00 PDT

<rdar://problem/81817224>

David Kilzer (:ddkilzer)

Comment 2 2021-08-11 15:48:24 PDT

Seen in these layout tests: fast/mediastream/getUserMedia-webaudio.html fast/mediastream/mediastreamtrack-audio-clone.html imported/w3c/web-platform-tests/webrtc/RTCDTMFSender-insertDTMF.https.html imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-iceConnectionState.https.html imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-track-stats.https.html imported/w3c/web-platform-tests/webrtc/protocol/missing-fields.html webrtc/audio-peer-connection-g722.html webrtc/audio-peer-connection-webaudio.html webrtc/audio-replace-track.html webrtc/peer-connection-audio-mute.html webrtc/peer-connection-audio-mute2.html webrtc/peer-connection-createMediaStreamDestination.html webrtc/peer-connection-remote-audio-mute.html webrtc/peer-connection-remote-audio-mute2.html

Chris Dumez

Comment 3 2021-08-11 15:51:35 PDT

cc'ing Jer for visibility here. I remember there are some constraints here as we don't want to be locking on the audio thread. My understanding was that CARingBuffer had some code that was intentionally racy to avoid locking.

David Kilzer (:ddkilzer)

Comment 4 2021-08-11 15:53:31 PDT

Created attachment 435381 [details] Patch v1

David Kilzer (:ddkilzer)

Comment 5 2021-08-11 15:56:40 PDT

(In reply to Chris Dumez from comment #3) > cc'ing Jer for visibility here. I remember there are some constraints here > as we don't want to be locking on the audio thread. My understanding was > that CARingBuffer had some code that was intentionally racy to avoid locking. Good to know. I think getCurrentFrameBounds() definitely needs locking (otherwise you get inconsistent values for start/end), but I could see an argument for not locking currentStartFrame() and currentEndFrame(). We can also tell TSan to ignore individual methods via the `SUPPRESS_TSAN` attribute.

Chris Dumez

Comment 6 2021-08-11 15:58:35 PDT

(In reply to David Kilzer (:ddkilzer) from comment #5) > (In reply to Chris Dumez from comment #3) > > cc'ing Jer for visibility here. I remember there are some constraints here > > as we don't want to be locking on the audio thread. My understanding was > > that CARingBuffer had some code that was intentionally racy to avoid locking. > > Good to know. I think getCurrentFrameBounds() definitely needs locking > (otherwise you get inconsistent values for start/end), but I could see an > argument for not locking currentStartFrame() and currentEndFrame(). > > We can also tell TSan to ignore individual methods via the `SUPPRESS_TSAN` > attribute. I defer to Eric and Jer here. The locking may be fine in this particular instance.

David Kilzer (:ddkilzer)

Comment 7 2021-08-11 16:00:44 PDT

(In reply to David Kilzer (:ddkilzer) from comment #4) > Created attachment 435381 [details] > Patch v1 Hmmm...can't use WTF::Locker in a const method?

Chris Dumez

Comment 8 2021-08-11 16:02:01 PDT

(In reply to David Kilzer (:ddkilzer) from comment #7) > (In reply to David Kilzer (:ddkilzer) from comment #4) > > Created attachment 435381 [details] > > Patch v1 > > Hmmm...can't use WTF::Locker in a const method? Mark Lock as mutable?

David Kilzer (:ddkilzer)

Comment 9 2021-08-11 16:47:30 PDT

Created attachment 435385 [details] Patch v2

Jer Noble

Comment 10 2021-08-11 16:50:00 PDT

Comment on attachment 435385 [details] Patch v2 The entire point of the CARingBuffer is for it to be lockless; adding a lock to address this race would be very bad for high-priority audio thread performance. We should find another way to address this race.

Jer Noble

Comment 11 2021-08-11 16:52:08 PDT

I'm not sure this is worth fixing; CARingBuffer is inherently racy, and that's fine. Can we just disable this warning somehow?

David Kilzer (:ddkilzer)

Comment 12 2021-08-11 16:53:58 PDT

(In reply to Jer Noble from comment #11) > I'm not sure this is worth fixing; CARingBuffer is inherently racy, and > that's fine. Can we just disable this warning somehow? Yes, with TSAN_SUPPRESS.

Chris Dumez

Comment 13 2021-08-11 16:57:04 PDT

``` volatile uint64_t m_startFrame; volatile uint64_t m_endFrame; volatile uint32_t m_updateCounter; ``` Should we mark these as atomic instead of volatile? I believe they are marked as volatile exactly to deal with this threading race. However, iirc, using volatile for such things is not entirely correct. cc'ing Geoff / Yusuke who probably know for sure.

David Kilzer (:ddkilzer)

Comment 14 2021-08-11 18:01:22 PDT

Created attachment 435389 [details] Patch v3

Sam Sneddon [:gsnedders]

Comment 15 2021-08-11 19:58:40 PDT

(In reply to Chris Dumez from comment #13) > ``` > volatile uint64_t m_startFrame; > volatile uint64_t m_endFrame; > volatile uint32_t m_updateCounter; > ``` > > Should we mark these as atomic instead of volatile? I believe they are > marked as volatile exactly to deal with this threading race. However, iirc, > using volatile for such things is not entirely correct. cc'ing Geoff / > Yusuke who probably know for sure. volatile is unsafe here and can give us races (e.g. on 32-bit we could in principle read while only the half of m_startFrame has been written); in the majority of cases like this we should be using atomics with relaxed ordering which shouldn't have any perf penalty.

David Kilzer (:ddkilzer)

Comment 16 2021-08-12 11:03:26 PDT

Created attachment 435435 [details] Patch v4

David Kilzer (:ddkilzer)

Comment 17 2021-08-12 11:05:14 PDT

(In reply to David Kilzer (:ddkilzer) from comment #16) > Created attachment 435435 [details] > Patch v4 Use the correct macro (SUPPRESS_TSAN, not TSAN_SUPPRESS) and put it in a location to make GCC happy?! Also removed the ASSERT() macros as this patch is only focused on suppressing the false positive TSan reports now.

David Kilzer (:ddkilzer)

Comment 18 2021-08-12 17:54:04 PDT

Comment on attachment 435435 [details] Patch v4 Setting cq+ because this change has no impact on non-TSan builds, so the api-ios test failure is unrelated: <https://ews-build.webkit.org/#/builders/9/builds/52389>

EWS

Comment 19 2021-08-12 18:13:26 PDT

Committed r281001 (240494@main): <https://commits.webkit.org/240494@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435435 [details].

Note You need to log in before you can comment on or make changes to this bug.