New issue
Advanced search Search tips

Issue 612948 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Data race in blink::PannerHandler::updateDirtyState

Project Member Reported by ClusterFuzz, May 18 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6446556059795456

Fuzzer: inferno_flicker
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race WRITE 1
Crash Address: 0x7d4800009999
Crash State:
  blink::PannerHandler::updateDirtyState
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96yX5g7yS_uCCGM7AneMDDHu9rhcFjGcNlrFTRZBcCX6Y3AKKtbIbFiY6sx9IQkQrAxeV0-0XtZLPgcI_RFtXSq0GNTGnfCyDS1kxYl2njH9vnAksoFPuWqsF-caiquoCsC-hbsP_Ql5eQPUu9xVcmcTX6tmcTVsaa1w3cmXhjnYQcZABY


Filer: mmohammad

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Owner: rtoy@chromium.org
Status: Assigned (was: Available)
Suspected CL : https://chromium.googlesource.com/chromium/src/+/2f2105fe40e9097470fe79d9d7b6fe2d88e73c38

rtoy@ Could you please look into this. Thanks

Comment 2 by rtoy@chromium.org, May 18 2016

Components: Blink>WebAudio
Project Member

Comment 3 by ClusterFuzz, May 19 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5576556679790592

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 4
Crash Address: 0x7ee57860dfb4
Crash State:
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  blink::AudioDestinationHandler::render
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96zoofhcB8Zgt6qkAhwno3NCD1YAQOpwUp26IVq9bcDxQst2lmdds_BS-wpyQUD9g_CrqrNjSlMZ8Xcz469d_4WM03kUv2LLA1fsE09PURaZHQZEmBeziROYHCovsqlBGEuN9vBVe4bwHeRX_2w38pMw_9oFuu4jEiMqa8zOeXFBjMsS2g


Filer: ranjitkan

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
Project Member

Comment 4 by ClusterFuzz, May 20 2016

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6082954530390016

Fuzzer: inferno_flicker
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 4
Crash Address: 0x7d4800009eb0
Crash State:
  blink::PannerHandler::updateDirtyState
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95w60N0g2SZDEdfFWeNrBCsxRO4H_m0hrMkslRUHlSDZH0w5HMWJm-ZMluQ5sviqtUuap--lGXqfL40r4erj7XQHYO66nIvv396K1sOHOEEBZRMNnSqlRNHl6TqW5r0hQH8-qe-cHrO_LwK_jOV9d3asfopy4--1WlMF6Bs7FmMp7bfMHU


Filer: ranjitkan

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

Comment 5 by rtoy@chromium.org, May 23 2016

Cc: hongchan@chromium.org

Comment 6 by sigbjo...@opera.com, May 24 2016

 Issue 613332  a duplicate?

Comment 7 by rtoy@chromium.org, May 24 2016

 Issue 613332  c#2 crash stack is identical to the crash stack here in c#3. (But the stacks here in c#1 and #3 are different.)

The code causing this is all closely related.
Project Member

Comment 8 by bugdroid1@chromium.org, May 24 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a6beb9f31fba1c7892fbfb4911f34e5a13492183

commit a6beb9f31fba1c7892fbfb4911f34e5a13492183
Author: rtoy <rtoy@chromium.org>
Date: Tue May 24 23:48:06 2016

Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.

BUG= 612948 ,  612955 ,  613332 
TEST=none

Review-Url: https://codereview.chromium.org/2006563003
Cr-Commit-Position: refs/heads/master@{#395738}

[modify] https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183/third_party/WebKit/Source/modules/webaudio/AudioListener.cpp
[modify] https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183/third_party/WebKit/Source/modules/webaudio/AudioListener.h
[modify] https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183/third_party/WebKit/Source/modules/webaudio/PannerNode.cpp
[modify] https://crrev.com/a6beb9f31fba1c7892fbfb4911f34e5a13492183/third_party/WebKit/Source/modules/webaudio/PannerNode.h

Project Member

Comment 9 by ClusterFuzz, May 25 2016

ClusterFuzz has detected this testcase as flaky and is unable to reproduce it in the original crash revision. Skipping fixed testing check and marking it as potentially fixed.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6446556059795456

Fuzzer: inferno_flicker
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race WRITE 1
Crash Address: 0x7d4800009999
Crash State:
  blink::PannerHandler::updateDirtyState
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96yX5g7yS_uCCGM7AneMDDHu9rhcFjGcNlrFTRZBcCX6Y3AKKtbIbFiY6sx9IQkQrAxeV0-0XtZLPgcI_RFtXSq0GNTGnfCyDS1kxYl2njH9vnAksoFPuWqsF-caiquoCsC-hbsP_Ql5eQPUu9xVcmcTX6tmcTVsaa1w3cmXhjnYQcZABY


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 10 by ClusterFuzz, May 25 2016

ClusterFuzz has detected this issue as fixed in range 395707:395767.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6082954530390016

Fuzzer: inferno_flicker
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 4
Crash Address: 0x7d4800009eb0
Crash State:
  blink::PannerHandler::updateDirtyState
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=395707:395767

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95w60N0g2SZDEdfFWeNrBCsxRO4H_m0hrMkslRUHlSDZH0w5HMWJm-ZMluQ5sviqtUuap--lGXqfL40r4erj7XQHYO66nIvv396K1sOHOEEBZRMNnSqlRNHl6TqW5r0hQH8-qe-cHrO_LwK_jOV9d3asfopy4--1WlMF6Bs7FmMp7bfMHU


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 11 by ClusterFuzz, May 25 2016

ClusterFuzz has detected this issue as fixed in range 395707:395767.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5576556679790592

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 4
Crash Address: 0x7ee57860dfb4
Crash State:
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  blink::AudioDestinationHandler::render
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=395707:395767

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96zoofhcB8Zgt6qkAhwno3NCD1YAQOpwUp26IVq9bcDxQst2lmdds_BS-wpyQUD9g_CrqrNjSlMZ8Xcz469d_4WM03kUv2LLA1fsE09PURaZHQZEmBeziROYHCovsqlBGEuN9vBVe4bwHeRX_2w38pMw_9oFuu4jEiMqa8zOeXFBjMsS2g


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 12 by ClusterFuzz, May 26 2016

ClusterFuzz has detected this issue as fixed in range 395707:395767.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=6446556059795456

Fuzzer: inferno_flicker
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race WRITE 1
Crash Address: 0x7d4800009999
Crash State:
  blink::PannerHandler::updateDirtyState
  blink::AudioListener::updateState
  blink::AbstractAudioContext::handlePreRenderTasks
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=393907:394251
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=395707:395767

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv96yX5g7yS_uCCGM7AneMDDHu9rhcFjGcNlrFTRZBcCX6Y3AKKtbIbFiY6sx9IQkQrAxeV0-0XtZLPgcI_RFtXSq0GNTGnfCyDS1kxYl2njH9vnAksoFPuWqsF-caiquoCsC-hbsP_Ql5eQPUu9xVcmcTX6tmcTVsaa1w3cmXhjnYQcZABY


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.

Comment 13 by rtoy@chromium.org, May 26 2016

Status: Fixed (was: Assigned)
All test cases are fixed.

Closing.

Comment 14 by rtoy@chromium.org, May 26 2016

Labels: Merge-Request-52 M-52
I'd like to get this fix into M52

Comment 15 by tin...@google.com, May 26 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Project Member

Comment 16 by bugdroid1@chromium.org, May 26 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/84ecd66b39c24e7614412f08571b98380d7cd41d

commit 84ecd66b39c24e7614412f08571b98380d7cd41d
Author: Raymond Toy <rtoy@chromium.org>
Date: Thu May 26 17:16:31 2016

Simplify notification of a dirty listener.

Instead of the listener updating the dirty state of all panners, just
set a dirty state flag in the listener.  Then, when a panner needs
to compute a new azimuth/elevation gain or cone gain, the panner
just checks to see if the listener is dirty.

This gets rid of the data race where both the audio thread and the
main thread are updating the dirty state of a panner.  The checking
and setting of the listener state happens on the audio thread.

The race from reading from the hash table of panners in AudioListener
and adding to the table from the main thread is also removed.

BUG= 612948 ,  612955 ,  613332 
TEST=none

Review-Url: https://codereview.chromium.org/2006563003
Cr-Commit-Position: refs/heads/master@{#395738}
(cherry picked from commit a6beb9f31fba1c7892fbfb4911f34e5a13492183)

Review URL: https://codereview.chromium.org/2012773005 .

Cr-Commit-Position: refs/branch-heads/2743@{#78}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/84ecd66b39c24e7614412f08571b98380d7cd41d/third_party/WebKit/Source/modules/webaudio/AudioListener.cpp
[modify] https://crrev.com/84ecd66b39c24e7614412f08571b98380d7cd41d/third_party/WebKit/Source/modules/webaudio/AudioListener.h
[modify] https://crrev.com/84ecd66b39c24e7614412f08571b98380d7cd41d/third_party/WebKit/Source/modules/webaudio/PannerNode.cpp
[modify] https://crrev.com/84ecd66b39c24e7614412f08571b98380d7cd41d/third_party/WebKit/Source/modules/webaudio/PannerNode.h

Project Member

Comment 17 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment