Bug 229014 - ThreadSanitizer: data race in WebCore::CARingBufferStorageVector::setCurrentFrameBounds() / getCurrentFrameBounds()
Summary: ThreadSanitizer: data race in WebCore::CARingBufferStorageVector::setCurrentF...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-08-11 15:47 PDT by David Kilzer (:ddkilzer)
Modified: 2021-08-12 18:13 PDT (History)
11 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 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
Comment 1 Radar WebKit Bug Importer 2021-08-11 15:48:00 PDT
<rdar://problem/81817224>
Comment 2 David Kilzer (:ddkilzer) 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
Comment 3 Chris Dumez 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.
Comment 4 David Kilzer (:ddkilzer) 2021-08-11 15:53:31 PDT
Created attachment 435381 [details]
Patch v1
Comment 5 David Kilzer (:ddkilzer) 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.
Comment 6 Chris Dumez 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.
Comment 7 David Kilzer (:ddkilzer) 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?
Comment 8 Chris Dumez 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?
Comment 9 David Kilzer (:ddkilzer) 2021-08-11 16:47:30 PDT
Created attachment 435385 [details]
Patch v2
Comment 10 Jer Noble 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.
Comment 11 Jer Noble 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?
Comment 12 David Kilzer (:ddkilzer) 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.
Comment 13 Chris Dumez 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.
Comment 14 David Kilzer (:ddkilzer) 2021-08-11 18:01:22 PDT
Created attachment 435389 [details]
Patch v3
Comment 15 Sam Sneddon [:gsnedders] 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.
Comment 16 David Kilzer (:ddkilzer) 2021-08-12 11:03:26 PDT
Created attachment 435435 [details]
Patch v4
Comment 17 David Kilzer (:ddkilzer) 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.
Comment 18 David Kilzer (:ddkilzer) 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>
Comment 19 EWS 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].