Commit 0abb8499 authored by scherkus's avatar scherkus Committed by Commit bot

Fix potential deadlock between Pipeline and VideoRendererImpl.

I don't believe we hit this in practice since the two lock acquiring
sequences we make happen as separate tasks executed on the same
thread.

To summarize the ThreadSanitizer warning:
  - Task A acquires VideoRendererImpl -> Pipeline lock via
    FrameReady() -> GetMediaDuration()
  - Task B acquires Pipeline -> VideoRendererImpl lock via
    DoStop() -> ~VideoRendererImpl()

Solution: don't hold onto the Pipeline lock when destroying renderers.

BUG=412097

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

Cr-Commit-Position: refs/heads/master@{#293890}
parent 01c8ca08
...@@ -405,10 +405,14 @@ void Pipeline::DoStop(const PipelineStatusCB& done_cb) { ...@@ -405,10 +405,14 @@ void Pipeline::DoStop(const PipelineStatusCB& done_cb) {
DCHECK(task_runner_->BelongsToCurrentThread()); DCHECK(task_runner_->BelongsToCurrentThread());
DCHECK(!pending_callbacks_.get()); DCHECK(!pending_callbacks_.get());
// TODO(scherkus): Enforce that Renderer is only called on a single thread,
// even for accessing media time http://crbug.com/370634
scoped_ptr<Renderer> renderer;
{ {
base::AutoLock auto_lock(lock_); base::AutoLock auto_lock(lock_);
renderer_.reset(); renderer.swap(renderer_);
} }
renderer.reset();
text_renderer_.reset(); text_renderer_.reset();
if (demuxer_) { if (demuxer_) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment