New issue
Advanced search Search tips

Issue 657001 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Data race in media::AudioRendererImpl::StopTicking

Project Member Reported by ClusterFuzz, Oct 18 2016

Issue description

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race WRITE 1
Crash Address: 0x7b480001eb88
Crash State:
  media::AudioRendererImpl::StopTicking
  media::RendererImpl::OnBufferingStateChange
  media::RendererImpl::RendererClientInternal::OnBufferingStateChange
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=425834:425887

Minimized Testcase (655.62 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96_CTQdNqjmvGEXJ8Pnfx9zAadVvzDKic9ULNjvp28J3auH_HIbvIJKfGeLi9dI9NG5RSlBSmLJfZr1DpxiSUYZHpR0beqaJ0zmUe8_uycx6Glb5_d7Fg1Q58JjAPuG4rbukE8WNmPtoj82ek8HXa7JhTKqD4CldfDsqc2llkxp4hmQ6JM?testcase_id=4898925561774080

Issue manually filed by: ajha

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

Comment 1 by ajha@chromium.org, Oct 18 2016

Cc: ajha@chromium.org
Components: Tools>Test>FindIt>CorrectResult
Labels: M-56 Te-Logged
Owner: chcunningham@chromium.org
Status: Assigned (was: Untriaged)
Suspected CLs	The result is a list of CLs that change the crashed files.

Author: chcunningham
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/9409db28cd7402d4bad8022afbd2b3719de72d08
Time: Tue Oct 18 02:49:58 2016
Lines 175, 180-194, 206 of file audio_renderer_impl.cc which potentially caused crash are changed in this cl (frame #0, "media::AudioRendererImpl::CurrentMediaTime"; frame #1, "non-virtual thunk to media::AudioRendererImpl::CurrentMediaTime").

Lines 175, 180-194, 206 of file audio_renderer_impl.cc which potentially caused crash are changed in this cl (frame #0, "media::AudioRendererImpl::CurrentMediaTime"; frame #1, "non-virtual thunk to media::AudioRendererImpl::CurrentMediaTime").
Minimum distance from crash line to modified line: 0. (file: audio_renderer_impl.cc, crashed on: 175, modified: 175).

Suspected Project: chromium
=============================================================================================================

chcunningham@: Could you please take a look at this.

Thank you!
Components:
On it. 
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 19 2016

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

commit e2f429e24bdbcd8e8379b909879e2c729490e1c6
Author: chcunningham <chcunningham@chromium.org>
Date: Wed Oct 19 22:41:05 2016

AudioRendererImpl: Fix data race.

Previous CL introduced multithreaded access to |rendering_|.
https://codereview.chromium.org/2423903003/

BUG= 657001 ,648710

Review-Url: https://chromiumcodereview.appspot.com/2434003002
Cr-Commit-Position: refs/heads/master@{#426312}

[modify] https://crrev.com/e2f429e24bdbcd8e8379b909879e2c729490e1c6/media/renderers/audio_renderer_impl.cc

Status: Fixed (was: Assigned)
Project Member

Comment 5 by ClusterFuzz, Oct 21 2016

ClusterFuzz has detected this issue as fixed in range 426296:426375.

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

Fuzzer: inferno_layout_test_unmodified
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race WRITE 1
Crash Address: 0x7b480001eb88
Crash State:
  media::AudioRendererImpl::StopTicking
  media::RendererImpl::OnBufferingStateChange
  media::RendererImpl::RendererClientInternal::OnBufferingStateChange
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=425834:425887
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=426296:426375

Minimized Testcase (655.62 Kb): https://cluster-fuzz.appspot.com/download/AMIfv96_CTQdNqjmvGEXJ8Pnfx9zAadVvzDKic9ULNjvp28J3auH_HIbvIJKfGeLi9dI9NG5RSlBSmLJfZr1DpxiSUYZHpR0beqaJ0zmUe8_uycx6Glb5_d7Fg1Q58JjAPuG4rbukE8WNmPtoj82ek8HXa7JhTKqD4CldfDsqc2llkxp4hmQ6JM?testcase_id=4898925561774080

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 6 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f4d92703c616c8a35ce3a252016abf6e9b99913

commit 4f4d92703c616c8a35ce3a252016abf6e9b99913
Author: Chris Cunningham <chcunningham@chromium.org>
Date: Thu Oct 20 23:41:54 2016

[TO 55] AudioRendererImpl: Fix data race.

Previous CL introduced multithreaded access to |rendering_|.
https://codereview.chromium.org/2423903003/

BUG= 657001 ,648710

Review-Url: https://chromiumcodereview.appspot.com/2434003002
Cr-Commit-Position: refs/heads/master@{#426312}
(cherry picked from commit e2f429e24bdbcd8e8379b909879e2c729490e1c6)

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

Cr-Commit-Position: refs/branch-heads/2883@{#224}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/4f4d92703c616c8a35ce3a252016abf6e9b99913/media/renderers/audio_renderer_impl.cc

Comment 7 by dimu@google.com, Nov 4 2016

[Automated comment] removing mislabelled merge-merged-2840

Comment 8 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
Project Member

Comment 9 by bugdroid1@chromium.org, Nov 4 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/273ba7eecd4b8654aae31b6980d8094a39b9a392

commit 273ba7eecd4b8654aae31b6980d8094a39b9a392
Author: Chris Cunningham <chcunningham@chromium.org>
Date: Fri Nov 04 23:21:24 2016

[TO 54] AudioRendererImpl: Don't advance time when rendering stops.

This is a quick work-around for the bug below. AudioRendererImpl was
recently improved to estimate time's advancing along the last rendered
audio buffer between Render() calls. This means the clock is capable of
still advancing a bit after OnBufferingStateChange indicates we've run
out of data. While tehcnically correct, the clocks advancing causes
blink to emit 'timeupdate' events with time advancing after emitting
'waiting' indicating that no data is buffered. This change stops the
clock once the render has underflowed.

Longterm, blink should be improved to gaurd against emitting timeupdate
after waiting. This larger change will be done separately to give it
time to bake. https://codereview.chromium.org/2425463002/

BUG=648710

Review-Url: https://codereview.chromium.org/2423903003
Cr-Commit-Position: refs/heads/master@{#425871}
(cherry picked from commit 9409db28cd7402d4bad8022afbd2b3719de72d08)

--ALSO--

[TO 54] AudioRendererImpl: Fix data race.

Previous CL introduced multithreaded access to |rendering_|.
https://codereview.chromium.org/2423903003/

BUG= 657001 ,648710

Review-Url: https://chromiumcodereview.appspot.com/2434003002
Cr-Commit-Position: refs/heads/master@{#426312}
(cherry picked from commit e2f429e24bdbcd8e8379b909879e2c729490e1c6)

TBR=sandersd@chromium.org

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

Cr-Commit-Position: refs/branch-heads/2840@{#821}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

[modify] https://crrev.com/273ba7eecd4b8654aae31b6980d8094a39b9a392/media/renderers/audio_renderer_impl.cc
[modify] https://crrev.com/273ba7eecd4b8654aae31b6980d8094a39b9a392/media/renderers/audio_renderer_impl.h
[modify] https://crrev.com/273ba7eecd4b8654aae31b6980d8094a39b9a392/media/renderers/audio_renderer_impl_unittest.cc
[modify] https://crrev.com/273ba7eecd4b8654aae31b6980d8094a39b9a392/third_party/WebKit/LayoutTests/http/tests/media/video-play-stall.html

Project Member

Comment 10 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