Data race in blink::ReverbInputBuffer::Write |
||||
Issue descriptionDo a tsan build and navigate to http://googlechrome.github.io/web-audio-samples/samples/audio/shiny-drum-machine.html There is a data race reported by tsan: WARNING: ThreadSanitizer: data race (pid=1280) Write of size 8 at 0x7b24000047d8 by thread T20 (mutexes: write M395236): #0 blink::ReverbInputBuffer::Write(float const*, unsigned long) third_party/WebKit/Source/platform/audio/ReverbInputBuffer.cpp:49:7 (libblink_platform.so+0x1cb52e) #1 blink::ReverbConvolver::Process(blink::AudioChannel const*, blink::AudioChannel*, unsigned long) third_party/WebKit/Source/platform/audio/ReverbConvolver.cpp:191:17 (libblink_platform.so+0x1ca18f) #2 blink::Reverb::Process(blink::AudioBus const*, blink::AudioBus*, unsigned long) third_party/WebKit/Source/platform/audio/Reverb.cpp:224:23 (libblink_platform.so+0x1c82ec) #3 blink::ConvolverHandler::Process(unsigned long) third_party/WebKit/Source/modules/webaudio/ConvolverNode.cpp:86:16 (libblink_modules.so+0xa8dc98) #4 blink::AudioHandler::ProcessIfNecessary(unsigned long) third_party/WebKit/Source/modules/webaudio/AudioNode.cpp:346:7 (libblink_modules.so+0xa58275) #5 blink::AudioNodeOutput::Pull(blink::AudioBus*, unsigned long) third_party/WebKit/Source/modules/webaudio/AudioNodeOutput.cpp:140:13 (libblink_modules.so+0xa63793) #6 blink::AudioNodeInput::SumAllConnections(blink::AudioBus*, unsigned long) third_party/WebKit/Source/modules/webaudio/AudioNodeInput.cpp:191:40 (libblink_modules.so+0xa61cc1) #7 blink::AudioNodeInput::Pull(blink::AudioBus*, unsigned long) third_party/WebKit/Source/modules/webaudio/AudioNodeInput.cpp:221:3 (libblink_modules.so+0xa61d9d) #8 blink::AudioHandler::PullInputs(unsigned long) third_party/WebKit/Source/modules/webaudio/AudioNode.cpp:380:12 (libblink_modules.so+0xa585e4) The audio thread is writing write_index_ while the convolver background thread is reading write_index_.
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1f2b8cb64f526e71a57386c42645d3cfe62048e0 commit 1f2b8cb64f526e71a57386c42645d3cfe62048e0 Author: Raymond Toy <rtoy@chromium.org> Date: Fri Jun 16 21:44:33 2017 Fix data race in accessing write_index in ReverbInputBuffer Use atomics to protect access to write_index_. This can be read from multiple threads (performing the convolution stages) and also written by the audio thread when new data needs to be buffered for convolution. BUG= 732918 TEST= Change-Id: I8594c350492977253667f1d38e432c97e73554cc Reviewed-on: https://chromium-review.googlesource.com/533641 Commit-Queue: Raymond Toy <rtoy@chromium.org> Reviewed-by: Hongchan Choi <hongchan@chromium.org> Cr-Commit-Position: refs/heads/master@{#480185} [modify] https://crrev.com/1f2b8cb64f526e71a57386c42645d3cfe62048e0/third_party/WebKit/Source/platform/audio/ReverbInputBuffer.cpp [modify] https://crrev.com/1f2b8cb64f526e71a57386c42645d3cfe62048e0/third_party/WebKit/Source/platform/audio/ReverbInputBuffer.h
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ccfa72e7b30a947010c262b5ad3f80814cddba38 commit ccfa72e7b30a947010c262b5ad3f80814cddba38 Author: Raymond Toy <rtoy@chromium.org> Date: Fri Jun 23 19:13:14 2017 Revert "Fix data race in accessing write_index in ReverbInputBuffer" This reverts commit 1f2b8cb64f526e71a57386c42645d3cfe62048e0. Reason for revert: Causes crashes. The CHECK is failing Original change's description: > Fix data race in accessing write_index in ReverbInputBuffer > > Use atomics to protect access to write_index_. This can be read from > multiple threads (performing the convolution stages) and also written > by the audio thread when new data needs to be buffered for > convolution. > > BUG= 732918 > TEST= > > Change-Id: I8594c350492977253667f1d38e432c97e73554cc > Reviewed-on: https://chromium-review.googlesource.com/533641 > Commit-Queue: Raymond Toy <rtoy@chromium.org> > Reviewed-by: Hongchan Choi <hongchan@chromium.org> > Cr-Commit-Position: refs/heads/master@{#480185} TBR=rtoy@chromium.org,hongchan@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 732918 Change-Id: I463a39e424a4b28fc7c7baefe8cebcb04c82cab6 Reviewed-on: https://chromium-review.googlesource.com/545855 Reviewed-by: Raymond Toy <rtoy@chromium.org> Reviewed-by: Hongchan Choi <hongchan@chromium.org> Commit-Queue: Raymond Toy <rtoy@chromium.org> Cr-Commit-Position: refs/heads/master@{#481985} [modify] https://crrev.com/ccfa72e7b30a947010c262b5ad3f80814cddba38/third_party/WebKit/Source/platform/audio/ReverbInputBuffer.cpp [modify] https://crrev.com/ccfa72e7b30a947010c262b5ad3f80814cddba38/third_party/WebKit/Source/platform/audio/ReverbInputBuffer.h
,
Jul 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e027b261dc44c339f395f2dba24c36923c405018 commit e027b261dc44c339f395f2dba24c36923c405018 Author: Raymond Toy <rtoy@chromium.org> Date: Mon Jul 10 20:01:45 2017 Fix data race in accessing write_index in ReverbInputBuffer Use atomics to protect access to write_index_. This can be read from multiple threads (performing the convolution stages) and also written by the audio thread when new data needs to be buffered for convolution. Ran the test from the bug to verify the race is gone. (The race was 100% reproducible with that test). BUG= 732918 TEST=none Change-Id: Iefa3cf1e1fc182e714ad4e330287cc83be75c36a Reviewed-on: https://chromium-review.googlesource.com/546095 Reviewed-by: Hongchan Choi <hongchan@chromium.org> Commit-Queue: Raymond Toy <rtoy@chromium.org> Cr-Commit-Position: refs/heads/master@{#485356} [modify] https://crrev.com/e027b261dc44c339f395f2dba24c36923c405018/third_party/WebKit/Source/platform/audio/ReverbInputBuffer.cpp [modify] https://crrev.com/e027b261dc44c339f395f2dba24c36923c405018/third_party/WebKit/Source/platform/audio/ReverbInputBuffer.h
,
Jul 13 2017
Verified using the test case that the race is gone.
,
Jul 14 2017
,
Oct 22 2017
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||
►
Sign in to add a comment |
||||
Comment 1 by rtoy@chromium.org
, Jun 13 2017Labels: -Restrict-View-Google Restrict-View-SecurityTeam