Avoid deadlock between Pipeline and RendererImpl.

media/base/pipeline.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/pipeline.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 135 matching lines @@
   volume_ = volume;
   if (running_) {
     task_runner_->PostTask(
         FROM_HERE,
         base::Bind(
             &Pipeline::VolumeChangedTask, weak_factory_.GetWeakPtr(), volume));
   }
 }
 
 TimeDelta Pipeline::GetMediaTime() const {
+  if (!renderer_)
+    return TimeDelta();
+
+  TimeDelta media_time = renderer_->GetMediaTime();
+
   base::AutoLock auto_lock(lock_);
-  return renderer_ ? std::min(renderer_->GetMediaTime(), duration_)
-                   : TimeDelta();
+  return std::min(media_time, duration_);
 }
 
 Ranges<TimeDelta> Pipeline::GetBufferedTimeRanges() const {
   base::AutoLock auto_lock(lock_);
   return buffered_time_ranges_;
 }
 
 TimeDelta Pipeline::GetMediaDuration() const {
   base::AutoLock auto_lock(lock_);
   return duration_;
@@ skipping 110 matching lines @@
 }
 
 void Pipeline::SetDuration(TimeDelta duration) {
   DCHECK(IsRunning());
   media_log_->AddEvent(
       media_log_->CreateTimeEvent(
           MediaLogEvent::DURATION_SET, "duration", duration));
   UMA_HISTOGRAM_LONG_TIMES("Media.Duration", duration);
 
   base::AutoLock auto_lock(lock_);
   duration_ = duration;

[scherkus (not reviewing), 2014/08/27 23:41:11]

I'm pretty sure this runs on the media thread

It seems silly that we both:
  a) store the duration and have a getter under lock
  b) fire a callback that the duration has changed

We also pass the locked version when initializing Renderer. It'd be good to
stick to a single idiom (polling for duration w/ lock OR callback that contains
the new duration) ... but not both.

Any thoughts whether cleaning that up would also fix the deadlock?

[xhwang, 2014/08/28 00:46:28]

That's true for FFmepgDemuxer, but not for ChunkDemuxer:

#0 0x7f6713187c1e base::debug::StackTrace::StackTrace()
#1 0x7f671321ee55 logging::LogMessage::~LogMessage()
#2 0x7f670aedcec0 media::Pipeline::SetDuration()
#3 0x7f670af64dd6 media::ChunkDemuxer::UpdateDuration()
#4 0x7f670af611ae media::ChunkDemuxer::OnSourceInitDone()
#5 0x7f670af6b358 base::internal::RunnableAdapter<>::Run()
#6 0x7f670af6b289 base::internal::InvokeHelper<>::MakeItSo()
#7 0x7f670af6b207 base::internal::Invoker<>::Run()
#8 0x7f670af6680b base::Callback<>::Run()
#9 0x7f670af58fb2 media::SourceState::OnSourceInitDone()
#10 0x7f670af710d8 base::internal::RunnableAdapter<>::Run()
#11 0x7f670af71029 base::internal::InvokeHelper<>::MakeItSo()
#12 0x7f670af70fc7 base::internal::Invoker<>::Run()
#13 0x7f670af6680b base::Callback<>::Run()
#14 0x7f670b09e769 media::WebMStreamParser::ParseInfoAndTracks()
#15 0x7f670b09d9af media::WebMStreamParser::Parse()
#16 0x7f670af5ad9a media::SourceState::Append()
#17 0x7f670af61dff media::ChunkDemuxer::AppendData()
#18 0x7f670854aceb content::WebSourceBufferImpl::append()
#19 0x7f670c419d25 blink::SourceBuffer::appendBufferAsyncPart()
#20 0x7f670c41f537 blink::AsyncMethodRunner<>::fired()
#21 0x7f670c41f66f blink::Timer<>::fired()
#22 0x7f67155c8260 blink::ThreadTimers::sharedTimerFiredInternal()
#23 0x7f67155c7e95 blink::ThreadTimers::sharedTimerFired()
#24 0x7f671583e318 blink::Scheduler::tickSharedTimer()
#25 0x7f671583e781 blink::Scheduler::sharedTimerAdapter()
#26 0x7f6707ffad43 content::BlinkPlatformImpl::DoTimeout()
#27 0x7f6707ffe162 base::internal::RunnableAdapter<>::Run()
#28 0x7f6707ffe0cc base::internal::InvokeHelper<>::MakeItSo()
#29 0x7f6707ffe07a base::internal::Invoker<>::Run()
#30 0x7f671317044e base::Callback<>::Run()
#31 0x7f67133289e6 base::Timer::RunScheduledTask()
#32 0x7f6713328b1c base::BaseTimerTaskInternal::Run()
#33 0x7f6713328de2 base::internal::RunnableAdapter<>::Run()
#34 0x7f6713328d4c base::internal::InvokeHelper<>::MakeItSo()
#35 0x7f6713328cf5 base::internal::Invoker<>::Run()
#36 0x7f671317044e base::Callback<>::Run()
#37 0x7f671318e083 base::debug::TaskAnnotator::RunTask()
#38 0x7f6713244615 base::MessageLoop::RunTask()
#39 0x7f671324475b base::MessageLoop::DeferOrRunPendingTask()
#40 0x7f6713244985 base::MessageLoop::DoWork()
#41 0x7f6713255408 base::MessagePumpDefault::Run()
#42 0x7f6713244270 base::MessageLoop::RunHandler()
#43 0x7f67132a9ce2 base::RunLoop::Run()
#44 0x7f6713243b11 base::MessageLoop::Run()
#45 0x7f67086088f5 content::RendererMain()
#46 0x7f6707555bd6 content::RunZygote()
#47 0x7f6707555f09 content::RunNamedProcessTypeMain()
#48 0x7f670755a078 content::ContentMainRunnerImpl::Run()
#49 0x7f6707555425 content::ContentMain()
#50 0x7f6717921345 ChromeMain
#51 0x7f67179212f2 main
#52 0x7f670097078d __libc_start_main
#53 0x7f67179211f9 <unknown>

   if (!duration_change_cb_.is_null())
     duration_change_cb_.Run();
 }
 
 void Pipeline::OnStateTransition(PipelineStatus status) {
   DCHECK(task_runner_->BelongsToCurrentThread());
   // Force post to process state transitions after current execution frame.
   task_runner_->PostTask(
       FROM_HERE,
       base::Bind(
@@ skipping 132 matching lines @@
 }
 
 void Pipeline::OnStopCompleted(PipelineStatus status) {
   DVLOG(2) << __FUNCTION__;
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, kStopping);
   DCHECK(!renderer_);
   DCHECK(!text_renderer_);
 
   {
-    base::AutoLock l(lock_);
+    base::AutoLock auto_lock(lock_);
     running_ = false;
   }
 
   SetState(kStopped);
   filter_collection_.reset();
   demuxer_ = NULL;
 
   // If we stop during initialization/seeking we want to run |seek_cb_|
   // followed by |stop_cb_| so we don't leave outstanding callbacks around.
   if (!seek_cb_.is_null()) {
@@ skipping 216 matching lines @@
       base::Bind(&Pipeline::GetMediaDuration, base::Unretained(this)));
 }
 
 void Pipeline::BufferingStateChanged(BufferingState new_buffering_state) {
   DVLOG(1) << __FUNCTION__ << "(" << new_buffering_state << ") ";
   DCHECK(task_runner_->BelongsToCurrentThread());
   buffering_state_cb_.Run(new_buffering_state);
 }
 
 }  // namespace media