diff options
| author | scherkus <scherkus@chromium.org> | 2014-09-09 00:42:25 -0700 |
|---|---|---|
| committer | Commit bot <commit-bot@chromium.org> | 2014-09-09 07:48:49 +0000 |
| commit | 0abb84994f293d6fc17e5c2d3e21a933d4f73fbc (patch) | |
| tree | 39a5386bb805669f190b188aec09f91af7ea90d9 /media/base/pipeline.cc | |
| parent | 01c8ca08dc8200c3c674726c1183916ebc7f2bab (diff) | |
| download | chromium_src-0abb84994f293d6fc17e5c2d3e21a933d4f73fbc.zip chromium_src-0abb84994f293d6fc17e5c2d3e21a933d4f73fbc.tar.gz chromium_src-0abb84994f293d6fc17e5c2d3e21a933d4f73fbc.tar.bz2 | |
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}
Diffstat (limited to 'media/base/pipeline.cc')
| -rw-r--r-- | media/base/pipeline.cc | 6 |
1 files changed, 5 insertions, 1 deletions
diff --git a/media/base/pipeline.cc b/media/base/pipeline.cc index 42a796f..6e680d5 100644 --- a/media/base/pipeline.cc +++ b/media/base/pipeline.cc @@ -405,10 +405,14 @@ void Pipeline::DoStop(const PipelineStatusCB& done_cb) { DCHECK(task_runner_->BelongsToCurrentThread()); 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_); - renderer_.reset(); + renderer.swap(renderer_); } + renderer.reset(); text_renderer_.reset(); if (demuxer_) { |