New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 684458 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Data race in media::VideoFrameCompositor::ProcessNewFrame

Project Member Reported by ClusterFuzz, Jan 24 2017

Issue description

Cc: msrchandra@chromium.org
Components: Blink>Media>Video
Labels: Test-Predator-Correct-CLs
Owner: avayvod@chromium.org
Status: Assigned (was: Untriaged)
Assigning to the concern owner from Find it results --
The result is a list of CLs that change the crashed files. 

Author: avayvod
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/590011e3f3b68faf7f68592b3b931181e0cb4672
Time: Fri Jan 20 02:01:00 2017
Lines 246-248 of file webmediaplayer_impl.cc which potentially caused crash are changed in this cl (frame #1, "media::WebMediaPlayerImpl::WebMediaPlayerImpl"). 

File render_frame_impl.cc is changed in this cl (and is part of stack frame #2, "content::RenderFrameImpl::createMediaPlayer")
Minimum distance from crash line to modified line: 0. (file: webmediaplayer_impl.cc, crashed on: 246, modified: 246). 

Author: avayvod
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/cc273ddad35337b82ddfdb77248986fc928d035e
Time: Thu Jan 19 19:35:12 2017
Lines 142 of file video_frame_compositor.cc which potentially caused crash are changed in this cl (frame #1, "media::VideoFrameCompositor::Stop").
Minimum distance from crash line to modified line: 0. (file: video_frame_compositor.cc, crashed on: 142, modified: 142).

@avayvod -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Status: Started (was: Assigned)
Sent https://codereview.chromium.org/2651323002 with the fix.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 27 2017

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

commit b5c8d0d1bed4056880ad70ed949714f9edc0f81b
Author: avayvod <avayvod@chromium.org>
Date: Fri Jan 27 01:11:35 2017

[Video] Reset new processed frame callback on the compositor thread.

The callback is set and used on the compositor thread but is reset on the
media thread, causing a data race.
Moved resetting the callback to the OnRenderingStateChange when the state is
set to false.

BUG= 684458 
TEST=manual

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

[modify] https://crrev.com/b5c8d0d1bed4056880ad70ed949714f9edc0f81b/media/blink/video_frame_compositor.cc

Labels: Merge-Request-57
Labels: M-57
Project Member

Comment 6 by ClusterFuzz, Jan 28 2017

ClusterFuzz has detected this issue as fixed in range 446318:446606.

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

Fuzzer: inferno_flicker
Job Type: linux_tsan_chrome_mp
Platform Id: linux

Crash Type: Data race READ 8
Crash Address: 0x7b40000010b8
Crash State:
  media::VideoFrameCompositor::ProcessNewFrame
  media::VideoFrameCompositor::PaintSingleFrame
  base::internal::Invoker<base::internal::BindState<void
  
Sanitizer: thread (TSAN)

Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=444758:445138
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_tsan_chrome_mp&range=446318:446606

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv94HbYarL7l2ZQKwuZvdVvx_ow_dH_k9ky0ZxTDbWXU9-bWFhEmsKkGea2N0dnF27vRcyoJOWm5JPsnAVQh3R69kNTUBneaWAjj01_jirZ0r6kP3tI8nFUKudME1bpXQfXbz5j3i1arCXu4c01LzDylvLwOelfmG4Z5104yxLIrjpypzkqlat1lLhKfoO1Qeb1yUnIQBdd05hnTzj78qZMj5xEriEB6uMhh4bjALxE9EjaettSU9Lz5gll1KoDsGHQlRkeFwKPoq-DxZi7ASYumV5DkfLo20KgvqaSmP8y7vl4bEuBffygwJFkSUvan-YW1KE_PPSli0raa0OZL1dSo_-6DQa3sFdBBZFk9DMhubdcVdtGA?testcase_id=6225523761217536


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 7 by ClusterFuzz, Jan 28 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Started)
ClusterFuzz testcase 6225523761217536 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Project Member

Comment 8 by sheriffbot@chromium.org, Jan 30 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

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

Comment 9 by gov...@chromium.org, Jan 30 2017

Please merge  your change to M57 branch 2987 ASAP.If merge happens today before 5:00 PM PT, then we can take it for tomorrow's last M57 Dev release. Thank you.
Project Member

Comment 10 by bugdroid1@chromium.org, Jan 30 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8249e65739295d3aa951eba40de6221e3b11aae6

commit 8249e65739295d3aa951eba40de6221e3b11aae6
Author: Anton Vayvod <avayvod@chromium.org>
Date: Mon Jan 30 20:14:27 2017

[Video] Reset new processed frame callback on the compositor thread.

The callback is set and used on the compositor thread but is reset on the
media thread, causing a data race.
Moved resetting the callback to the OnRenderingStateChange when the state is
set to false.

BUG= 684458 
TEST=manual

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

Review-Url: https://codereview.chromium.org/2660993002 .
Cr-Commit-Position: refs/branch-heads/2987@{#178}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/8249e65739295d3aa951eba40de6221e3b11aae6/media/blink/video_frame_compositor.cc

Sign in to add a comment