Crash under RemoteAudioDestination::render(): Thread[3] EXC_BAD_ACCESS (SIGSEGV) (KERN_INVALID_ADDRESS at 0x0000000000000000) [ 0] 0x00007ff917d41ec0 WebCore`WebCore::CARingBuffer::fetchIfHasEnoughData(AudioBufferList*, unsigned long, unsigned long long, WebCore::CARingBuffer::FetchMode) [inlined] std::__1::unique_ptr<WebCore::CARingBufferStorage, std::__1::default_delete<WebCore::CARingBufferStorage> >::get() const at memory:2361:19 0x00007ff917d41eb4: movq %rcx, %r13 0x00007ff917d41eb7: movq %rdx, %r15 0x00007ff917d41eba: movq %rsi, %r12 0x00007ff917d41ebd: movq %rdi, %rbx -> 0x00007ff917d41ec0: movq (%rdi), %rdi 0x00007ff917d41ec3: movq (%rdi), %rax 0x00007ff917d41ec6: callq *0x38(%rax) 0x00007ff917d41ec9: movq (%rbx), %rdi 0x00007ff917d41ecc: movq (%rdi), %rax [ 0] 0x00007ff917d41ec0 WebCore`WebCore::CARingBuffer::fetchIfHasEnoughData(AudioBufferList*, unsigned long, unsigned long long, WebCore::CARingBuffer::FetchMode) [inlined] WTF::UniqueRef<WebCore::CARingBufferStorage>::operator->() at UniqueRef.h:70 [ 0] 0x00007ff917d41ec0 WebCore`WebCore::CARingBuffer::fetchIfHasEnoughData(AudioBufferList*, unsigned long, unsigned long long, WebCore::CARingBuffer::FetchMode) [inlined] WebCore::CARingBuffer::updateFrameBounds() at CARingBuffer.cpp:329 325 } 326 327 void CARingBuffer::updateFrameBounds() 328 { -> 329 m_buffers->updateFrameBounds(); 330 } 331 332 bool CARingBuffer::fetchIfHasEnoughData(AudioBufferList* list, size_t frameCount, uint64_t startFrame, FetchMode mode) 333 { [ 0] 0x00007ff917d41ec0 WebCore`WebCore::CARingBuffer::fetchIfHasEnoughData(AudioBufferList*, unsigned long, unsigned long long, WebCore::CARingBuffer::FetchMode) [inlined] WebCore::CARingBuffer::getCurrentFrameBounds(unsigned long long&, unsigned long long&) at CARingBuffer.cpp:269 265 } 266 267 void CARingBuffer::getCurrentFrameBounds(uint64_t& startFrame, uint64_t& endFrame) 268 { -> 269 updateFrameBounds(); 270 getCurrentFrameBoundsWithoutUpdate(startFrame, endFrame); 271 } 272 273 void CARingBuffer::getCurrentFrameBoundsWithoutUpdate(uint64_t& startFrame, uint64_t& endFrame) [ 0] 0x00007ff917d41ec0 WebCore`WebCore::CARingBuffer::fetchIfHasEnoughData(AudioBufferList*, unsigned long, unsigned long long, WebCore::CARingBuffer::FetchMode) + 32 at CARingBuffer.cpp:336 [ 1] 0x00007ff918eb07bc WebKit`WebKit::RemoteAudioDestination::render(double, unsigned long long, unsigned int, AudioBufferList*) + 84 at RemoteAudioDestinationManager.cpp:126:27 [ 2] 0x000000010667f217 CoreAudio`ausdk::AUInputElement::PullInput(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int) [inlined] ausdk::AUInputElement::PullInputWithBufferList(unsigned int&, AudioTimeStamp const&, unsigned int, unsigned int, AudioBufferList&) + 75 at AUInputElement.h:69:15
<rdar://76643365>
Created attachment 426263 [details] Patch
Comment on attachment 426263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426263&action=review > Source/WebKit/ChangeLog:10 > + GPUConnectionToWebProcess::didClose() would get called and en up destroying s/en up/end up/g > Source/WebKit/ChangeLog:43 > + was not thread safe. Even if m_isPlaying was atomic, m_isPlaying get set to true I feel “m_isPlaying" here is the root cause of this crash. But I like your refactoring.
Created attachment 426268 [details] Patch
(In reply to Peng Liu from comment #3) > Comment on attachment 426263 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426263&action=review > > > Source/WebKit/ChangeLog:10 > > + GPUConnectionToWebProcess::didClose() would get called and en up destroying > > s/en up/end up/g > > > Source/WebKit/ChangeLog:43 > > + was not thread safe. Even if m_isPlaying was atomic, m_isPlaying get set to true > > I feel “m_isPlaying" here is the root cause of this crash. But I like your > refactoring. The root cause of this crash is that we weren't calling m_audioOutputUnitAdaptor.stop() in the destructor (or before destroying). There was no way to make this safe using m_isPlaying.
Comment on attachment 426263 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=426263&action=review >>> Source/WebKit/ChangeLog:10 >>> + GPUConnectionToWebProcess::didClose() would get called and en up destroying >> >> s/en up/end up/g > > The root cause of this crash is that we weren't calling m_audioOutputUnitAdaptor.stop() in the destructor (or before destroying). There was no way to make this safe using m_isPlaying. Hmm, just curious, if we call `m_audioOutputUnitAdaptor.stop()` before destroying and make `m_isPlaying` atomic, will the crash be fixed?
(In reply to Peng Liu from comment #6) > Comment on attachment 426263 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=426263&action=review > > >>> Source/WebKit/ChangeLog:10 > >>> + GPUConnectionToWebProcess::didClose() would get called and en up destroying > >> > >> s/en up/end up/g > > > > The root cause of this crash is that we weren't calling m_audioOutputUnitAdaptor.stop() in the destructor (or before destroying). There was no way to make this safe using m_isPlaying. > > Hmm, just curious, if we call `m_audioOutputUnitAdaptor.stop()` before > destroying and make `m_isPlaying` atomic, will the crash be fixed? I you call m_audioOutputUnitAdaptor.stop() before destroying, the crash would be fixed. You would not need to make m_isPlaying atomic to address the crash.
Comment on attachment 426268 [details] Patch Will add a test.
Created attachment 426284 [details] Patch
Committed r276188 (236670@main): <https://commits.webkit.org/236670@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 426284 [details].